orpiske commented on code in PR #9339:
URL: https://github.com/apache/camel/pull/9339#discussion_r1104496662
##########
components/camel-kafka/src/main/java/org/apache/camel/processor/resume/kafka/KafkaResumeStrategyConfiguration.java:
##########
@@ -37,29 +38,23 @@ public Properties getProducerProperties() {
}
void setProducerProperties(Properties producerProperties) {
- assert producerProperties != null;
-
- this.producerProperties = producerProperties;
+ this.producerProperties = ObjectHelper.notNull(producerProperties,
"producerProperties");
Review Comment:
Assert is the correct one: this not user-facing public code.
##########
components/camel-aws/camel-aws2-kinesis/src/main/java/org/apache/camel/component/aws2/kinesis/consumer/KinesisDefaultResumeAdapter.java:
##########
@@ -46,8 +47,8 @@ public void setRequestBuilder(GetShardIteratorRequest.Builder
resumable) {
@Override
public void resume() {
- assert streamName != null;
- assert resumable != null;
Review Comment:
Using Java's assert is the correct away to assert for nullity in non-public
code as is the case here (obs.: the code is internally public, but not public
in the to Camel users).
##########
components/camel-wal/src/main/java/org/apache/camel/component/wal/LogWriter.java:
##########
@@ -239,7 +240,7 @@ public void updateState(EntryInfo.CachedEntryInfo
entryInfo, LogEntry.EntryState
Trying to update a persisted entry here is not acceptable
*/
- assert layerInfo != null;
+ ObjectHelper.notNull(layerInfo, "layerInfo");
Review Comment:
This is also not correct and also impacts the performance of the log writer
as the assert is native and would not be enabled in production code (whereas
the ObjectHelper will incur the cost of `notNull` check on a critical section
of this code).
##########
components/camel-kafka/src/main/java/org/apache/camel/processor/resume/kafka/KafkaResumeStrategyConfiguration.java:
##########
@@ -37,29 +38,23 @@ public Properties getProducerProperties() {
}
void setProducerProperties(Properties producerProperties) {
- assert producerProperties != null;
-
- this.producerProperties = producerProperties;
+ this.producerProperties = ObjectHelper.notNull(producerProperties,
"producerProperties");
}
public Properties getConsumerProperties() {
return consumerProperties;
}
void setConsumerProperties(Properties consumerProperties) {
- assert consumerProperties != null;
-
- this.consumerProperties = consumerProperties;
+ this.consumerProperties = ObjectHelper.notNull(consumerProperties,
"consumerProperties");
}
public String getTopic() {
return topic;
}
void setTopic(String topic) {
- assert topic != null;
-
- this.topic = topic;
Review Comment:
Same as above.
##########
test-infra/camel-test-infra-core/src/test/java/org/apache/camel/test/infra/core/api/CamelTestSupportHelper.java:
##########
@@ -30,7 +31,7 @@ public interface CamelTestSupportHelper {
default <T extends Endpoint> T getMandatoryEndpoint(String uri, Class<T>
type) {
T endpoint = getCamelContextExtension().getContext().getEndpoint(uri,
type);
- assert endpoint != null : "No endpoint found for uri: " + uri;
Review Comment:
Assert is OK here.
##########
components/camel-kafka/src/main/java/org/apache/camel/processor/resume/kafka/KafkaResumeStrategyConfiguration.java:
##########
@@ -37,29 +38,23 @@ public Properties getProducerProperties() {
}
void setProducerProperties(Properties producerProperties) {
- assert producerProperties != null;
-
- this.producerProperties = producerProperties;
+ this.producerProperties = ObjectHelper.notNull(producerProperties,
"producerProperties");
}
public Properties getConsumerProperties() {
return consumerProperties;
}
void setConsumerProperties(Properties consumerProperties) {
- assert consumerProperties != null;
-
- this.consumerProperties = consumerProperties;
Review Comment:
Same as above
##########
core/camel-support/src/main/java/org/apache/camel/support/resume/AdapterHelper.java:
##########
@@ -41,9 +42,9 @@ private AdapterHelper() {
}
public static ResumeAdapter eval(CamelContext context, ResumeAware
resumeAware, ResumeStrategy resumeStrategy) {
- assert context != null;
- assert resumeAware != null;
- assert resumeStrategy != null;
+ ObjectHelper.notNull(context, "context");
+ ObjectHelper.notNull(resumeAware, "resumeAware");
+ ObjectHelper.notNull(resumeStrategy, "resumeStrategy");
Review Comment:
Assert is the correct one here as this is an internal interface where the
nullity condition should not happen in any circumstance.
##########
components/camel-caffeine/src/main/java/org/apache/camel/component/caffeine/resume/CaffeineCache.java:
##########
@@ -57,7 +58,7 @@ public CaffeineCache(Cache<K, Object> cache, long cacheSize) {
@Override
public boolean contains(K key, Object entry) {
- assert key != null;
+ ObjectHelper.notNull(key, "key");
Review Comment:
Assert is the correct check here, as this is set internally by the resume
code and it should not be null. The assert only adds a layer of check when
running in tests as surefire/failsafe will enable native Java assertions by
default in those cases.
##########
test-infra/camel-test-infra-core/src/test/java/org/apache/camel/test/infra/core/DefaultAnnotationProcessor.java:
##########
@@ -151,13 +152,12 @@ private void doInvokeFixture(Annotation annotation, Field
field, Object instance
if (field.getType() == MockEndpoint.class) {
final MockEndpoint mockEndpoint =
contextExtension.getMockEndpoint(uri);
- assert mockEndpoint != null;
-
+ ObjectHelper.notNull(mockEndpoint, "mockEndpoint");
field.set(instance, mockEndpoint);
} else {
final Endpoint endpoint = context.getEndpoint(uri);
- assert endpoint != null;
+ ObjectHelper.notNull(endpoint, "endpoint");
Review Comment:
A bit torn on this one, as the assert is missing a message saying what's
going on.
--
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-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
