Uploaded image for project: 'Kafka'
  1. Kafka
  2. KAFKA-6926

Reduce NPath exceptions in Connect

Details

    • Improvement
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 2.0.0
    • 2.1.0
    • connect
    • None

    Description

      The recent upgrade of Checkstyle and move to Java 8 has caused some existing code to not pass the NPath rule. Look at the classes to see what might need to be changed to remove the class from the suppression rule:

      AbstractStatus|ConnectRecord|ConnectSchema|DistributedHerder|FileStreamSourceTask|JsonConverter|KafkaConfigBackingStore

      Attachments

        Issue Links

          Activity

            githubbot ASF GitHub Bot added a comment -

            rhauch opened a new pull request #5051: KAFKA-6926: Simplified some logic to eliminate some suppressions of NPath complexity checks
            URL: https://github.com/apache/kafka/pull/5051

            Requires #5046 to be merged first. This PR should be rebased once #5046 is merged.

                1. Committer Checklist (excluded from commit message)
            • [ ] Verify design and implementation
            • [ ] Verify test coverage and CI build status
            • [ ] Verify documentation (including upgrade notes)

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

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

            githubbot ASF GitHub Bot added a comment - rhauch opened a new pull request #5051: KAFKA-6926 : Simplified some logic to eliminate some suppressions of NPath complexity checks URL: https://github.com/apache/kafka/pull/5051 Requires #5046 to be merged first. This PR should be rebased once #5046 is merged. Committer Checklist (excluded from commit message) [ ] Verify design and implementation [ ] Verify test coverage and CI build status [ ] Verify documentation (including upgrade notes) ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            ijuma closed pull request #5051: KAFKA-6926: Simplified some logic to eliminate some suppressions of NPath complexity checks
            URL: https://github.com/apache/kafka/pull/5051

            This is a PR merged from a forked repository.
            As GitHub hides the original diff on merge, it is displayed below for
            the sake of provenance:

            As this is a foreign pull request (from a fork), the diff is supplied
            below (as it won't show otherwise due to GitHub magic):

            diff --git a/checkstyle/suppressions.xml b/checkstyle/suppressions.xml
            index ba48c38cb28..cba38f2b172 100644
            — a/checkstyle/suppressions.xml
            +++ b/checkstyle/suppressions.xml
            @@ -114,7 +114,7 @@
            files="Values.java"/>

            <suppress checks="NPathComplexity"

            • files="(AbstractStatus|ConnectHeaders|ConnectRecord|ConnectSchema|DistributedHerder|FileStreamSourceTask|JsonConverter|KafkaConfigBackingStore).java"/>
              + files="(DistributedHerder|JsonConverter|KafkaConfigBackingStore|FileStreamSourceTask).java"/>

            <suppress checks="MethodLength"
            files="Values.java"/>
            diff --git a/connect/api/src/main/java/org/apache/kafka/connect/connector/ConnectRecord.java b/connect/api/src/main/java/org/apache/kafka/connect/connector/ConnectRecord.java
            index 2ad8a046d76..2c5f514836d 100644
            — a/connect/api/src/main/java/org/apache/kafka/connect/connector/ConnectRecord.java
            +++ b/connect/api/src/main/java/org/apache/kafka/connect/connector/ConnectRecord.java
            @@ -155,24 +155,14 @@ public boolean equals(Object o)

            { ConnectRecord that = (ConnectRecord) o; - if (kafkaPartition != null ? !kafkaPartition.equals(that.kafkaPartition) : that.kafkaPartition != null) - return false; - if (topic != null ? !topic.equals(that.topic) : that.topic != null) - return false; - if (keySchema != null ? !keySchema.equals(that.keySchema) : that.keySchema != null) - return false; - if (key != null ? !key.equals(that.key) : that.key != null) - return false; - if (valueSchema != null ? !valueSchema.equals(that.valueSchema) : that.valueSchema != null) - return false; - if (value != null ? !value.equals(that.value) : that.value != null) - return false; - if (timestamp != null ? !timestamp.equals(that.timestamp) : that.timestamp != null) - return false; - if (!Objects.equals(headers, that.headers)) - return false; - - return true; + return Objects.equals(kafkaPartition, that.kafkaPartition) + && Objects.equals(topic, that.topic) + && Objects.equals(keySchema, that.keySchema) + && Objects.equals(key, that.key) + && Objects.equals(valueSchema, that.valueSchema) + && Objects.equals(value, that.value) + && Objects.equals(timestamp, that.timestamp) + && Objects.equals(headers, that.headers); }

            @Override
            diff --git a/connect/api/src/main/java/org/apache/kafka/connect/data/ConnectSchema.java b/connect/api/src/main/java/org/apache/kafka/connect/data/ConnectSchema.java
            index ff8271635f3..a42a33e8af7 100644
            — a/connect/api/src/main/java/org/apache/kafka/connect/data/ConnectSchema.java
            +++ b/connect/api/src/main/java/org/apache/kafka/connect/data/ConnectSchema.java
            @@ -218,14 +218,10 @@ public static void validateValue(String name, Schema schema, Object value)

            { if (!schema.isOptional()) throw new DataException("Invalid value: null used for required field: \"" + name + "\", schema type: " + schema.type()); - else - return; + return; }
            • List<Class> expectedClasses = LOGICAL_TYPE_CLASSES.get(schema.name());
              -
            • if (expectedClasses == null)
            • expectedClasses = SCHEMA_TYPE_CLASSES.get(schema.type());
              + List<Class> expectedClasses = expectedClassesFor(schema);

            if (expectedClasses == null)
            throw new DataException("Invalid Java object for schema type " + schema.type()
            @@ -266,6 +262,13 @@ public static void validateValue(String name, Schema schema, Object value) {
            }
            }

            + private static List<Class> expectedClassesFor(Schema schema)

            { + List<Class> expectedClasses = LOGICAL_TYPE_CLASSES.get(schema.name()); + if (expectedClasses == null) + expectedClasses = SCHEMA_TYPE_CLASSES.get(schema.type()); + return expectedClasses; + }

            +
            /**

            • Validate that the value can be used for this schema, i.e. that its type matches the schema type and optional
            • requirements. Throws a DataException if the value is invalid.
              diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractStatus.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractStatus.java
              index 00a050a310f..dd1b94a854e 100644
                • a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractStatus.java
                  +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractStatus.java
                  @@ -16,6 +16,8 @@
                  */
                  package org.apache.kafka.connect.runtime;

            +import java.util.Objects;
            +
            public abstract class AbstractStatus<T> {

            public enum State {
            @@ -81,12 +83,11 @@ public boolean equals(Object o)

            { AbstractStatus<?> that = (AbstractStatus<?>) o; - if (generation != that.generation) return false; - if (id != null ? !id.equals(that.id) : that.id != null) return false; - if (state != that.state) return false; - if (trace != null ? !trace.equals(that.trace) : that.trace != null) return false; - return workerId != null ? workerId.equals(that.workerId) : that.workerId == null; - + return generation == that.generation + && Objects.equals(id, that.id) + && state == that.state + && Objects.equals(trace, that.trace) + && Objects.equals(workerId, that.workerId); }

            @Override

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

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

            githubbot ASF GitHub Bot added a comment - ijuma closed pull request #5051: KAFKA-6926 : Simplified some logic to eliminate some suppressions of NPath complexity checks URL: https://github.com/apache/kafka/pull/5051 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/checkstyle/suppressions.xml b/checkstyle/suppressions.xml index ba48c38cb28..cba38f2b172 100644 — a/checkstyle/suppressions.xml +++ b/checkstyle/suppressions.xml @@ -114,7 +114,7 @@ files="Values.java"/> <suppress checks="NPathComplexity" files="(AbstractStatus|ConnectHeaders|ConnectRecord|ConnectSchema|DistributedHerder|FileStreamSourceTask|JsonConverter|KafkaConfigBackingStore).java"/> + files="(DistributedHerder|JsonConverter|KafkaConfigBackingStore|FileStreamSourceTask).java"/> <suppress checks="MethodLength" files="Values.java"/> diff --git a/connect/api/src/main/java/org/apache/kafka/connect/connector/ConnectRecord.java b/connect/api/src/main/java/org/apache/kafka/connect/connector/ConnectRecord.java index 2ad8a046d76..2c5f514836d 100644 — a/connect/api/src/main/java/org/apache/kafka/connect/connector/ConnectRecord.java +++ b/connect/api/src/main/java/org/apache/kafka/connect/connector/ConnectRecord.java @@ -155,24 +155,14 @@ public boolean equals(Object o) { ConnectRecord that = (ConnectRecord) o; - if (kafkaPartition != null ? !kafkaPartition.equals(that.kafkaPartition) : that.kafkaPartition != null) - return false; - if (topic != null ? !topic.equals(that.topic) : that.topic != null) - return false; - if (keySchema != null ? !keySchema.equals(that.keySchema) : that.keySchema != null) - return false; - if (key != null ? !key.equals(that.key) : that.key != null) - return false; - if (valueSchema != null ? !valueSchema.equals(that.valueSchema) : that.valueSchema != null) - return false; - if (value != null ? !value.equals(that.value) : that.value != null) - return false; - if (timestamp != null ? !timestamp.equals(that.timestamp) : that.timestamp != null) - return false; - if (!Objects.equals(headers, that.headers)) - return false; - - return true; + return Objects.equals(kafkaPartition, that.kafkaPartition) + && Objects.equals(topic, that.topic) + && Objects.equals(keySchema, that.keySchema) + && Objects.equals(key, that.key) + && Objects.equals(valueSchema, that.valueSchema) + && Objects.equals(value, that.value) + && Objects.equals(timestamp, that.timestamp) + && Objects.equals(headers, that.headers); } @Override diff --git a/connect/api/src/main/java/org/apache/kafka/connect/data/ConnectSchema.java b/connect/api/src/main/java/org/apache/kafka/connect/data/ConnectSchema.java index ff8271635f3..a42a33e8af7 100644 — a/connect/api/src/main/java/org/apache/kafka/connect/data/ConnectSchema.java +++ b/connect/api/src/main/java/org/apache/kafka/connect/data/ConnectSchema.java @@ -218,14 +218,10 @@ public static void validateValue(String name, Schema schema, Object value) { if (!schema.isOptional()) throw new DataException("Invalid value: null used for required field: \"" + name + "\", schema type: " + schema.type()); - else - return; + return; } List<Class> expectedClasses = LOGICAL_TYPE_CLASSES.get(schema.name()); - if (expectedClasses == null) expectedClasses = SCHEMA_TYPE_CLASSES.get(schema.type()); + List<Class> expectedClasses = expectedClassesFor(schema); if (expectedClasses == null) throw new DataException("Invalid Java object for schema type " + schema.type() @@ -266,6 +262,13 @@ public static void validateValue(String name, Schema schema, Object value) { } } + private static List<Class> expectedClassesFor(Schema schema) { + List<Class> expectedClasses = LOGICAL_TYPE_CLASSES.get(schema.name()); + if (expectedClasses == null) + expectedClasses = SCHEMA_TYPE_CLASSES.get(schema.type()); + return expectedClasses; + } + /** Validate that the value can be used for this schema, i.e. that its type matches the schema type and optional requirements. Throws a DataException if the value is invalid. diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractStatus.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractStatus.java index 00a050a310f..dd1b94a854e 100644 a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractStatus.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractStatus.java @@ -16,6 +16,8 @@ */ package org.apache.kafka.connect.runtime; +import java.util.Objects; + public abstract class AbstractStatus<T> { public enum State { @@ -81,12 +83,11 @@ public boolean equals(Object o) { AbstractStatus<?> that = (AbstractStatus<?>) o; - if (generation != that.generation) return false; - if (id != null ? !id.equals(that.id) : that.id != null) return false; - if (state != that.state) return false; - if (trace != null ? !trace.equals(that.trace) : that.trace != null) return false; - return workerId != null ? workerId.equals(that.workerId) : that.workerId == null; - + return generation == that.generation + && Objects.equals(id, that.id) + && state == that.state + && Objects.equals(trace, that.trace) + && Objects.equals(workerId, that.workerId); } @Override ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org

            People

              rhauch Randall Hauch
              rhauch Randall Hauch
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: