Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -152,24 +152,37 @@ public String toJson() throws IOException {
}

/**
* Applies defaults for association creation or upsert.
* For CREATE: schema implies frozen STRONG; frozen=false or WEAK with schema are rejected.
* For UPSERT: schema implies non-frozen STRONG; frozen and lifecycle are only defaulted if null.
* Applies defaults for an association that does not yet exist. Only call this while creating
* the association: an upsert that updates an existing one keeps that association's stored
* lifecycle and would be wrongly rejected here for carrying a schema.
*
* <p>For a create op a schema implies frozen STRONG, and frozen=false or WEAK alongside a
* schema are rejected. For an upsert op no schema is accepted at all, since a schema implies
* a topic-owned STRONG association and those can only be created together with the topic.
* Either way frozen and lifecycle are defaulted when null.
*
* <p>The matching check on an explicit STRONG lifecycle lives in the registry, which can
* exempt subjects in IMPORT mode.
*
* @param isCreate whether this is a create op rather than an upsert op
*/
public void applyDefaults(boolean isCreate) {
if (!isCreate && getSchema() != null) {
throw new IllegalPropertyException(
"schema", "cannot be provided when adding an association to an existing topic; "
+ "an association with a schema must be created with the topic");
}
if (getSchema() != null) {
if (getLifecycle() == LifecyclePolicy.WEAK) {
throw new IllegalPropertyException(
"lifecycle", "cannot be WEAK when schema is provided");
}
if (getFrozen() != null && getFrozen() != isCreate) {
if (Boolean.FALSE.equals(getFrozen())) {
throw new IllegalPropertyException(
"frozen", isCreate
? "cannot be false when schema is provided for create"
: "cannot be true when creating via upsert; use create instead");
"frozen", "cannot be false when a schema is provided");
}
setLifecycle(LifecyclePolicy.STRONG);
setFrozen(isCreate);
setFrozen(true);
} else {
if (getFrozen() == null) {
setFrozen(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,24 +143,37 @@ public int hashCode() {
}

/**
* Applies defaults for association creation or upsert.
* For CREATE: schema implies frozen STRONG; frozen=false or WEAK with schema are rejected.
* For UPSERT: schema implies non-frozen STRONG; frozen and lifecycle are only defaulted if null.
* Applies defaults for an association that does not yet exist. Only call this while creating
* the association: an upsert that updates an existing one keeps that association's stored
* lifecycle and would be wrongly rejected here for carrying a schema.
*
* <p>For a create op a schema implies frozen STRONG, and frozen=false or WEAK alongside a
* schema are rejected. For an upsert op no schema is accepted at all, since a schema implies
* a topic-owned STRONG association and those can only be created together with the topic.
* Either way frozen and lifecycle are defaulted when null.
*
* <p>The matching check on an explicit STRONG lifecycle lives in the registry, which can
* exempt subjects in IMPORT mode.
*
* @param isCreate whether this is a create op rather than an upsert op
*/
public void applyDefaults(boolean isCreate) {
if (!isCreate && getSchema() != null) {
throw new IllegalPropertyException(
"schema", "cannot be provided when adding an association to an existing topic; "
+ "an association with a schema must be created with the topic");
}
if (getSchema() != null) {
if (getLifecycle() == LifecyclePolicy.WEAK) {
throw new IllegalPropertyException(
"lifecycle", "cannot be WEAK when schema is provided");
}
if (getFrozen() != null && getFrozen() != isCreate) {
if (Boolean.FALSE.equals(getFrozen())) {
throw new IllegalPropertyException(
"frozen", isCreate
? "cannot be false when schema is provided for create"
: "cannot be true when creating via upsert; use create instead");
"frozen", "cannot be false when a schema is provided");
}
setLifecycle(LifecyclePolicy.STRONG);
setFrozen(isCreate);
setFrozen(true);
} else {
if (getFrozen() == null) {
setFrozen(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.io.IOException;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;

@JsonInclude(JsonInclude.Include.NON_EMPTY)
@JsonIgnoreProperties(ignoreUnknown = true)
Expand Down Expand Up @@ -59,12 +60,23 @@ public AssociationCreateOrUpdateRequest(

public AssociationCreateOrUpdateRequest(
AssociationOpRequest request, AssociationOp op) {
this(request, ImmutableList.of((AssociationCreateOrUpdateOp) op));
}

/**
* Builds a single request carrying every op in {@code ops}, so that a batch can apply them
* as a unit. Repeated association types are rejected when the request is applied.
*/
public AssociationCreateOrUpdateRequest(
AssociationOpRequest request, List<? extends AssociationCreateOrUpdateOp> ops) {
this(
request.getResourceName(),
request.getResourceNamespace(),
request.getResourceId(),
request.getResourceType(),
ImmutableList.of(new AssociationCreateOrUpdateInfo((AssociationCreateOrUpdateOp) op))
ops.stream()
.map(AssociationCreateOrUpdateInfo::new)
.collect(Collectors.toList())
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@
import io.confluent.kafka.schemaregistry.client.rest.exceptions.IllegalPropertyException;
import io.confluent.kafka.schemaregistry.utils.JacksonMapper;
import java.io.IOException;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;

@JsonInclude(JsonInclude.Include.NON_EMPTY)
@JsonIgnoreProperties(ignoreUnknown = true)
Expand Down Expand Up @@ -141,10 +143,20 @@
return JacksonMapper.INSTANCE.writeValueAsString(this);
}

private static String associationTypeOf(AssociationOp op) {
if (op instanceof AssociationCreateOrUpdateOp) {
return ((AssociationCreateOrUpdateOp) op).getAssociationType();
}
if (op instanceof AssociationDeleteOp) {
return ((AssociationDeleteOp) op).getAssociationType();
}
return null;
}

// Validates resource fields, then for each op: validates the op, defaults
// the subject to :.ns:name-type for STRONG associations, and enforces that
// frozen associations use the default subject.
public void validate(boolean dryRun) {

Check failure on line 159 in client/src/main/java/io/confluent/kafka/schemaregistry/client/rest/entities/requests/AssociationOpRequest.java

View check run for this annotation

SonarQube-Confluent / SonarQube Code Analysis

Refactor this method to reduce its Cognitive Complexity from 33 to the 15 allowed.

[S3776] Cognitive Complexity of methods should not be too high See more on https://sonarqube.confluent.io/project/issues?id=schema-registry&pullRequest=4460&issues=9d7b787f-0070-4b6e-af7e-7385e0fccf06&open=9d7b787f-0070-4b6e-af7e-7385e0fccf06
checkName(getResourceName(), "resourceName");
checkName(getResourceNamespace(), "resourceNamespace");
if (!dryRun && (getResourceId() == null || getResourceId().isEmpty())) {
Expand All @@ -161,8 +173,16 @@
if (getAssociations() == null || getAssociations().isEmpty()) {
throw new IllegalPropertyException("associations", "cannot be null or empty");
}
Set<String> associationTypes = new HashSet<>();
for (AssociationOp op : getAssociations()) {
op.validate(dryRun);
String associationType = associationTypeOf(op);
if (associationType != null && !associationTypes.add(associationType)) {
throw new IllegalPropertyException(
"associationType",
"may only appear once per request, but '" + associationType + "' appears more"
+ " than once");
}
if (op instanceof AssociationCreateOrUpdateOp) {
AssociationCreateOrUpdateOp createOrUpdateOp = (AssociationCreateOrUpdateOp) op;
if (op instanceof AssociationCreateOp) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
@Test
public void testInfoValidLifecycleDoesNotThrow() {
AssociationCreateOrUpdateInfo info = new AssociationCreateOrUpdateInfo(
"test-subject", null, LifecyclePolicy.STRONG, null, null, null);

Check failure on line 35 in client/src/test/java/io/confluent/kafka/schemaregistry/client/rest/entities/requests/AssociationsRequestTest.java

View check run for this annotation

SonarQube-Confluent / SonarQube Code Analysis

Define a constant instead of duplicating this literal "test-subject" 28 times.

[S1192] String literals should not be duplicated See more on https://sonarqube.confluent.io/project/issues?id=schema-registry&pullRequest=4460&issues=81746df3-b0e5-49a7-a599-72e7e9b867d6&open=81746df3-b0e5-49a7-a599-72e7e9b867d6
info.validate(false, false);
}

Expand Down Expand Up @@ -100,7 +100,7 @@
@Test
public void testCreateOpWithSchemaDefaultsToStrongFrozen() {
RegisterSchemaRequest schema = new RegisterSchemaRequest();
schema.setSchema("{\"type\":\"string\"}");

Check failure on line 103 in client/src/test/java/io/confluent/kafka/schemaregistry/client/rest/entities/requests/AssociationsRequestTest.java

View check run for this annotation

SonarQube-Confluent / SonarQube Code Analysis

Define a constant instead of duplicating this literal "{\"type\":\"string\"}" 30 times.

[S1192] String literals should not be duplicated See more on https://sonarqube.confluent.io/project/issues?id=schema-registry&pullRequest=4460&issues=55c25660-02f1-4cb1-ae67-424a49dfd328&open=55c25660-02f1-4cb1-ae67-424a49dfd328
AssociationCreateOp op = new AssociationCreateOp(
"test-subject", null, null, null, schema, null);
op.validate(false);
Expand Down Expand Up @@ -213,6 +213,105 @@
info.validate(false, false);
}

// An upsert may only create a WEAK association: a STRONG association is owned by its topic
// and has to be created with it, and a schema implies STRONG.

@Test(expected = IllegalPropertyException.class)
public void testUpsertInfoCreatingWithSchemaAndSubjectThrows() {
RegisterSchemaRequest schema = new RegisterSchemaRequest();
schema.setSchema("{\"type\":\"string\"}");
AssociationCreateOrUpdateInfo info = new AssociationCreateOrUpdateInfo(
"test-subject", null, null, null, schema, null);
info.applyDefaults(false);
}

@Test(expected = IllegalPropertyException.class)
public void testUpsertInfoCreatingWithSchemaAndStrongLifecycleThrows() {
RegisterSchemaRequest schema = new RegisterSchemaRequest();
schema.setSchema("{\"type\":\"string\"}");
AssociationCreateOrUpdateInfo info = new AssociationCreateOrUpdateInfo(
"test-subject", null, LifecyclePolicy.STRONG, null, schema, null);
info.applyDefaults(false);
}

@Test(expected = IllegalPropertyException.class)
public void testUpsertInfoCreatingWithSchemaAndNoSubjectThrows() {
RegisterSchemaRequest schema = new RegisterSchemaRequest();
schema.setSchema("{\"type\":\"string\"}");
AssociationCreateOrUpdateInfo info = new AssociationCreateOrUpdateInfo(
null, null, null, null, schema, null);
info.applyDefaults(false);
}

/**
* An explicit STRONG lifecycle without a schema is left alone here: the registry rejects it,
* so that subjects in IMPORT mode can be exempted.
*/
@Test
public void testUpsertInfoCreatingWithStrongLifecycleAndNoSchemaIsLeftToRegistry() {
AssociationCreateOrUpdateInfo info = new AssociationCreateOrUpdateInfo(
"test-subject", null, LifecyclePolicy.STRONG, null, null, null);
info.applyDefaults(false);
assertEquals(LifecyclePolicy.STRONG, info.getLifecycle());
assertEquals(Boolean.FALSE, info.getFrozen());
}

@Test
public void testUpsertInfoCreatingFrozenStrongKeepsFrozenForImport() {
AssociationCreateOrUpdateInfo info = new AssociationCreateOrUpdateInfo(
"test-subject", null, LifecyclePolicy.STRONG, true, null, null);
info.applyDefaults(false);
assertEquals(LifecyclePolicy.STRONG, info.getLifecycle());
assertEquals(Boolean.TRUE, info.getFrozen());
}

@Test
public void testUpsertInfoCreatingWeakDoesNotThrow() {
AssociationCreateOrUpdateInfo info = new AssociationCreateOrUpdateInfo(
"test-subject", null, null, null, null, null);
info.applyDefaults(false);
assertEquals(LifecyclePolicy.WEAK, info.getLifecycle());
assertEquals(Boolean.FALSE, info.getFrozen());
}

@Test
public void testCreateInfoWithSchemaAndSubjectStillDefaultsToStrongFrozen() {
RegisterSchemaRequest schema = new RegisterSchemaRequest();
schema.setSchema("{\"type\":\"string\"}");
AssociationCreateOrUpdateInfo info = new AssociationCreateOrUpdateInfo(
"test-subject", null, null, null, schema, null);
info.applyDefaults(true);
assertEquals(LifecyclePolicy.STRONG, info.getLifecycle());
assertEquals(Boolean.TRUE, info.getFrozen());
}

@Test(expected = IllegalPropertyException.class)
public void testUpsertOpCreatingWithSchemaAndSubjectThrows() {
RegisterSchemaRequest schema = new RegisterSchemaRequest();
schema.setSchema("{\"type\":\"string\"}");
AssociationUpsertOp op = new AssociationUpsertOp(
"test-subject", null, null, null, schema, null);
op.applyDefaults(false);
}

@Test(expected = IllegalPropertyException.class)
public void testUpsertOpCreatingWithSchemaAndNoSubjectThrows() {
RegisterSchemaRequest schema = new RegisterSchemaRequest();
schema.setSchema("{\"type\":\"string\"}");
AssociationUpsertOp op = new AssociationUpsertOp(
null, null, null, null, schema, null);
op.applyDefaults(false);
}

@Test
public void testUpsertOpCreatingWithStrongLifecycleAndNoSchemaIsLeftToRegistry() {
AssociationUpsertOp op = new AssociationUpsertOp(
"test-subject", null, LifecyclePolicy.STRONG, null, null, null);
op.applyDefaults(false);
assertEquals(LifecyclePolicy.STRONG, op.getLifecycle());
assertEquals(Boolean.FALSE, op.getFrozen());
}

// Requirement #3: Default subject + subject required

@Test
Expand Down
Loading
Loading