Skip to content

DGS-20986 - Modify POST /subjects lookup to fail only if both normalize=true fails #3841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Nitss10 merged 8 commits into from
Aug 5, 2025
Merged

DGS-20986 - Modify POST /subjects lookup to fail only if both normalize=true fails #3841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fb1284e
lookup normalize change logic
Nitss10 Aug 4, 2025
560c7e4
Modified POST /subjects lookup to fail only if normalize true fails
Nitss10 Aug 5, 2025
276a629
Test case for the lookup API with retry logic
Nitss10 Aug 5, 2025
8d9a2bc
Refactoring logs
Nitss10 Aug 5, 2025
91d0eaf
Merge remote-tracking branch 'origin/8.0.x' into lookup_normalization
Nitss10 Aug 5, 2025
023c2c2
Refactoring checkstyle
Nitss10 Aug 5, 2025
12e614d
Removed try catch
Nitss10 Aug 5, 2025
f7736e7
Improved identation
Nitss10 Aug 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion core/src/main/java/io/confluent/kafka/schemaregistry/rest/resources/SubjectsResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ public void lookUpSchemaUnderSubject(
subject, lookupDeletedSchema, request.getSchemaType());

subject = QualifiedSubject.normalize(schemaRegistry.tenant(), subject);

// returns version if the schema exists. Otherwise returns 404
Schema schema = new Schema(subject, request);
io.confluent.kafka.schemaregistry.client.rest.entities.Schema matchingSchema;
Expand All @@ -131,6 +130,15 @@ public void lookUpSchemaUnderSubject(
}
matchingSchema = schemaRegistry.lookUpSchemaUnderSubjectUsingContexts(
subject, schema, normalize, lookupDeletedSchema);

// If first attempt failed with normalize=false, try again with normalize=true
if (matchingSchema == null && !normalize) {
log.debug("No matching schema found with normalize = false,"
+ " retrying with normalize = true");
matchingSchema = schemaRegistry.lookUpSchemaUnderSubjectUsingContexts(
subject, schema, true, lookupDeletedSchema);
}

if (matchingSchema == null) {
if (!schemaRegistry.hasSubjects(subject, lookupDeletedSchema)) {
throw Errors.subjectNotFoundException(subject);
Expand Down
33 changes: 33 additions & 0 deletions core/src/test/java/io/confluent/kafka/schemaregistry/rest/RestApiTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@

@Test
public void testRegisterBadDefault() throws Exception {
String subject = "testSubject";

Check failure on line 245 in core/src/test/java/io/confluent/kafka/schemaregistry/rest/RestApiTest.java

View check run for this annotation

SonarQube-Confluent / schema-registry Sonarqube Results

core/src/test/java/io/confluent/kafka/schemaregistry/rest/RestApiTest.java#L245 

Define a constant instead of duplicating this literal "testSubject" 21 times.

String schemaString = "{\"type\":\"record\","
+ "\"name\":\"myrecord\","
Expand Down Expand Up @@ -1424,6 +1424,39 @@
assertTrue(!messages.isEmpty() && messages.get(0).contains("Invalid schema"));
}

@Test
public void testLookUpSchemaWithNormalizationRetry() throws Exception {
String subject = "testSubject";

String schemaString1 = "{\"type\":\"record\",\"name\":\"User\",\"fields\":[{\"name\":\"id\",\"type\":\"int\"},{\"name\":\"email4\",\"type\":\"string\"}]}";

// Register the original schema
TestUtils.registerAndVerifySchema(restApp.restClient, schemaString1, 1, subject);

// Same schema with different field ordering (semantically equivalent)
String schemaString2 = "{\"type\":\"record\",\"name\":\"User\",\"fields\":[{\"type\":\"int\",\"name\":\"id\"},{\"type\":\"string\",\"name\":\"email4\"}]}";

RegisterSchemaRequest request = new RegisterSchemaRequest();
request.setSchema(schemaString2);

io.confluent.kafka.schemaregistry.client.rest.entities.Schema schema =
Copy link
Member

@karthikeyanas karthikeyanas Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a test case where semantically same schema with normalize=true fails lookup as we are expected not to retry in this case.

Copy link
Member Author

@Nitss10 Nitss10 Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But how will semantically same schema with normalize=true fail the lookup? Isnt that what normalization means?

Copy link
Member

@karthikeyanas karthikeyanas Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's right. I was looking for the test cases

  1. Semantically same schema but with added spaces/ordering succeeds with normalize=false
  2. Semantically same schema but with added spaces/ordering succeeds with normalize=true
  3. Semantically different schema fails with normalize=false
  4. Semantically different schema fails with normalize=true

I think 2 & 4 are already covered by existing cases, we are good with this.

Copy link
Member

@DhirajSuri27 DhirajSuri27 Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semantically same schema but with added spaces/ordering succeeds with normalize=false

Nit: Shouldn't this fail if the ordering is different and normalize=false ?

restApp.restClient.lookUpSubjectVersion(request, subject, false, false);
assertNotNull(schema);
assertEquals(1, schema.getVersion().intValue());

// Different schema with different field name (not semantically equivalent)
String invalidSchema = "{\"type\":\"record\",\"name\":\"User\",\"fields\":[{\"type\":\"int\",\"name\":\"id\"},{\"type\":\"string\",\"name\":\"email6\"}]}";
request.setSchema(invalidSchema);

// Should fail since schemas are actually different
try {
restApp.restClient.lookUpSubjectVersion(request, subject, true, false);
fail("Should fail as schemas are not semantically equivalent");
} catch (RestClientException e) {
assertEquals(Errors.SCHEMA_NOT_FOUND_ERROR_CODE, e.getErrorCode());
}
}

@Test
public void testBad() throws Exception {
String subject1 = "testTopic1";
Expand Down