bugfix: fixed next page id calculation (#195)

# Goal The goal of this PR is fix the bug which exports only the last page instead of all of them when more than 1 page is created via adding connections. Closes #194 # Checklist - [X] updated versions - [X] Tests added
ProjectLibertyLabs · Apr 3, 2024 · 0d03e91 · 0d03e91
1 parent a04476c
commit 0d03e91
Show file tree

Hide file tree

Showing 12 changed files with 105 additions and 37 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bridge/common/Cargo.toml b/bridge/common/Cargo.toml
@@ -1,12 +1,12 @@
 [package]
 name = "dsnp-graph-sdk-common"
-version = "1.1.0"
+version = "1.1.1"
 edition = "2021"
 license = "Apache-2.0"
 publish = false
 
 [dependencies]
-dsnp-graph-core = { version = "1.1.0", path = "../../core" }
-dsnp-graph-config = { version = "1.1.0", path = "../../config" }
+dsnp-graph-core = { version = "1.1.1", path = "../../core" }
+dsnp-graph-config = { version = "1.1.1", path = "../../config" }
 libc = "0.2.153"
 protobuf = { version = "3.4.0", features = ["with-bytes"] }
diff --git a/bridge/ffi/Cargo.toml b/bridge/ffi/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "dsnp-graph-sdk-ffi"
-version = "1.1.0"
+version = "1.1.1"
 edition = "2021"
 license = "Apache-2.0"
 publish = false
@@ -10,8 +10,8 @@ name = "dsnp_graph_sdk_ffi"
 crate-type = ["staticlib"]
 
 [dependencies]
-dsnp-graph-core = { version = "1.1.0", path = "../../core" }
-dsnp-graph-config = { version = "1.1.0", path = "../../config" }
+dsnp-graph-core = { version = "1.1.1", path = "../../core" }
+dsnp-graph-config = { version = "1.1.1", path = "../../config" }
 libc = "0.2.153"
 lazy_static = "1.4.0"
 anyhow = "1.0.80"
diff --git a/bridge/jni/Cargo.toml b/bridge/jni/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "dsnp-graph-sdk-jni"
-version = "1.1.0"
+version = "1.1.1"
 edition = "2021"
 license = "Apache-2.0"
 publish = false
@@ -10,9 +10,9 @@ name = "dsnp_graph_sdk_jni"
 crate-type = ["cdylib"]
 
 [dependencies]
-dsnp-graph-core = { version = "1.1.0", path = "../../core" }
-dsnp-graph-config = { version = "1.1.0", path = "../../config" }
-dsnp-graph-sdk-common = { version = "1.1.0", path = "../common" }
+dsnp-graph-core = { version = "1.1.1", path = "../../core" }
+dsnp-graph-config = { version = "1.1.1", path = "../../config" }
+dsnp-graph-sdk-common = { version = "1.1.1", path = "../common" }
 jni = "0.21.1"
 cfg-if = "1.0.0"
 log = { version = "^0.4.21", features = ["std", "max_level_debug", "release_max_level_debug"] }

diff --git a/bridge/node/Cargo.toml b/bridge/node/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "dsnp-graph-sdk-node"
-version = "1.1.0"
+version = "1.1.1"
 edition = "2021"
 license = "Apache-2.0"
 publish = false
@@ -11,8 +11,8 @@ name = "dsnp_graph_sdk_node"
 crate-type = ["cdylib"]
 
 [dependencies]
-dsnp-graph-core = { version = "1.1.0", path = "../../core" }
-dsnp-graph-config = { version = "1.1.0", path = "../../config" }
+dsnp-graph-core = { version = "1.1.1", path = "../../core" }
+dsnp-graph-config = { version = "1.1.1", path = "../../config" }
 neon = { version = "1.0.0", default-features = false, features = ["napi-6"] }
 serde = { version = "1.0.197", features = ["derive"] }
 serde_json = "1.0.114"

diff --git a/bridge/node/package-lock.json b/bridge/node/package-lock.json
diff --git a/bridge/node/package.json b/bridge/node/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@dsnp/graph-sdk",
-  "version": "1.1.0",
+  "version": "1.1.1",
   "author": "Amplica Labs",
   "license": "Apache-2.0",
   "description": "dsnp-graph-sdk-node",
@@ -55,6 +55,6 @@
   },
   "homepage": "https://github.com/LibertyDSNP/graph-sdk/bridge/node/README.md",
   "customProperties": {
-    "uploadedBinariesVersion": "v1.1.0"
+    "uploadedBinariesVersion": "v1.1.1"
   }
 }
