Improve test assertions

carlesarnal · carlesarnal · commit c4d940649ef1 · 2024-08-01T16:51:08.000+02:00
diff --git a/app/src/main/java/io/apicurio/registry/storage/impl/kafkasql/KafkaSqlRegistryStorage.java b/app/src/main/java/io/apicurio/registry/storage/impl/kafkasql/KafkaSqlRegistryStorage.java
@@ -621,6 +621,8 @@ public void importData(EntityInputStream entities, boolean preserveGlobalId, boo
         DataImporter dataImporter = new SqlDataImporter(log, utils, this, preserveGlobalId,
                 preserveContentId);
         dataImporter.importData(entities, () -> {
+            // TODO Re-visit this, since Apicurio Registry 3 all messages live in the same partition, so there
+            // should be no need to wait.
             // Because importing just pushes a bunch of Kafka messages, we may need to
             // wait for a few seconds before we send the reset messages. Due to partitioning,
             // we can't guarantee ordering of these next two messages, and we NEED them to
@@ -636,7 +638,7 @@ public void importData(EntityInputStream entities, boolean preserveGlobalId, boo
     }
 
     /**
-     * @see io.apicurio.registry.storage.RegistryStorage#importData(io.apicurio.registry.storage.impexp.EntityInputStream,
+     * @see io.apicurio.registry.storage.RegistryStorage#upgradeData(io.apicurio.registry.storage.impexp.EntityInputStream,
      *      boolean, boolean)
      */
     @Override
@@ -645,6 +647,8 @@ public void upgradeData(EntityInputStream entities, boolean preserveGlobalId, bo
         DataImporter dataImporter = new SqlDataUpgrader(log, utils, this, preserveGlobalId,
                 preserveContentId);
         dataImporter.importData(entities, () -> {
+            // TODO Re-visit this, since Apicurio Registry 3 all messages live in the same partition, so there
+            // should be no need to wait.
             // Because importing just pushes a bunch of Kafka messages, we may need to
             // wait for a few seconds before we send the reset messages. Due to partitioning,
             // we can't guarantee ordering of these next two messages, and we NEED them to
diff --git a/app/src/main/java/io/apicurio/registry/storage/importing/v2/SqlDataUpgrader.java b/app/src/main/java/io/apicurio/registry/storage/importing/v2/SqlDataUpgrader.java
@@ -5,10 +5,12 @@
 import io.apicurio.registry.model.GAV;
 import io.apicurio.registry.storage.RegistryStorage;
 import io.apicurio.registry.storage.dto.ArtifactReferenceDto;
+import io.apicurio.registry.storage.error.InvalidArtifactTypeException;
 import io.apicurio.registry.storage.error.VersionAlreadyExistsException;
 import io.apicurio.registry.storage.impexp.EntityInputStream;
 import io.apicurio.registry.storage.impl.sql.RegistryStorageContentUtils;
 import io.apicurio.registry.storage.impl.sql.SqlUtil;
+import io.apicurio.registry.types.ContentTypes;
 import io.apicurio.registry.types.RegistryException;
 import io.apicurio.registry.types.VersionState;
 import io.apicurio.registry.utils.impexp.Entity;
@@ -32,6 +34,13 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import static io.apicurio.registry.types.ArtifactType.*;
+
+/**
+ * This class takes a stream of Registry v2 entities and imports them into the application using
+ * {@link SqlDataUpgrader#importData(EntityInputStream, Runnable)} as it's entry point. It must be used in the
+ * upgrade process from v2 to v3.
+ */
 public class SqlDataUpgrader extends AbstractDataImporter {
 
     protected RegistryStorageContentUtils utils;
@@ -97,7 +106,7 @@ public void importArtifactVersion(ArtifactVersionEntity entity) {
             io.apicurio.registry.utils.impexp.v3.ArtifactVersionEntity newEntity = io.apicurio.registry.utils.impexp.v3.ArtifactVersionEntity
                     .builder().createdOn(entity.createdOn).description(entity.description)
                     .labels(entity.labels != null
-                        ? entity.labels.stream().collect(Collectors.toMap(label -> label, label -> label))
+                        ? entity.labels.stream().collect(Collectors.toMap(label -> label, label -> null))
                         : Collections.emptyMap())
                     .name(entity.name).owner(entity.createdBy)
                     .state(VersionState.fromValue(entity.state.value())).artifactId(entity.artifactId)
@@ -111,7 +120,7 @@ public void importArtifactVersion(ArtifactVersionEntity entity) {
                         .artifactType(entity.artifactType).createdOn(entity.createdOn)
                         .description(entity.description).groupId(entity.groupId)
                         .labels(entity.labels != null
-                            ? entity.labels.stream().collect(Collectors.toMap(label -> label, label -> label))
+                            ? entity.labels.stream().collect(Collectors.toMap(label -> label, s -> null))
                             : Collections.emptyMap())
                         .modifiedBy(entity.createdBy).modifiedOn(entity.createdOn).name(entity.name)
                         .owner(entity.createdBy).build();
@@ -169,10 +178,10 @@ public void importContent(ContentEntity entity) {
 
             // Finally, using the information from the old content, a V3 content entity is created.
             io.apicurio.registry.utils.impexp.v3.ContentEntity newEntity = io.apicurio.registry.utils.impexp.v3.ContentEntity
-                    .builder().contentType(entity.artifactType).contentHash(entity.contentHash)
-                    .artifactType(entity.artifactType).contentBytes(entity.contentBytes)
-                    .serializedReferences(entity.serializedReferences).canonicalHash(entity.canonicalHash)
-                    .contentId(entity.contentId).build();
+                    .builder().contentType(determineContentType(entity.artifactType, typedContent))
+                    .contentHash(entity.contentHash).artifactType(entity.artifactType)
+                    .contentBytes(entity.contentBytes).serializedReferences(entity.serializedReferences)
+                    .canonicalHash(entity.canonicalHash).contentId(entity.contentId).build();
 
             storage.importContent(newEntity);
             log.debug("Content imported successfully: {}", entity);
@@ -210,7 +219,7 @@ public void importGroup(GroupEntity entity) {
         try {
             io.apicurio.registry.utils.impexp.v3.GroupEntity newEntity = io.apicurio.registry.utils.impexp.v3.GroupEntity
                     .builder().artifactsType(entity.artifactsType).createdOn(entity.createdOn)
-                    .description(entity.description).groupId(entity.groupId).labels(entity.properties)
+                    .description(entity.description).groupId(entity.groupId).labels(Collections.emptyMap())
                     .modifiedBy(entity.modifiedBy).modifiedOn(entity.modifiedOn).owner(entity.createdBy)
                     .build();
             storage.importGroup(newEntity);
@@ -267,4 +276,28 @@ public void importData(EntityInputStream entities, Runnable postImportAction) {
             throw new RegistryException("Could not read next entity to import", ex);
         }
     }
+
+    private String determineContentType(String artifactTypeHint, TypedContent content) {
+        if (content.getContentType() != null) {
+            return content.getContentType();
+        } else {
+            switch (artifactTypeHint) {
+                case ASYNCAPI:
+                case JSON:
+                case OPENAPI:
+                case AVRO:
+                    // WARNING: This is only safe here. We can safely return JSON because in V2 we were
+                    // transforming all YAML to JSON before storing the content in the database.
+                    return ContentTypes.APPLICATION_JSON;
+                case PROTOBUF:
+                    return ContentTypes.APPLICATION_PROTOBUF;
+                case GRAPHQL:
+                    return ContentTypes.APPLICATION_GRAPHQL;
+                case XML:
+                case XSD:
+                    return ContentTypes.APPLICATION_XML;
+            }
+        }
+        throw new InvalidArtifactTypeException("Invalid or unknown artifact type: " + artifactTypeHint);
+    }
 }
diff --git a/app/src/main/java/io/apicurio/registry/upgrade/v2/DataUpgrader.java b/app/src/main/java/io/apicurio/registry/upgrade/v2/DataUpgrader.java
@@ -1,4 +1,4 @@
-package io.apicurio.registry.upgrade;
+package io.apicurio.registry.upgrade.v2;
 
 import io.apicurio.common.apps.config.Info;
 import io.apicurio.registry.storage.RegistryStorage;
@@ -23,6 +23,11 @@
 import java.util.Optional;
 import java.util.zip.ZipInputStream;
 
+/**
+ * This class, once the storage is ready, checks if a v2 export file has been configured to restore the data
+ * from an older server and, if there's one present, triggers the data upgrade process, eventually changing
+ * the data structure and inserting the information into the database in v3 format.
+ */
 @ApplicationScoped
 public class DataUpgrader {
 
@@ -34,13 +39,13 @@ public class DataUpgrader {
     RegistryStorage storage;
 
     @ConfigProperty(name = "apicurio.upgrade.file.location")
-    @Info(category = "import", description = "The import URL", availableSince = "3.0.0")
-    Optional<URL> regisrtryV2ExportFile;
+    @Info(category = "upgrade", description = "The URL to a v2 export file", availableSince = "3.0.0")
+    Optional<URL> registryV2ExportFile;
 
     void onStorageReady(@ObservesAsync StorageEvent ev) {
-        if (StorageEventType.READY.equals(ev.getType()) && regisrtryV2ExportFile.isPresent()) {
+        if (StorageEventType.READY.equals(ev.getType()) && registryV2ExportFile.isPresent()) {
             log.info("Registry V2 export file exists.");
-            final URL registryV2ExportUrl = regisrtryV2ExportFile.get();
+            final URL registryV2ExportUrl = registryV2ExportFile.get();
             try (final InputStream registryV2ExportZip = new BufferedInputStream(
                     registryV2ExportUrl.openStream())) {
                 log.info("Importing {} on startup.", registryV2ExportUrl);
diff --git a/app/src/test/java/io/apicurio/registry/DataUpgradeTest.java b/app/src/test/java/io/apicurio/registry/DataUpgradeTest.java
@@ -1,14 +1,17 @@
 package io.apicurio.registry;
 
+import io.apicurio.registry.model.GroupId;
+import io.apicurio.registry.rest.client.models.ArtifactReference;
+import io.apicurio.registry.utils.IoUtil;
 import io.apicurio.registry.utils.tests.ApicurioTestTags;
 import io.quarkus.test.junit.QuarkusTest;
 import io.quarkus.test.junit.TestProfile;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
 
-import static io.restassured.RestAssured.given;
-import static org.hamcrest.Matchers.is;
+import java.util.List;
 
 @QuarkusTest
 @TestProfile(DataUpgradeTestProfile.class)
@@ -22,8 +25,77 @@ protected void beforeEach() throws Exception {
     }
 
     @Test
-    public void testCheckImportedData() throws Exception {
-        given().when().accept(CT_JSON).get("/registry/v3/search/artifacts").then().statusCode(200)
-                .body("count", is(26));
+    public void testArtifactsCount() {
+        Assertions.assertEquals(26, clientV3.search().artifacts().get().getCount());
+    }
+
+    @Test
+    public void testCheckGlobalRules() throws Exception {
+        // Global rules are enabled in the export file, they must be activated.
+        Assertions.assertEquals(3, clientV3.admin().rules().get().size());
+        Assertions.assertEquals("FULL", clientV3.admin().rules().byRuleType("VALIDITY").get().getConfig());
+        Assertions.assertEquals("BACKWARD",
+                clientV3.admin().rules().byRuleType("COMPATIBILITY").get().getConfig());
+        Assertions.assertEquals("FULL", clientV3.admin().rules().byRuleType("INTEGRITY").get().getConfig());
+    }
+
+    @Test
+    public void testCheckAvroWithReferences() throws Exception {
+        String dereferencedContent = IoUtil.toString(clientV3.groups().byGroupId("default").artifacts()
+                .byArtifactId("AvroSerdeReferencesExample-value").versions().byVersionExpression("1")
+                .content().get());
+
+        Assertions.assertEquals(
+                "{\"type\":\"record\",\"name\":\"TradeRaw\",\"namespace\":\"com.kubetrade.schema.trade\",\"fields\":[{\"name\":\"tradeKey\",\"type\":{\"type\":\"record\",\"name\":\"TradeKey\",\"fields\":[{\"name\":\"exchange\",\"type\":{\"type\":\"enum\",\"name\":\"Exchange\",\"namespace\":\"com.kubetrade.schema.common\",\"symbols\":[\"GEMINI\"]}},{\"name\":\"key\",\"type\":{\"type\":\"string\",\"avro.java.string\":\"String\"}}]}},{\"name\":\"symbol\",\"type\":{\"type\":\"string\",\"avro.java.string\":\"String\"}},{\"name\":\"payload\",\"type\":{\"type\":\"string\",\"avro.java.string\":\"String\"}}]}",
+                dereferencedContent);
+    }
+
+    @Test
+    public void testCheckProtobufWithReferences() throws Exception {
+        List<String> artifactReferences = clientV3.groups()
+                .byGroupId(GroupId.DEFAULT.getRawGroupIdWithDefaultString()).artifacts()
+                .byArtifactId("ProtobufSerdeReferencesExample-value").versions().byVersionExpression("1")
+                .references().get().stream().map(ArtifactReference::getArtifactId).toList();
+
+        Assertions.assertTrue(artifactReferences.containsAll(List.of("google/protobuf/timestamp.proto",
+                "sample/table_info.proto", "sample/table_notification_type.proto")));
+        Assertions.assertEquals(3, artifactReferences.size());
+    }
+
+    @Test
+    public void testCheckJsonWithReferences() throws Exception {
+        List<String> artifactReferences = clientV3.groups()
+                .byGroupId(GroupId.DEFAULT.getRawGroupIdWithDefaultString()).artifacts()
+                .byArtifactId("JsonSerdeReferencesExample").versions().byVersionExpression("1").references()
+                .get().stream().map(ArtifactReference::getArtifactId).toList();
+
+        Assertions.assertEquals(4, artifactReferences.size());
+        Assertions.assertTrue(artifactReferences
+                .containsAll(List.of("city", "qualification", "citizenIdentifier", "address")));
+
+        List<ArtifactReference> cityReferences = clientV3.groups()
+                .byGroupId(GroupId.DEFAULT.getRawGroupIdWithDefaultString()).artifacts().byArtifactId("city")
+                .versions().byVersionExpression("1").references().get();
+
+        Assertions.assertEquals(1, cityReferences.size());
+        Assertions.assertTrue(cityReferences.stream().anyMatch(
+                artifactReference -> artifactReference.getArtifactId().equals("cityQualification")));
+
+        List<ArtifactReference> identifierReferences = clientV3.groups()
+                .byGroupId(GroupId.DEFAULT.getRawGroupIdWithDefaultString()).artifacts()
+                .byArtifactId("citizenIdentifier").versions().byVersionExpression("1").references().get();
+
+        Assertions.assertEquals(1, identifierReferences.size());
+        Assertions.assertTrue(identifierReferences.stream().anyMatch(
+                artifactReference -> artifactReference.getArtifactId().equals("identifierQualification")));
+
+        /*
+         * FIXME:carnalca this cannot be asserted until json schema dereferencing is implemented in v3. The
+         * intention here is to make sure that the content can be dereferenced as in Registry v2. String
+         * dereferencedContent = IoUtil.toString(clientV3.groups().byGroupId("default")
+         * .artifacts().byArtifactId("JsonSerdeReferencesExample") .versions().byVersionExpression("1")
+         * .content() .get(configuration -> { configuration.queryParameters.references =
+         * HandleReferencesType.DEREFERENCE; }));
+         */
     }
 }
diff --git a/docs/modules/ROOT/partials/getting-started/ref-registry-all-configs.adoc b/docs/modules/ROOT/partials/getting-started/ref-registry-all-configs.adoc
@@ -384,11 +384,6 @@ The following {registry} configuration options are available for each component
 |
 |`2.1.0.Final`
 |The import URL
-|`apicurio.upgrade.file.location`
-|`optional<url>`
-|
-|`3.0.0`
-|The import URL
 |===
 
 == limits
@@ -831,3 +826,19 @@ The following {registry} configuration options are available for each component
 |Navigation prefix for all UI paths
 |===
 
+== upgrade
+.upgrade configuration options
+[.table-expandable,width="100%",cols="6,3,2,3,5",options="header"]
+|===
+|Name
+|Type
+|Default
+|Available from
+|Description
+|`apicurio.upgrade.file.location`
+|`optional<url>`
+|
+|`3.0.0`
+|The URL to a v2 export file
+|===
+
diff --git a/schema-util/util-provider/src/main/java/io/apicurio/registry/types/provider/AvroArtifactTypeUtilProvider.java b/schema-util/util-provider/src/main/java/io/apicurio/registry/types/provider/AvroArtifactTypeUtilProvider.java
@@ -9,6 +9,7 @@
 import io.apicurio.registry.content.extract.ContentExtractor;
 import io.apicurio.registry.content.refs.JsonSchemaReferenceFinder;
 import io.apicurio.registry.content.refs.ReferenceFinder;
+import io.apicurio.registry.content.util.ContentTypeUtil;
 import io.apicurio.registry.rules.compatibility.AvroCompatibilityChecker;
 import io.apicurio.registry.rules.compatibility.CompatibilityChecker;
 import io.apicurio.registry.rules.validity.AvroContentValidator;
@@ -36,7 +37,10 @@ private static String removeQuotedBrackets(String content) {
     @Override
     public boolean acceptsContent(TypedContent content, Map<String, TypedContent> resolvedReferences) {
         try {
-
+            if (content.getContentType() != null && content.getContentType().toLowerCase().contains("json")
+                    && !ContentTypeUtil.isParsableJson(content.getContent())) {
+                return false;
+            }
             // Avro without quote
             final Schema.Parser parser = new Schema.Parser();
             final List<Schema> schemaRefs = new ArrayList<>();
diff --git a/schema-util/util-provider/src/main/java/io/apicurio/registry/types/provider/JsonArtifactTypeUtilProvider.java b/schema-util/util-provider/src/main/java/io/apicurio/registry/types/provider/JsonArtifactTypeUtilProvider.java
@@ -24,6 +24,10 @@ public class JsonArtifactTypeUtilProvider extends AbstractArtifactTypeUtilProvid
     @Override
     public boolean acceptsContent(TypedContent content, Map<String, TypedContent> resolvedReferences) {
         try {
+            if (content.getContentType() != null && content.getContentType().toLowerCase().contains("json")
+                    && !ContentTypeUtil.isParsableJson(content.getContent())) {
+                return false;
+            }
             JsonNode tree = ContentTypeUtil.parseJson(content.getContent());
             if (tree.has("$schema") && tree.get("$schema").asText().contains("json-schema.org")
                     || tree.has("properties")) {
diff --git a/schema-util/util-provider/src/main/java/io/apicurio/registry/types/provider/ProtobufArtifactTypeUtilProvider.java b/schema-util/util-provider/src/main/java/io/apicurio/registry/types/provider/ProtobufArtifactTypeUtilProvider.java
@@ -25,6 +25,10 @@ public class ProtobufArtifactTypeUtilProvider extends AbstractArtifactTypeUtilPr
     @Override
     public boolean acceptsContent(TypedContent content, Map<String, TypedContent> resolvedReferences) {
         try {
+            String contentType = content.getContentType();
+            if (contentType != null && !contentType.toLowerCase().contains("proto")) {
+                return false;
+            }
             ProtobufFile.toProtoFileElement(content.getContent().content());
             return true;
         } catch (Exception e) {
