Skip to content

Checkstyle code-safety cleanup: avoid parameter reassignment and null-side equals #1039

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
akshayrai merged 1 commit into from
Jun 29, 2026
Merged

Checkstyle code-safety cleanup: avoid parameter reassignment and null-side equals #1039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 8 additions & 8 deletions datastream-client/src/main/java/com/linkedin/datastream/BaseRestClientFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ public synchronized T getClient(String uri, Map<String, Object> httpConfig, Prop
* @param restClient custom R2 RestClient
*/
public synchronized void registerRestClient(String uri, RestClient restClient) {
uri = RestliUtils.sanitizeUri(uri);
_restClients.put(getKey(uri, Collections.emptyMap()), restClient);
String sanitizedUri = RestliUtils.sanitizeUri(uri);
_restClients.put(getKey(sanitizedUri, Collections.emptyMap()), restClient);
}

/**
Expand All @@ -124,16 +124,16 @@ public synchronized void registerRestClient(String uri, RestClient restClient) {
*/
public synchronized void addOverride(String uri, T restliClient) {
Validate.notNull(restliClient, "null RestClient");
uri = RestliUtils.sanitizeUri(uri);
if (_overrides.containsKey(uri)) {
_logger.warn("Replacing existing RestliClient override for URI: " + uri);
String sanitizedUri = RestliUtils.sanitizeUri(uri);
if (_overrides.containsKey(sanitizedUri)) {
_logger.warn("Replacing existing RestliClient override for URI: " + sanitizedUri);
}
_overrides.put(uri, restliClient);
_overrides.put(sanitizedUri, restliClient);
}

private T getOverride(String uri) {
uri = RestliUtils.sanitizeUri(uri);
return _overrides.getOrDefault(uri, null);
String sanitizedUri = RestliUtils.sanitizeUri(uri);
return _overrides.getOrDefault(sanitizedUri, null);
}

private static String getKey(String uri, Map<String, Object> httpConfig) {
Expand Down
6 changes: 2 additions & 4 deletions datastream-common/src/main/java/com/linkedin/datastream/avrogenerator/FieldMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,9 @@ public FieldMetadata(@NotNull String dbFieldName, String nullable, int dbFieldPo
*/
public static Map<String, String> parseMetadata(String meta) {
// cut off the trailing delimiter ";"
if (meta.endsWith(META_LIST_DELIMITER)) {
meta = meta.substring(0, meta.length() - 1);
}
String trimmedMeta = meta.endsWith(META_LIST_DELIMITER) ? meta.substring(0, meta.length() - 1) : meta;

String[] parts = meta.split(META_LIST_DELIMITER);
String[] parts = trimmedMeta.split(META_LIST_DELIMITER);
Map<String, String> metas = new HashMap<>(parts.length);
for (String part : parts) {
String[] keyValuePair = part.split(META_VALUE_DELIMITER);
Expand Down
9 changes: 3 additions & 6 deletions ...tream-common/src/main/java/com/linkedin/datastream/connectors/CommonConnectorMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,7 @@ public void updateRebalanceRate(long val) {
* @return List of exposed Brooklin metrics for the connector activity
*/
public static List<BrooklinMetricInfo> getEventProcessingMetrics(String prefix) {
prefix = Strings.nullToEmpty(prefix);
return EventProcMetrics.getMetricInfos(prefix);
return EventProcMetrics.getMetricInfos(Strings.nullToEmpty(prefix));
}

/**
Expand All @@ -502,8 +501,7 @@ public static List<BrooklinMetricInfo> getEventProcessingMetrics(String prefix)
* @return List of exposed Brooklin metrics for the connector activity
*/
public static List<BrooklinMetricInfo> getEventPollMetrics(String prefix) {
prefix = Strings.nullToEmpty(prefix);
return PollMetrics.getMetricInfos(prefix);
return PollMetrics.getMetricInfos(Strings.nullToEmpty(prefix));
}

/**
Expand All @@ -512,7 +510,6 @@ public static List<BrooklinMetricInfo> getEventPollMetrics(String prefix) {
* @return List of exposed Brooklin metrics for the connector activity
*/
public static List<BrooklinMetricInfo> getPartitionSpecificMetrics(String prefix) {
prefix = Strings.nullToEmpty(prefix);
return PartitionMetrics.getMetricInfos(prefix);
return PartitionMetrics.getMetricInfos(Strings.nullToEmpty(prefix));
}
}
7 changes: 4 additions & 3 deletions ...c/main/java/com/linkedin/datastream/connectors/kafka/AbstractKafkaBasedConnectorTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,11 +351,12 @@ protected void rewindAndPausePartitionsOnSendException() {
}

private boolean containsTransientException(Throwable ex) {
while (ex instanceof DatastreamRuntimeException) {
if (ex instanceof DatastreamTransientException) {
Throwable cause = ex;
while (cause instanceof DatastreamRuntimeException) {
if (cause instanceof DatastreamTransientException) {
return true;
}
ex = ex.getCause();
cause = cause.getCause();
}
return false;
}
Expand Down
24 changes: 12 additions & 12 deletions ...rc/main/java/com/linkedin/datastream/connectors/kafka/KafkaBasedConnectorTaskMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,19 +324,19 @@ public void updatePerEventProcessingTimeNanos(long val) {
*/
public static List<BrooklinMetricInfo> getKafkaBasedConnectorTaskSpecificMetrics(String prefix) {
List<BrooklinMetricInfo> metrics = new ArrayList<>();
prefix = Strings.nullToEmpty(prefix);
String safePrefix = Strings.nullToEmpty(prefix);
// Specify the attributes to expose to the final metric registry.
metrics.add(new BrooklinGaugeInfo(prefix + NUM_CONFIG_PAUSED_PARTITIONS));
metrics.add(new BrooklinGaugeInfo(prefix + NUM_AUTO_PAUSED_PARTITIONS_ON_ERROR));
metrics.add(new BrooklinGaugeInfo(prefix + NUM_AUTO_PAUSED_PARTITIONS_ON_INFLIGHT_MESSAGES));
metrics.add(new BrooklinGaugeInfo(prefix + NUM_AUTO_PAUSED_PARTITIONS_WAITING_FOR_DEST_TOPIC));
metrics.add(new BrooklinGaugeInfo(prefix + NUM_AUTO_PAUSED_PARTITIONS_WAITING_SOURCE_TOPIC_ACCESS));
metrics.add(new BrooklinGaugeInfo(prefix + NUM_TOPICS));
metrics.add(new BrooklinGaugeInfo(prefix + CONSUMER_OFFSET_WATERMARK_SPAN));
metrics.add(new BrooklinGaugeInfo(prefix + CONSUMER_LICLOSEST_DATA_LOSS_ESTIMATION));
metrics.add(new BrooklinHistogramInfo(prefix + POLL_DURATION_MS));
metrics.add(new BrooklinHistogramInfo(prefix + TIME_SPENT_BETWEEN_POLLS_MS));
metrics.add(new BrooklinHistogramInfo(prefix + PER_EVENT_PROCESSING_TIME_NANOS));
metrics.add(new BrooklinGaugeInfo(safePrefix + NUM_CONFIG_PAUSED_PARTITIONS));
metrics.add(new BrooklinGaugeInfo(safePrefix + NUM_AUTO_PAUSED_PARTITIONS_ON_ERROR));
metrics.add(new BrooklinGaugeInfo(safePrefix + NUM_AUTO_PAUSED_PARTITIONS_ON_INFLIGHT_MESSAGES));
metrics.add(new BrooklinGaugeInfo(safePrefix + NUM_AUTO_PAUSED_PARTITIONS_WAITING_FOR_DEST_TOPIC));
metrics.add(new BrooklinGaugeInfo(safePrefix + NUM_AUTO_PAUSED_PARTITIONS_WAITING_SOURCE_TOPIC_ACCESS));
metrics.add(new BrooklinGaugeInfo(safePrefix + NUM_TOPICS));
metrics.add(new BrooklinGaugeInfo(safePrefix + CONSUMER_OFFSET_WATERMARK_SPAN));
metrics.add(new BrooklinGaugeInfo(safePrefix + CONSUMER_LICLOSEST_DATA_LOSS_ESTIMATION));
metrics.add(new BrooklinHistogramInfo(safePrefix + POLL_DURATION_MS));
metrics.add(new BrooklinHistogramInfo(safePrefix + TIME_SPENT_BETWEEN_POLLS_MS));
metrics.add(new BrooklinHistogramInfo(safePrefix + PER_EVENT_PROCESSING_TIME_NANOS));
return Collections.unmodifiableList(metrics);
}
}
5 changes: 3 additions & 2 deletions datastream-kafka/src/main/java/com/linkedin/datastream/kafka/KafkaDestination.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,15 @@ public static KafkaDestination parse(String uri) {
Validate.isTrue(uri.startsWith(SCHEME_KAFKA) || uri.startsWith(SCHEME_SECURE_KAFKA),
"Invalid scheme in URI: " + uri);

String decodedUri;
try {
// Decode URI in case it's escaped
uri = URIUtil.decode(uri);
decodedUri = URIUtil.decode(uri);
} catch (Exception e) {
throw new DatastreamRuntimeException("Failed to decode Kafka destination URI: " + uri, e);
}

URI u = URI.create(uri);
URI u = URI.create(decodedUri);
String scheme = u.getScheme();
String zkAddress = u.getAuthority();
String path = u.getPath();
Expand Down
6 changes: 3 additions & 3 deletions ...ver-api/src/main/java/com/linkedin/datastream/server/DatastreamProducerRecordBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ public void setSourceCheckpoint(String sourceCheckpoint) {
* Add the event with key and value to the DatastreamProducerRecord. Datastream producer record can have multiple events.
*/
public void addEvent(Object key, Object value, Object previousValue, Map<String, String> metadata) {
key = key == null ? new byte[0] : key;
value = value == null ? new byte[0] : value;
_events.add(new BrooklinEnvelope(key, value, previousValue, metadata));
Object eventKey = key == null ? new byte[0] : key;
Object eventValue = value == null ? new byte[0] : value;
_events.add(new BrooklinEnvelope(eventKey, eventValue, previousValue, metadata));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion datastream-server/src/main/java/com/linkedin/datastream/server/DatastreamTaskImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public String toJson() throws IOException {

@JsonIgnore
public String getDatastreamTaskName() {
return _id.equals("") ? _taskPrefix : _taskPrefix + "_" + _id;
return "".equals(_id) ? _taskPrefix : _taskPrefix + "_" + _id;

@dhananjay-sawner dhananjay-sawner Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: Should we use StringUtils.isEmpty() for better readability

@akshayrai akshayrai Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@dhananjay-sawner I'd rather do that in a separate PR. Since this is a wide scanning PR, I want to keep it simple and just address checkstyle issue here.

@dhananjay-sawner dhananjay-sawner Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sure, looks good

}

@JsonIgnore
Expand Down
2 changes: 1 addition & 1 deletion datastream-testcommon/src/main/java/com/linkedin/datastream/connectors/BrokenConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class BrokenConnector implements Connector, DiagnosticsAware {
public BrokenConnector(Properties properties) throws Exception {
_properties = properties;
String dummyConfigValue = _properties.getProperty("dummyProperty", "");
if (!dummyConfigValue.equals("dummyValue")) {
if (!"dummyValue".equals(dummyConfigValue)) {
throw new Exception("Invalid config value for dummyProperty. Expected: dummyValue");
}
}
Expand Down
2 changes: 1 addition & 1 deletion datastream-testcommon/src/main/java/com/linkedin/datastream/connectors/DummyConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class DummyConnector implements Connector, DiagnosticsAware {
public DummyConnector(Properties properties) throws Exception {
_properties = properties;
String dummyConfigValue = _properties.getProperty("dummyProperty", "");
if (!dummyConfigValue.equals("dummyValue")) {
if (!"dummyValue".equals(dummyConfigValue)) {
throw new Exception("Invalid config value for dummyProperty. Expected: dummyValue");
}
}
Expand Down
15 changes: 8 additions & 7 deletions datastream-utils/src/main/java/com/linkedin/datastream/common/RestliUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ public final class RestliUtils {
* a {@code "/"} appended to it unless it already has one.
*/
public static String sanitizeUri(String uri) {
uri = uri.toLowerCase();
if (!uri.startsWith(URI_SCHEME) && !uri.startsWith(URI_SCHEME_SSL)) {
String sanitized = uri.toLowerCase();
if (!sanitized.startsWith(URI_SCHEME) && !sanitized.startsWith(URI_SCHEME_SSL)) {
// use plaintext by default
uri = URI_SCHEME + uri;
sanitized = URI_SCHEME + sanitized;
}
return StringUtils.appendIfMissing(uri, "/");
return StringUtils.appendIfMissing(sanitized, "/");
}

/**
Expand All @@ -41,14 +41,15 @@ public static String sanitizeUri(String uri) {
* @param <T> type of elements in the Stream
*/
public static <T> Stream<T> withPaging(Stream<T> stream, final PagingContext pagingContext) {
Stream<T> paged = stream;
if (pagingContext.hasStart()) {
stream = stream.skip(pagingContext.getStart());
paged = paged.skip(pagingContext.getStart());
}

if (pagingContext.hasCount()) {
stream = stream.limit(pagingContext.getCount());
paged = paged.limit(pagingContext.getCount());
}

return stream;
return paged;
}
}
11 changes: 6 additions & 5 deletions datastream-utils/src/main/java/com/linkedin/datastream/common/zk/ZkClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,12 @@ public void ensurePath(String path) {
Stack<String> pstack = new Stack<>();

// push paths in stack because we need to create from parent to children
while (!exists(path)) {
pstack.push(path);
path = path.substring(0, path.lastIndexOf(ZK_PATH_SEPARATOR));
if (path.isEmpty()) {
path = ZK_PATH_SEPARATOR;
String current = path;
while (!exists(current)) {
pstack.push(current);
current = current.substring(0, current.lastIndexOf(ZK_PATH_SEPARATOR));
if (current.isEmpty()) {
current = ZK_PATH_SEPARATOR;
}
}

Expand Down
Loading