diff --git a/config/Cargo.toml b/config/Cargo.toml
@@ -5,7 +5,7 @@ homepage = "https://spec.dsnp.org"
 license = "Apache-2.0"
 publish = false
 repository = "https://github.com/libertyDSNP/graph-sdk/"
-version = "1.1.0"
+version = "1.1.1"
 
 [lib]
 name = "dsnp_graph_config"

diff --git a/core/Cargo.toml b/core/Cargo.toml
@@ -5,7 +5,7 @@ homepage = "https://spec.dsnp.org"
 license = "Apache-2.0"
 publish = false
 repository = "https://github.com/libertyDSNP/graph-sdk/"
-version = "1.1.0"
+version = "1.1.1"
 
 [lib]
 name = "dsnp_graph_core"
@@ -16,7 +16,7 @@ doctest = false
 anyhow = "1.0.69"
 apache-avro = { version = "0.16.0", features = ["snappy"] }
 dryoc = "0.5.3"
-dsnp-graph-config = { version = "1.1.0", path = "../config" }
+dsnp-graph-config = { version = "1.1.1", path = "../config" }
 lazy_static = "1.4.0"
 log = { version = "^0.4.21", features = ["std", "max_level_debug", "release_max_level_debug"] }
 log-result-proc-macro = { path = "../log-result-proc-macro" }

diff --git a/core/src/api/api.rs b/core/src/api/api.rs
@@ -733,7 +733,7 @@ mod test {
 	}
 
 	#[test]
-	#[timeout(3000)]
+	#[timeout(100000)]
 	fn add_large_number_of_follows_to_private_follow_graph_should_succeed() {
 		// arrange
 		let env = Environment::Mainnet;
@@ -751,9 +751,9 @@ mod test {
 			key_type: GraphKeyType::X25519,
 		};
 		let dsnp_user_id = 7002;
-		let input = ImportBundleBuilder::new(env, dsnp_user_id, schema_id)
-			.with_key_pairs(&vec![keypair])
-			.with_encryption_key(resolved_key)
+		let input = ImportBundleBuilder::new(env.clone(), dsnp_user_id, schema_id)
+			.with_key_pairs(&vec![keypair.clone()])
+			.with_encryption_key(resolved_key.clone())
 			.build();
 
 		// act
@@ -792,6 +792,34 @@ mod test {
 
 		// assert
 		assert!(res.is_ok());
+
+		let connections =
+			state.get_connections_for_user_graph(&dsnp_user_id, &schema_id, true).unwrap();
+		let before_export_set: HashSet<_> = connections.iter().map(|e| e.user_id).collect();
+
+		let export = state.export_updates();
+
+		assert!(export.is_ok());
+		println!("after export physical mem: {}", mem_usage.physical_mem);
+
+		let updates = export.unwrap();
+
+		let mut updated_state = GraphState::new(env.clone());
+		let updated_input = ImportBundleBuilder::new(env.clone(), dsnp_user_id, schema_id)
+			.with_key_pairs(&vec![keypair])
+			.with_encryption_key(resolved_key.clone())
+			.build();
+
+		let new_import = ImportBundleBuilder::build_from(&updated_input, &updates);
+		let res = updated_state.import_users_data(&vec![new_import]);
+
+		assert!(res.is_ok());
+
+		let connections = updated_state
+			.get_connections_for_user_graph(&dsnp_user_id, &schema_id, false)
+			.unwrap();
+		let after_reimport_set: HashSet<_> = connections.iter().map(|e| e.user_id).collect();
+		assert_eq!(before_export_set, after_reimport_set);
 	}
 
 	#[test]

diff --git a/core/src/graph/graph.rs b/core/src/graph/graph.rs
@@ -114,8 +114,17 @@ impl Graph {
 	}
 
 	/// Get next available PageId for this graph
-	pub fn get_next_available_page_id(&self) -> Option<PageId> {
-		let existing_pages = self.pages.inner().keys().cloned().collect::<HashSet<PageId>>();
+	pub fn get_next_available_page_id(
+		&self,
+		updated_pages: &BTreeMap<PageId, GraphPage>,
+	) -> Option<PageId> {
+		let existing_pages = self
+			.pages
+			.inner()
+			.keys()
+			.cloned()
+			.chain(updated_pages.keys().cloned())
+			.collect::<HashSet<PageId>>();
 		(0..=(self.environment.get_config().max_page_id as PageId))
 			.find(|&pid| !existing_pages.contains(&pid))
 	}
@@ -338,7 +347,7 @@ impl Graph {
 		// At this point, all existing pages are aggressively full. Add new pages
 		// as needed to accommodate any remaining connections to be added, filling aggressively.
 		while let Some(_) = add_iter.peek() {
-			let mut new_page = match self.get_next_available_page_id() {
+			let mut new_page = match self.get_next_available_page_id(&updated_pages) {
 				Some(next_page_id) =>
 					Ok(GraphPage::new(self.get_connection_type().privacy_type(), next_page_id)),
 				None => Err(DsnpGraphError::GraphIsFull),
@@ -862,7 +871,7 @@ mod test {
 			))),
 		};
 
-		assert_eq!(graph.get_next_available_page_id(), None);
+		assert_eq!(graph.get_next_available_page_id(&BTreeMap::default()), None);
 	}
 
 	#[test]
@@ -890,7 +899,35 @@ mod test {
 			))),
 		};
 
