DGS-9162: Address review feedback

confluentinc · Dec 8, 2023 · 3bc36b7 · 3bc36b7
1 parent 6b468ab
commit 3bc36b7
Show file tree

Hide file tree

Showing 6 changed files with 10 additions and 32 deletions.
diff --git a/client/src/main/java/io/confluent/kafka/schemaregistry/client/MockSchemaRegistryClient.java b/client/src/main/java/io/confluent/kafka/schemaregistry/client/MockSchemaRegistryClient.java
@@ -63,7 +63,7 @@ public class MockSchemaRegistryClient implements SchemaRegistryClient {
 
   private static final String WILDCARD = "*";
 
-  private Config defaultConfig = new Config("BACKWARD", true);
+  private Config defaultConfig = new Config("BACKWARD");
   private final Map<String, Map<ParsedSchema, Integer>> schemaToIdCache;
   private final Map<String, Map<ParsedSchema, Integer>> registeredSchemaCache;
   private final Map<String, Map<Integer, ParsedSchema>> idToSchemaCache;

diff --git a/client/src/main/java/io/confluent/kafka/schemaregistry/client/rest/entities/Config.java b/client/src/main/java/io/confluent/kafka/schemaregistry/client/rest/entities/Config.java
@@ -66,12 +66,6 @@ public Config(@JsonProperty("compatibilityLevel") String compatibilityLevel) {
     this.compatibilityLevel = compatibilityLevel;
   }
 
-  public Config(@JsonProperty("compatibilityLevel") String compatibilityLevel,
-                @JsonProperty("validateFields") Boolean validateFields) {
-    this.compatibilityLevel = compatibilityLevel;
-    this.validateFields = validateFields;
-  }
-
   public Config() {
   }
 

diff --git a/core/src/main/java/io/confluent/kafka/schemaregistry/rest/resources/ConfigResource.java b/core/src/main/java/io/confluent/kafka/schemaregistry/rest/resources/ConfigResource.java
@@ -235,9 +235,6 @@ public ConfigUpdateRequest updateTopLevelConfig(
         throw new RestInvalidRuleSetException(e.getMessage());
       }
     }
-    if (request.isValidateFields() == null) {
-      request.setValidateFields(schemaRegistry.isDefaultValidateFields());
-    }
     try {
       Map<String, String> headerProperties = requestHeaderBuilder.buildRequestHeaders(
           headers, schemaRegistry.config().whitelistHeaders());

diff --git a/core/src/main/java/io/confluent/kafka/schemaregistry/storage/KafkaSchemaRegistry.java b/core/src/main/java/io/confluent/kafka/schemaregistry/storage/KafkaSchemaRegistry.java
@@ -1994,9 +1994,7 @@ public String getGroupId() {
   public Config getConfig(String subject)
       throws SchemaRegistryStoreException {
     try {
-      return lookupCache.config(subject,
-          false,
-          new Config(defaultCompatibilityLevel.name, defaultValidateFields));
+      return lookupCache.config(subject, false, new Config(defaultCompatibilityLevel.name));
     } catch (StoreException e) {
       throw new SchemaRegistryStoreException("Failed to write new config value to the store", e);
     }
@@ -2005,9 +2003,7 @@ public Config getConfig(String subject)
   public Config getConfigInScope(String subject)
       throws SchemaRegistryStoreException {
     try {
-      return lookupCache.config(subject,
-          true,
-          new Config(defaultCompatibilityLevel.name, defaultValidateFields));
+      return lookupCache.config(subject, true, new Config(defaultCompatibilityLevel.name));
     } catch (StoreException e) {
       throw new SchemaRegistryStoreException("Failed to write new config value to the store", e);
     }
@@ -2339,13 +2335,8 @@ public HostnameVerifier getHostnameVerifier() throws SchemaRegistryStoreExceptio
                     + " not supported");
   }
 
-  public boolean isDefaultValidateFields() {
-    return defaultValidateFields;
-  }
-
-
-  private static boolean isSchemaFieldValidationEnabled(Config config) {
-    return config.isValidateFields() != null ? config.isValidateFields() : false;
+  private boolean isSchemaFieldValidationEnabled(Config config) {
+    return config.isValidateFields() != null ? config.isValidateFields() : defaultValidateFields;
   }
 
   private static class RawSchema {

diff --git a/core/src/main/java/io/confluent/kafka/schemaregistry/storage/LookupCache.java b/core/src/main/java/io/confluent/kafka/schemaregistry/storage/LookupCache.java
@@ -106,17 +106,13 @@ public interface LookupCache<K,V> extends Store<K,V> {
    * @param subject the subject
    * @param returnTopLevelIfNotFound whether to return the top level scope if not found
    * @param defaultForTopLevel default value for the top level scope
-   * @param defaultForValidateFields default value for the top level scope
    * @return the compatibility level if found, otherwise null
    */
   default CompatibilityLevel compatibilityLevel(String subject,
                                                 boolean returnTopLevelIfNotFound,
-                                                CompatibilityLevel defaultForTopLevel,
-                                                boolean defaultForValidateFields)
+                                                CompatibilityLevel defaultForTopLevel)
       throws StoreException {
-    Config config = config(subject,
-        returnTopLevelIfNotFound,
-        new Config(defaultForTopLevel.name, defaultForValidateFields));
+    Config config = config(subject, returnTopLevelIfNotFound, new Config(defaultForTopLevel.name));
     return CompatibilityLevel.forName(config.getCompatibilityLevel());
   }
 

diff --git a/core/src/test/java/io/confluent/kafka/schemaregistry/rest/RestApiCompatibilityTest.java b/core/src/test/java/io/confluent/kafka/schemaregistry/rest/RestApiCompatibilityTest.java
@@ -189,7 +189,7 @@ public void testCompatibilityLevelChangeToBackward() throws Exception {
             restApp.restClient.registerSchema(schemaString1, subject));
     // verify that default compatibility level is backward
     assertEquals("Default compatibility level should be backward",
-            new Config(CompatibilityLevel.BACKWARD.name, false),
+            new Config(CompatibilityLevel.BACKWARD.name),
             restApp.restClient.getConfig(null));
     // change it to forward
     assertEquals("Changing compatibility level should succeed",
@@ -200,7 +200,7 @@ public void testCompatibilityLevelChangeToBackward() throws Exception {
 
     // verify that new compatibility level is forward
     assertEquals("New compatibility level should be forward",
-            new Config(CompatibilityLevel.FORWARD.name, false),
+            new Config(CompatibilityLevel.FORWARD.name),
             restApp.restClient.getConfig(null));
 
     // register schema that is forward compatible with schemaString1
@@ -222,7 +222,7 @@ public void testCompatibilityLevelChangeToBackward() throws Exception {
 
     // verify that new compatibility level is backward
     assertEquals("Updated compatibility level should be backward",
-            new Config(CompatibilityLevel.BACKWARD.name, false),
+            new Config(CompatibilityLevel.BACKWARD.name),
             restApp.restClient.getConfig(null));
 
             // register forward compatible schema, which should fail