[GitHub] [pulsar] BewareMyPower commented on a diff in pull request #15718: [fix][schema] Fix update schema forward compatibility strategy behavior

[GitBox <gi...@apache.org>]

BewareMyPower commented on code in PR #15718:
URL: https://github.com/apache/pulsar/pull/15718#discussion_r885226788


##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleSchemaTest.java:
##########
@@ -1299,4 +1300,41 @@ public void testConsumeAvroMessagesWithoutSchema() throws Exception {
         producer.close();
         consumer.close();
     }
+
+    @Test
+    public void testUpdateSchemaForward() throws Exception {
+        String topic = "my-property/my-ns/test";
+        admin.namespaces().setSchemaCompatibilityStrategy(
+                "my-property/my-ns/",
+                SchemaCompatibilityStrategy.FORWARD);
+        Schema<V3Data> v3Schema = Schema.AVRO(V3Data.class);
+        Schema<V4Data> v4Schema = Schema.AVRO(V4Data.class);
+
+        admin.schemas().createSchema(topic, v3Schema.getSchemaInfo());
+        admin.schemas().createSchema(topic, v4Schema.getSchemaInfo());
+
+        try {
+            admin.schemas().createSchema(topic, v3Schema.getSchemaInfo());
+            fail("Should fail here, since the forward schema compatibility strategy doesn't allow removing the field.");
+        } catch (Exception e) {
+            assertTrue(e.getMessage().contains("org.apache.avro.SchemaValidationException: Unable to read schema"));
+        }
+    }
+
+    @Data
+    @Builder
+    @NoArgsConstructor
+    @AllArgsConstructor
+    static class V4Data {
+        int i;
+        int j;
+    }
+
+    @Data
+    @Builder
+    @NoArgsConstructor
+    @AllArgsConstructor
+    static class V3Data {
+        int i;
+    }

Review Comment:
   We can remove these two classes and reuse `V1Data` and `IncompatibleData` instead.



##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleSchemaTest.java:
##########
@@ -1299,4 +1300,41 @@ public void testConsumeAvroMessagesWithoutSchema() throws Exception {
         producer.close();
         consumer.close();
     }
+
+    @Test
+    public void testUpdateSchemaForward() throws Exception {
+        String topic = "my-property/my-ns/test";
+        admin.namespaces().setSchemaCompatibilityStrategy(
+                "my-property/my-ns/",
+                SchemaCompatibilityStrategy.FORWARD);
+        Schema<V3Data> v3Schema = Schema.AVRO(V3Data.class);
+        Schema<V4Data> v4Schema = Schema.AVRO(V4Data.class);
+
+        admin.schemas().createSchema(topic, v3Schema.getSchemaInfo());
+        admin.schemas().createSchema(topic, v4Schema.getSchemaInfo());
+
+        try {
+            admin.schemas().createSchema(topic, v3Schema.getSchemaInfo());
+            fail("Should fail here, since the forward schema compatibility strategy doesn't allow removing the field.");
+        } catch (Exception e) {

Review Comment:
   ```suggestion
           } catch (PulsarAdminException.ConflictException e) {
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org