-		assert_eq!(graph.get_next_available_page_id(), Some(8));
+		assert_eq!(graph.get_next_available_page_id(&BTreeMap::default()), Some(8));
+	}
+
+	#[test]
+	fn get_next_available_page_should_include_updated_pages() {
+		let environment = Environment::Mainnet;
+		let user_id = 3;
+		const CONN_TYPE: ConnectionType = ConnectionType::Follow(PrivacyType::Public);
+		const PRIV_TYPE: PrivacyType = CONN_TYPE.privacy_type();
+		let schema_id = environment
+			.get_config()
+			.get_schema_id_from_connection_type(CONN_TYPE)
+			.expect("should exist");
+		let mut updated_pages: BTreeMap<_, _> = (0..environment.get_config().max_page_id as PageId)
+			.map(|page_id: PageId| (page_id, GraphPage::new(PRIV_TYPE, page_id)))
+			.collect();
+		updated_pages.remove(&8);
+		let graph = Graph {
+			environment,
+			schema_id, // doesn't matter which type
+			user_id,
+			pages: PageMap::new(),
+			user_key_manager: Arc::new(RwLock::new(UserKeyManager::new(
+				user_id,
+				Arc::new(RwLock::new(SharedStateManager::new())),
+			))),
+		};
+
+		assert_eq!(graph.get_next_available_page_id(&updated_pages), Some(8));
 	}
 
 	#[test]

diff --git a/core/src/tests/helpers.rs b/core/src/tests/helpers.rs
@@ -20,7 +20,10 @@ use base64::{engine::general_purpose, Engine as _};
 use ctor::ctor;
 use dryoc::keypair::StackKeyPair;
 use dsnp_graph_config::{DsnpVersion, Environment, GraphKeyType};
-use std::sync::{Arc, RwLock};
+use std::{
+	collections::BTreeMap,
+	sync::{Arc, RwLock},
+};
 
 #[ctor]
 fn test_harness_init() {
@@ -134,7 +137,7 @@ pub fn create_aggressively_full_page(
 	shared_state: &Arc<RwLock<SharedStateManager>>,
 ) -> PageId {
 	let connection_type = graph.get_connection_type();
-	let page_id = graph.get_next_available_page_id().unwrap();
+	let page_id = graph.get_next_available_page_id(&BTreeMap::default()).unwrap();
 	let mut page = GraphPage::new(connection_type.privacy_type(), page_id);
 	let mut connection_id = start_conn_id;
 	let encryption_key = graph