Deprecate MongoCollections#get (#19470)

* Deprecate MongoCollections#get * Use MongoCollections#collection instead #get * Consistently handle id attribute
Graylog2 · May 31, 2024 · 02b611a · 02b611a
1 parent d019301
commit 02b611a
Show file tree

Hide file tree

Showing 11 changed files with 48 additions and 35 deletions.
diff --git a/graylog2-server/src/main/java/org/graylog/scheduler/DBCustomJobDefinitionService.java b/graylog2-server/src/main/java/org/graylog/scheduler/DBCustomJobDefinitionService.java
@@ -42,7 +42,7 @@ public class DBCustomJobDefinitionService {
 
     @Inject
     public DBCustomJobDefinitionService(MongoCollections collections) {
-        this.db = collections.get(DBJobDefinitionService.COLLECTION_NAME, JobDefinitionDto.class);
+        this.db = collections.collection(DBJobDefinitionService.COLLECTION_NAME, JobDefinitionDto.class);
     }
 
     public JobDefinitionDto findOrCreate(JobDefinitionDto dto) {

diff --git a/graylog2-server/src/main/java/org/graylog/scheduler/DBJobTriggerService.java b/graylog2-server/src/main/java/org/graylog/scheduler/DBJobTriggerService.java
@@ -119,7 +119,7 @@ public DBJobTriggerService(MongoCollections mongoCollections,
         this.clock = clock;
         this.schedulerCapabilitiesService = schedulerCapabilitiesService;
         this.lockExpirationDuration = lockExpirationDuration;
-        this.collection = mongoCollections.get(COLLECTION_NAME, JobTriggerDto.class);
+        this.collection = mongoCollections.collection(COLLECTION_NAME, JobTriggerDto.class);
         this.mongoUtils = mongoCollections.utils(collection);
 
         collection.createIndex(Indexes.ascending(FIELD_JOB_DEFINITION_ID));

diff --git a/graylog2-server/src/main/java/org/graylog/scheduler/JobDefinitionDto.java b/graylog2-server/src/main/java/org/graylog/scheduler/JobDefinitionDto.java
@@ -20,25 +20,18 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.google.auto.value.AutoValue;
+import org.graylog2.database.MongoEntity;
 import org.mongojack.Id;
 import org.mongojack.ObjectId;
 
-import javax.annotation.Nullable;
-
 @AutoValue
 @JsonDeserialize(builder = JobDefinitionDto.Builder.class)
-public abstract class JobDefinitionDto {
+public abstract class JobDefinitionDto implements MongoEntity {
     private static final String FIELD_ID = "id";
     public static final String FIELD_TITLE = "title";
     private static final String FIELD_DESCRIPTION = "description";
     public static final String FIELD_CONFIG = "config";
 
-    @Id
-    @ObjectId
-    @Nullable
-    @JsonProperty(FIELD_ID)
-    public abstract String id();
-
     @JsonProperty(FIELD_TITLE)
     public abstract String title();
 

diff --git a/graylog2-server/src/main/java/org/graylog2/database/MongoCollections.java b/graylog2-server/src/main/java/org/graylog2/database/MongoCollections.java
@@ -46,16 +46,25 @@ public MongoCollections(MongoJackObjectMapperProvider objectMapperProvider, Mong
      * <p>
      * <b>Prefer using {@link #collection(String, Class)} to get a more strictly typed collection!</b>
      *
+     * @deprecated Use {@link #collection(String, Class)} or {@link #nonEntityCollection(String, Class)} instead.
+     */
+    @Deprecated
+    public <T> MongoCollection<T> get(String collectionName, Class<T> valueType) {
+        return nonEntityCollection(collectionName, valueType);
+    }
+
+    /**
+     * Get a MongoCollection for document types that don't implement {@link MongoEntity}.
+     * <p>
+     * <b> Prefer using {@link #collection(String, Class)}. Only use this method if, for some reason, the document
+     * class cannot implement the {@link MongoEntity} interface.</b>
+     *
      * @param collectionName Name of the collection
      * @param valueType      Java type of the documents stored in the collection
      * @return A collection using a Jackson codec for serialization and deserialization
      */
-    public <T> MongoCollection<T> get(String collectionName, Class<T> valueType) {
-        final MongoCollection<T> collection = mongoConnection.getMongoDatabase().getCollection(collectionName, valueType);
-        final CustomJacksonCodecRegistry jacksonCodecRegistry = new CustomJacksonCodecRegistry(this.objectMapper,
-                collection.getCodecRegistry());
-        jacksonCodecRegistry.addCodecForClass(valueType);
-        return collection.withCodecRegistry(jacksonCodecRegistry);
+    public <T> MongoCollection<T> nonEntityCollection(String collectionName, Class<T> valueType) {
+        return getCollection(collectionName, valueType);
     }
 
     /**
@@ -66,7 +75,7 @@ public <T> MongoCollection<T> get(String collectionName, Class<T> valueType) {
      * @return A collection using a Jackson codec for serialization and deserialization
      */
     public <T extends MongoEntity> MongoCollection<T> collection(String collectionName, Class<T> valueType) {
-        return get(collectionName, valueType);
+        return getCollection(collectionName, valueType);
     }
 
     /**
@@ -103,4 +112,13 @@ public <T extends MongoEntity> MongoUtils<T> utils(MongoCollection<T> collection
     public <T extends ScopedEntity> ScopedEntityMongoUtils<T> scopedEntityUtils(MongoCollection<T> collection, EntityScopeService entityScopeService) {
         return new ScopedEntityMongoUtils<>(collection, entityScopeService);
     }
+
+    private <T> MongoCollection<T> getCollection(String collectionName, Class<T> valueType) {
+        final MongoCollection<T> collection = mongoConnection.getMongoDatabase().getCollection(collectionName, valueType);
+        final CustomJacksonCodecRegistry jacksonCodecRegistry = new CustomJacksonCodecRegistry(this.objectMapper,
+                collection.getCodecRegistry());
+        jacksonCodecRegistry.addCodecForClass(valueType);
+        return collection.withCodecRegistry(jacksonCodecRegistry);
+    }
+
 }
diff --git a/...og2-server/src/main/java/org/graylog2/indexer/indexset/profile/IndexFieldTypeProfile.java b/...og2-server/src/main/java/org/graylog2/indexer/indexset/profile/IndexFieldTypeProfile.java
@@ -17,6 +17,7 @@
 package org.graylog2.indexer.indexset.profile;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
+import org.graylog2.database.MongoEntity;
 import org.graylog2.indexer.indexset.CustomFieldMappings;
 import org.mongojack.Id;
 import org.mongojack.ObjectId;
@@ -30,7 +31,7 @@
 public record IndexFieldTypeProfile(@JsonProperty(ID_FIELD_NAME) @Nullable @Id @ObjectId String id,
                                     @JsonProperty(NAME_FIELD_NAME) String name,
                                     @JsonProperty(DESCRIPTION_FIELD_NAME) String description,
-                                    @JsonProperty(CUSTOM_MAPPINGS_FIELD_NAME) @Nonnull CustomFieldMappings customFieldMappings) {
+                                    @JsonProperty(CUSTOM_MAPPINGS_FIELD_NAME) @Nonnull CustomFieldMappings customFieldMappings) implements MongoEntity {
 
     public static final String ID_FIELD_NAME = "id";
     public static final String NAME_FIELD_NAME = "name";

diff --git a/...ver/src/main/java/org/graylog2/indexer/indexset/profile/IndexFieldTypeProfileService.java b/...ver/src/main/java/org/graylog2/indexer/indexset/profile/IndexFieldTypeProfileService.java
@@ -90,7 +90,7 @@ public IndexFieldTypeProfileService(final MongoConnection mongoConnection,
                                         final IndexSetService indexSetService) {
         super(mongoConnection, mapper, IndexFieldTypeProfile.class, INDEX_FIELD_TYPE_PROFILE_MONGO_COLLECTION_NAME);
         this.db.createIndex(new BasicDBObject(IndexFieldTypeProfile.NAME_FIELD_NAME, 1), new BasicDBObject("unique", false));
-        this.profileCollection = mongoCollections.get(INDEX_FIELD_TYPE_PROFILE_MONGO_COLLECTION_NAME, IndexFieldTypeProfile.class);
+        this.profileCollection = mongoCollections.collection(INDEX_FIELD_TYPE_PROFILE_MONGO_COLLECTION_NAME, IndexFieldTypeProfile.class);
         this.indexSetService = indexSetService;
         this.dbQueryCreator = new DbQueryCreator(IndexFieldTypeProfile.NAME_FIELD_NAME, ATTRIBUTES);
         this.indexFieldTypeProfileUsagesService = indexFieldTypeProfileUsagesService;

diff --git a/graylog2-server/src/test/java/org/graylog2/database/HelpersAndUtilitiesTest.java b/graylog2-server/src/test/java/org/graylog2/database/HelpersAndUtilitiesTest.java
@@ -62,7 +62,7 @@ private TestDTO newDto(String title) {
     @BeforeEach
     void setUp(MongoDBTestService mongoDBTestService, MongoJackObjectMapperProvider objectMapperProvider) {
         final MongoCollections mongoCollections = new MongoCollections(objectMapperProvider, mongoDBTestService.mongoConnection());
-        collection = mongoCollections.get("test", TestDTO.class);
+        collection = mongoCollections.collection("test", TestDTO.class);
         utils = mongoCollections.utils(collection);
         paginationHelper = mongoCollections.paginationHelper(collection);
     }

diff --git a/graylog2-server/src/test/java/org/graylog2/database/MongoCollectionsTest.java b/graylog2-server/src/test/java/org/graylog2/database/MongoCollectionsTest.java
@@ -61,7 +61,7 @@ record Person(@JsonProperty("id") @Id @org.mongojack.ObjectId String id,
                   @JsonProperty("object_id") ObjectId objectId,
                   @JsonProperty("first_name") String firstName,
                   @JsonProperty("created_at") ZonedDateTime createdAt,
-                  @JsonProperty("last_modified_at") DateTime lastModifiedAt) {
+                  @JsonProperty("last_modified_at") DateTime lastModifiedAt) implements MongoEntity {
     }
 
     @JsonIgnoreProperties(ignoreUnknown = true)
@@ -71,11 +71,12 @@ record Secret(@JsonProperty("encrypted_value") EncryptedValue encryptedValue) {}
     record IgnoreTest(@JsonProperty("ignore_me_not") String ignoreMeNot,
                       @MongoIgnore @JsonProperty("ignore_me") String ignoreMe) {}
 
-    record IdGenerationTest(@Nullable @JsonProperty("id") @Id @org.mongojack.ObjectId String id) {}
+    record IdGenerationTest(@Nullable @JsonProperty("id") @Id @org.mongojack.ObjectId String id)
+            implements MongoEntity {}
 
     @JsonIgnoreProperties(ignoreUnknown = true)
     record TimestampTest(@Nullable @JsonProperty("id") @Id @org.mongojack.ObjectId String id,
-                         @JsonProperty("timestamp") DateTime timestamp) {}
+                         @JsonProperty("timestamp") DateTime timestamp) implements MongoEntity {}
 
     @BeforeEach
     void setUp(MongoDBTestService mongoDBTestService, MongoJackObjectMapperProvider mongoJackObjectMapperProvider) {
@@ -85,7 +86,7 @@ void setUp(MongoDBTestService mongoDBTestService, MongoJackObjectMapperProvider
 
     @Test
     void testBasicTypes() {
-        final MongoCollection<Person> collection = collections.get("people", Person.class);
+        final MongoCollection<Person> collection = collections.collection("people", Person.class);
         final Person person = new Person(
                 "000000000000000000000001",
                 "000000000000000000000002",
@@ -115,7 +116,7 @@ void testBasicTypes() {
 
     @Test
     void testEncryptedValue() {
-        final MongoCollection<Secret> collection = collections.get("secrets", Secret.class);
+        final MongoCollection<Secret> collection = collections.nonEntityCollection("secrets", Secret.class);
         final EncryptedValue encryptedValue = encryptedValueService.encrypt("gary");
         collection.insertOne(new Secret(encryptedValue));
         assertThat(collection.find().first()).isNotNull().satisfies(secret -> {
@@ -129,22 +130,22 @@ void testEncryptedValue() {
     void testMongoIgnore() {
         // @MongoIgnore should prevent a property from being written to Mongo. But if it's returned from Mongo,
         // e.g. because it was calculated by an aggregation, it should be populated in the returned object.
-        final MongoCollection<IgnoreTest> collection = collections.get("ignoreTest", IgnoreTest.class);
+        final MongoCollection<IgnoreTest> collection = collections.nonEntityCollection("ignoreTest", IgnoreTest.class);
         collection.insertOne(new IgnoreTest("I should be present", "I should be gone"));
         assertThat(collection.find().first()).isEqualTo(new IgnoreTest("I should be present", null));
 
-        final MongoCollection<Document> rawCollection = collections.get("alsoIgnoreTest", Document.class);
+        final MongoCollection<Document> rawCollection = collections.nonEntityCollection("alsoIgnoreTest", Document.class);
         rawCollection.insertOne(new Document(Map.of(
                 "ignore_me_not", "I should be present",
                 "ignore_me", "I sneaked in")));
 
-        final MongoCollection<IgnoreTest> collection2 = collections.get("alsoIgnoreTest", IgnoreTest.class);
+        final MongoCollection<IgnoreTest> collection2 = collections.nonEntityCollection("alsoIgnoreTest", IgnoreTest.class);
         assertThat(collection2.find().first()).isEqualTo(new IgnoreTest("I should be present", "I sneaked in"));
     }
 
     @Test
     void testIdGeneration() {
-        final MongoCollection<IdGenerationTest> collection = collections.get("id-generation-test", IdGenerationTest.class);
+        final MongoCollection<IdGenerationTest> collection = collections.collection("id-generation-test", IdGenerationTest.class);
         final var testObject = new IdGenerationTest(null);
         final InsertOneResult result = collection.insertOne(testObject);
         final BsonValue insertedId = result.getInsertedId();
@@ -155,7 +156,7 @@ void testIdGeneration() {
 
     @Test
     void testTimestampToJodaDateTimeConversion() {
-        final MongoCollection<TimestampTest> collection = collections.get("timestamp-test", TimestampTest.class);
+        final MongoCollection<TimestampTest> collection = collections.collection("timestamp-test", TimestampTest.class);
 
         final DateTime now = DateTime.now(DateTimeZone.UTC);
 

diff --git a/...rver/src/test/java/org/graylog2/database/pagination/DefaultMongoPaginationHelperTest.java b/...rver/src/test/java/org/graylog2/database/pagination/DefaultMongoPaginationHelperTest.java
@@ -65,7 +65,7 @@ private record DTO(String id, String name) implements MongoEntity {}
     @BeforeEach
     void setUp(MongoDBTestService mongoDBTestService, MongoJackObjectMapperProvider objectMapperProvider) {
         mongoCollections = new MongoCollections(objectMapperProvider, mongoDBTestService.mongoConnection());
-        collection = mongoCollections.get("test", DTO.class);
+        collection = mongoCollections.collection("test", DTO.class);
         paginationHelper = new DefaultMongoPaginationHelper<>(collection);
 
         collection.insertMany(DTOs);

diff --git a/graylog2-server/src/test/java/org/graylog2/database/utils/MongoUtilsTest.java b/graylog2-server/src/test/java/org/graylog2/database/utils/MongoUtilsTest.java
@@ -52,7 +52,7 @@ private record DTO(String id, String name) implements MongoEntity {}
     @BeforeEach
     void setUp(MongoDBTestService mongoDBTestService, MongoJackObjectMapperProvider objectMapperProvider) {
         mongoCollections = new MongoCollections(objectMapperProvider, mongoDBTestService.mongoConnection());
-        collection = mongoCollections.get("test", DTO.class);
+        collection = mongoCollections.collection("test", DTO.class);
         utils = mongoCollections.utils(collection);
     }
 
@@ -76,7 +76,7 @@ void testInsertedIdAsString() {
 
     @Test
     void testNullInsertedId() {
-        final var rawCollection = mongoCollections.get("raw_bson_test", RawBsonDocument.class);
+        final var rawCollection = mongoCollections.nonEntityCollection("raw_bson_test", RawBsonDocument.class);
         final RawBsonDocument doc = RawBsonDocument.parse("{\"name\":\"a\"}");
         assertThatThrownBy(() -> insertedId(rawCollection.insertOne(doc)))
                 .isInstanceOf(IllegalArgumentException.class)

diff --git a/graylog2-server/src/test/java/org/graylog2/database/utils/ScopedEntityMongoUtilsTest.java b/graylog2-server/src/test/java/org/graylog2/database/utils/ScopedEntityMongoUtilsTest.java
@@ -45,7 +45,7 @@ public class ScopedEntityMongoUtilsTest {
     @BeforeEach
     void setUp(MongoDBTestService mongoDBTestService, MongoJackObjectMapperProvider objectMapperProvider) {
         MongoCollections mongoCollections = new MongoCollections(objectMapperProvider, mongoDBTestService.mongoConnection());
-        collection = mongoCollections.get("test", ScopedDTO.class);
+        collection = mongoCollections.collection("test", ScopedDTO.class);
         EntityScopeService scopeService = new EntityScopeService(Set.of(
                 new DefaultEntityScope(),
                 new ImmutableScope(),