address all comments

kbase · Jan 18, 2024 · 07aabea · 07aabea
1 parent 0f7db8e
commit 07aabea
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 46 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -27,6 +27,9 @@ jobs:
           - java: '8'
             mongo: 'mongodb-linux-x86_64-ubuntu2204-7.0.4'
             wired_tiger: 'false'
+          - java: '8'
+            mongo: 'mongodb-linux-x86_64-ubuntu2204-7.0.4'
+            wired_tiger: 'true'
           - java: '11'
             mongo: 'mongodb-linux-x86_64-3.6.23'
             wired_tiger: 'true'

diff --git a/build.xml b/build.xml
@@ -30,7 +30,6 @@
     <include name="google/guava-18.0.jar"/>
     <include name="ini4j/ini4j-0.5.2.jar"/>
     <include name="jcommander/jcommander-1.48.jar"/>
-<!--    <include name="mongo/mongo-java-driver-3.8.2.jar"/>-->
     <include name="mongo/mongodb-driver-core-4.11.1.jar"/>
     <include name="mongo/mongodb-driver-sync-4.11.1.jar"/>
     <include name="bson/bson-record-codec-4.11.1.jar"/>

diff --git a/src/us/kbase/auth2/service/AuthBuilder.java b/src/us/kbase/auth2/service/AuthBuilder.java
@@ -64,25 +64,17 @@ public AuthBuilder(
 
 	private MongoClient buildMongo(final AuthStartupConfig c) throws StorageInitException {
 		//TODO ZLATER MONGO handle shards & replica sets
+		MongoClientSettings.Builder mongoBuilder = MongoClientSettings.builder().applyToClusterSettings(
+				builder -> builder.hosts(Arrays.asList(new ServerAddress(c.getMongoHost()))));
+
 		try {
 			if (c.getMongoUser().isPresent()) {
 				final MongoCredential creds = MongoCredential.createCredential(
 						c.getMongoUser().get(), c.getMongoDatabase(), c.getMongoPwd().get());
 				// unclear if and when it's safe to clear the password
-				return MongoClients.create(
-						MongoClientSettings.builder()
-								.credential(creds)
-								.applyToClusterSettings(builder ->
-										builder.hosts(Arrays.asList(
-												new ServerAddress(c.getMongoHost()))))
-								.build());
+				return MongoClients.create(mongoBuilder.credential(creds).build());
 			} else {
-				return MongoClients.create(
-						MongoClientSettings.builder()
-								.applyToClusterSettings(builder ->
-										builder.hosts(Arrays.asList(
-												new ServerAddress(c.getMongoHost()))))
-								.build());
+				return MongoClients.create(mongoBuilder.build());
 			}
 		} catch (MongoException e) {
 			LoggerFactory.getLogger(getClass()).error(

diff --git a/src/us/kbase/test/auth2/MongoController.java b/src/us/kbase/test/auth2/MongoController.java
@@ -34,8 +34,8 @@ public class MongoController {
         tempDirectories.add(DATA_DIR);
     }
 
-    private final static Version MONGO_DB_3 =
-            Version.forIntegers(3,6,23);
+    private final static Version MONGO_DB_6_1 =
+            Version.forIntegers(6,1);
 
     private final Path tempDir;
 
@@ -109,8 +109,11 @@ public List<String> getMongoServerStartCommand(final String mongoExe,
         command.addAll(Arrays.asList(mongoExe, "--port", "" + port,
                 "--dbpath", tempDir.resolve(DATA_DIR).toString()));
 
-        // In version 3.6, the --nojournal option is deprecated
-        if (dbVer.lessThanOrEqualTo(MONGO_DB_3)) {
+        // Starting in MongoDB 6.1, journaling is always enabled.
+        // As a result, MongoDB removes the storage.journal.enabled option
+        // and the corresponding --journal and --nojournal command-line options.
+        // https://www.mongodb.com/docs/manual/release-notes/6.1/#changes-to-journaling
+        if (dbVer.lessThan(MONGO_DB_6_1)) {
             command.addAll(Arrays.asList("--nojournal"));
         }
         if (useWiredTiger) {

diff --git a/src/us/kbase/test/auth2/lib/storage/mongo/MongoStorageDuplicateKeyCheckerTest.java b/src/us/kbase/test/auth2/lib/storage/mongo/MongoStorageDuplicateKeyCheckerTest.java
@@ -130,8 +130,9 @@ public void unparseable() throws Exception {
 			getInstance(e);
 			fail("expected exception");
 		} catch (InvocationTargetException ex) {
-			assertThat("some dup key message", ex.getTargetException().getMessage(),
-					is("Unable to parse duplicate key error: Write operation error on server 127.0.0.1:27017. " +
+			TestCommon.assertExceptionCorrect((Exception) ex.getTargetException(),
+					new AuthStorageException("Unable to parse duplicate key error: " +
+							"Write operation error on server 127.0.0.1:27017. " +
 							"Write error: WriteError{code=11000, message='some "));
 		}
 	}

diff --git a/src/us/kbase/test/auth2/lib/storage/mongo/MongoStorageStartUpTest.java b/src/us/kbase/test/auth2/lib/storage/mongo/MongoStorageStartUpTest.java
@@ -2,10 +2,13 @@
 
 import static org.hamcrest.CoreMatchers.is;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 import static us.kbase.test.auth2.TestCommon.set;
 
-import java.util.*;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.function.Consumer;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -151,8 +154,7 @@ public void checkCollectionNames() throws Exception {
 
 	@Test
 	public void indexesConfig() {
-		final Set<Document> indexes = new HashSet<>();
-		updateIndexes("config", indexes);
+		final Set<Document> indexes = updateIndexes("config");
 		assertThat("incorrect indexes", indexes, is(set(
 				new Document("v", indexVer)
 						.append("unique", true)
@@ -166,8 +168,7 @@ public void indexesConfig() {
 
 	@Test
 	public void indexesConfigApp() {
-		final Set<Document> indexes = new HashSet<>();
-		updateIndexes("config_app", indexes);
+		final Set<Document> indexes = updateIndexes("config_app");
 		assertThat("incorrect indexes", indexes, is(set(
 				new Document("v", indexVer)
 						.append("unique", true)
@@ -181,8 +182,7 @@ public void indexesConfigApp() {
 
 	@Test
 	public void indexesConfigExt() {
-		final Set<Document> indexes = new HashSet<>();
-		updateIndexes("config_ext", indexes);
+		final Set<Document> indexes = updateIndexes("config_ext");
 		assertThat("incorrect indexes", indexes, is(set(
 				new Document("v", indexVer)
 						.append("unique", true)
@@ -196,8 +196,7 @@ public void indexesConfigExt() {
 
 	@Test
 	public void indexesConfigProv() {
-		final Set<Document> indexes = new HashSet<>();
-		updateIndexes("config_prov", indexes);
+		final Set<Document> indexes = updateIndexes("config_prov");
 		assertThat("incorrect indexes", indexes, is(set(
 				new Document("v", indexVer)
 						.append("unique", true)
@@ -211,8 +210,7 @@ public void indexesConfigProv() {
 
 	@Test
 	public void indexesCustRoles() {
-		final Set<Document> indexes = new HashSet<>();
-		updateIndexes("cust_roles", indexes);
+		final Set<Document> indexes = updateIndexes("cust_roles");
 		assertThat("incorrect indexes", indexes, is(set(
 				new Document("v", indexVer)
 						.append("unique", true)
@@ -226,8 +224,7 @@ public void indexesCustRoles() {
 
 	@Test
 	public void indexesTestCustRoles() {
-		final Set<Document> indexes = new HashSet<>();
-		updateIndexes("test_cust_roles", indexes);
+		final Set<Document> indexes = updateIndexes("test_cust_roles");
 		assertThat("incorrect indexes", indexes, is(set(
 				new Document("v", indexVer)
 						.append("unique", true)
@@ -245,8 +242,7 @@ public void indexesTestCustRoles() {
 
 	@Test
 	public void indexesTempData() {
-		final Set<Document> indexes = new HashSet<>();
-		updateIndexes("tempdata", indexes);
+		final Set<Document> indexes = updateIndexes("tempdata");
 		assertThat("incorrect indexes", indexes, is(set(
 				new Document("v", indexVer)
 						.append("unique", true)
@@ -272,8 +268,7 @@ public void indexesTempData() {
 
 	@Test
 	public void indexesTokens() {
-		final Set<Document> indexes = new HashSet<>();
-		updateIndexes("tokens", indexes);
+		final Set<Document> indexes = updateIndexes("tokens");
 		assertThat("incorrect indexes", indexes, is(set(
 				new Document("v", indexVer)
 						.append("unique", true)
@@ -298,8 +293,7 @@ public void indexesTokens() {
 
 	@Test
 	public void indexesTestTokens() {
-		final Set<Document> indexes = new HashSet<>();
-		updateIndexes("test_tokens", indexes);
+		final Set<Document> indexes = updateIndexes("test_tokens");
 		assertThat("incorrect indexes", indexes, is(set(
 				new Document("v", indexVer)
 						.append("unique", true)
@@ -324,8 +318,7 @@ public void indexesTestTokens() {
 
 	@Test
 	public void indexesUsers() {
-		final Set<Document> indexes = new HashSet<>();
-		updateIndexes("users", indexes);
+		final Set<Document> indexes = updateIndexes("users");
 		assertThat("incorrect indexes", indexes, is(set(
 				new Document("v", indexVer)
 						.append("key", new Document("custrls", 1))
@@ -359,8 +352,7 @@ public void indexesUsers() {
 
 	@Test
 	public void indexesTestUsers() {
-		final Set<Document> indexes = new HashSet<>();
-		updateIndexes("test_users", indexes);
+		final Set<Document> indexes = updateIndexes("test_users");
 		assertThat("incorrect indexes", indexes, is(set(
 				new Document("v", indexVer)
 						.append("key", new Document("custrls", 1))
@@ -387,13 +379,18 @@ public void indexesTestUsers() {
 				)));
 	}
 
-	public void updateIndexes(final String name, final Set<Document> indexes) {
+	public Set<Document> updateIndexes(final String name) {
+		final Set<Document> indexes = new HashSet<>();
 		for (Document index: db.getCollection(name).listIndexes()) {
+			// In MongoDB 4.4, the listIndexes and the mongo shell helper method db.collection.getIndexes()
+			// no longer returns the namespace ns field in the index specification documents.
 			index.remove("ns");
+			// some versions of Mongo return ints, some longs. Convert all to longs.
 			if (index.containsKey("expireAfterSeconds")) {
-				index.put("expireAfterSeconds", Long.valueOf(index.get("expireAfterSeconds").toString()));
+				index.put("expireAfterSeconds", Long.valueOf(index.getInteger("expireAfterSeconds")));
 			}
 			indexes.add(index);
 		}
+		return indexes;
 	}
 }