Skip to content

Commit

Permalink
chore: add ErrorProne (#258)
Browse files Browse the repository at this point in the history
* BROKE: Switch on ErrorProne, and Nullaway, and get to compile-clean (tests still br0ked)

* FAIL: WIP - testCompile passes, but some tests fail

* Build green

* Ready for merge?
  • Loading branch information
kittylyst authored Sep 24, 2024
1 parent 3b5101b commit c742557
Show file tree
Hide file tree
Showing 35 changed files with 312 additions and 151 deletions.
5 changes: 5 additions & 0 deletions api/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
<artifactId>wildfly-elytron-x500-cert</artifactId>
</dependency>

<dependency>
<groupId>org.jspecify</groupId>
<artifactId>jspecify</artifactId>
</dependency>

<!-- Test dependencies -->

<dependency>
Expand Down
3 changes: 3 additions & 0 deletions api/src/main/java/com/redhat/insights/Filtering.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
import java.util.HashMap;
import java.util.Map;
import java.util.function.Function;
import org.jspecify.annotations.NullMarked;

/** Insights data filtering function. */
@NullMarked
public enum Filtering implements Function<Map<String, Object>, Map<String, Object>> {
DEFAULT(Utils::defaultMasking),

Expand All @@ -20,6 +22,7 @@ public enum Filtering implements Function<Map<String, Object>, Map<String, Objec
this.mask = mask;
}

@Override
public Map<String, Object> apply(Map<String, Object> unfiltered) {
return mask.apply(unfiltered);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import org.jspecify.annotations.NullMarked;

/** A scheduler based on a single-threaded {@link ScheduledThreadPoolExecutor}. */
@NullMarked
public class InsightsCustomScheduledExecutor extends ScheduledThreadPoolExecutor
implements InsightsScheduler {
private final InsightsLogger logger;
Expand Down
5 changes: 4 additions & 1 deletion api/src/main/java/com/redhat/insights/InsightsErrorCode.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
/* Copyright (C) Red Hat 2022-2023 */
/* Copyright (C) Red Hat 2022-2024 */
package com.redhat.insights;

import org.jspecify.annotations.NullMarked;

/**
* Client internal errors.
*
* @author Emmanuel Hugonnet (c) 2023 Red Hat, Inc.
*/
@NullMarked
public enum InsightsErrorCode {
NONE(0),
OPT_OUT(1),
Expand Down
7 changes: 5 additions & 2 deletions api/src/main/java/com/redhat/insights/InsightsException.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
/* Copyright (C) Red Hat 2023 */
/* Copyright (C) Red Hat 2023-2024 */
package com.redhat.insights;

import static com.redhat.insights.InsightsErrorCode.NONE;

import org.jspecify.annotations.NullMarked;

/**
* General-purpose insights client exception type.
*
* <p>Lower-level exceptions (IO, etc.) shall be wrapped or attached to an {@link
* InsightsException}.
*/
@NullMarked
public final class InsightsException extends RuntimeException {

private final InsightsErrorCode error;
Expand Down Expand Up @@ -42,6 +45,6 @@ public InsightsException(InsightsErrorCode error, Throwable cause) {

@Override
public String getMessage() {
return error.formatMessage(super.getMessage());
return error.formatMessage(String.valueOf(super.getMessage()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.security.NoSuchAlgorithmException;
import java.util.concurrent.*;
import java.util.function.Supplier;
import org.jspecify.annotations.NullMarked;

/**
* The controller class has primarily responsibility for managing the upload of {@code CONNECT} and
Expand All @@ -24,6 +25,7 @@
* <p>Client code must explicitly manage the lifecycle of the controller object, and shut it down at
* application exit.
*/
@NullMarked
public final class InsightsReportController {

private final InsightsLogger logger;
Expand Down
4 changes: 3 additions & 1 deletion api/src/main/java/com/redhat/insights/InsightsScheduler.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
/* Copyright (C) Red Hat 2023 */
/* Copyright (C) Red Hat 2023-2024 */
package com.redhat.insights;

import java.util.List;
import java.util.concurrent.ScheduledFuture;
import org.jspecify.annotations.NullMarked;

/** Interface for scheduling {@code CONNECT} and {@code UPDATE} events. */
@NullMarked
public interface InsightsScheduler {

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
/* Copyright (C) Red Hat 2023 */
/* Copyright (C) Red Hat 2023-2024 */
package com.redhat.insights.config;

import org.jspecify.annotations.NullMarked;

/**
* Base configuration that uses the default methods from {@link InsightsConfiguration}, and where
* only {@link InsightsConfiguration#getIdentificationName()} needs to be overridden.
*/
@NullMarked
public abstract class DefaultInsightsConfiguration implements InsightsConfiguration {}
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@
import com.redhat.insights.InsightsException;
import java.time.Duration;
import java.util.Optional;
import org.jspecify.annotations.NullMarked;

/**
* Configuration where values from {@link DefaultInsightsConfiguration} can be overridden by
* environment variables and system properties.
*
* <p>Environment variables take priority over system properties.
*/
@NullMarked
public class EnvAndSysPropsInsightsConfiguration extends DefaultInsightsConfiguration {

public static final String ENV_IDENTIFICATION_NAME = "RHT_INSIGHTS_JAVA_IDENTIFICATION_NAME";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@

import java.time.Duration;
import java.util.Optional;
import org.jspecify.annotations.NullMarked;

/**
* Insights client configuration, covering topics such as PEM file locations, endpoint parameters,
* proxing and more.
*/
@NullMarked
public interface InsightsConfiguration {

String DEFAULT_RHEL_CERT_FILE_PATH = "/etc/pki/consumer/cert.pem";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@
import com.redhat.insights.InsightsException;
import com.redhat.insights.config.InsightsConfiguration;
import com.redhat.insights.logging.InsightsLogger;
import org.jspecify.annotations.NullMarked;

/**
* A general-purpose exponential backoff implementation.
*
* <p>It wraps an execution that might throw an exception (see {@link Action}). In this case
* attempts will be retried with the provided parameters (count, initial delay, factor).
*/
@NullMarked
public final class BackoffWrapper {

@FunctionalInterface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import org.jspecify.annotations.NullMarked;

/** A client that writes the insights report to a file. */
@NullMarked
public class InsightsFileWritingClient implements InsightsHttpClient {
private final InsightsLogger logger;
private final InsightsConfiguration config;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* Copyright (C) Red Hat 2023 */
/* Copyright (C) Red Hat 2023-2024 */
package com.redhat.insights.http;

import static com.redhat.insights.InsightsErrorCode.ERROR_GZIP_FILE;
Expand All @@ -8,14 +8,17 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.zip.GZIPOutputStream;
import org.jspecify.annotations.NullMarked;

/**
* The main interface used for delivering Insights data (aka archives) to Red Hat. Note that the
* interface name contains "Http" because the primary (and desired) transport is over HTTPS.
* However, we need to also support a file-based mechanism - but only temporarily - so we don't warp
* the interface name.
*/
@NullMarked
public interface InsightsHttpClient {

public static final String GENERAL_MIME_TYPE =
Expand Down Expand Up @@ -72,7 +75,8 @@ static byte[] gzipReport(final byte[] report) {
gzip.close();
return baos.toByteArray();
} catch (IOException iox) {
throw new InsightsException(ERROR_GZIP_FILE, "Failed to GZIP report: " + report, iox);
throw new InsightsException(
ERROR_GZIP_FILE, "Failed to GZIP report: " + Arrays.toString(report), iox);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* Copyright (C) Red Hat 2023 */
/* Copyright (C) Red Hat 2023-2024 */
package com.redhat.insights.http;

import static com.redhat.insights.InsightsErrorCode.ERROR_CLIENT_FAILED;
Expand All @@ -8,11 +8,13 @@
import com.redhat.insights.reports.InsightsReport;
import java.util.Arrays;
import java.util.List;
import org.jspecify.annotations.NullMarked;

/**
* An implementation of the Insights client interface that can try multiple connection paths. This
* includes the expected primary use case of HTTPS, then File.
*/
@NullMarked
public class InsightsMultiClient implements InsightsHttpClient {
private final InsightsLogger logger;
private final List<InsightsHttpClient> clients;
Expand Down
31 changes: 15 additions & 16 deletions api/src/main/java/com/redhat/insights/jars/JarAnalyzer.java
Original file line number Diff line number Diff line change
Expand Up @@ -138,21 +138,20 @@ private JarInfo getJarInfo(String jarFilename, URL url, Map<String, String> attr
try {
getExtraAttributes(jarInputStream, attributes);

Map<String, String> pom = getPom(jarInputStream);
Optional<Map<String, String>> oPom = getPom(jarInputStream);

// if we find exactly one pom, use it
if (pom != null) {
if (oPom.isPresent()) {
Map<String, String> pom = oPom.get();
attributes.putAll(pom);
return new JarInfo(jarFilename, pom.get("version"), attributes);
return new JarInfo(jarFilename, pom.getOrDefault("version", UNKNOWN_VERSION), attributes);
}
} catch (Exception ex) {
logger.error(url + "Exception getting extra attributes or pom", ex);
}

String version = getVersion(jarInputStream);
if (version == null || "".equals(version)) {
version = UNKNOWN_VERSION;
}
Optional<String> oVersion = getVersion(jarInputStream);
String version = oVersion.orElse(UNKNOWN_VERSION);

return new JarInfo(jarFilename, version, attributes);
}
Expand All @@ -163,26 +162,26 @@ private JarInfo getJarInfo(String jarFilename, URL url, Map<String, String> attr
* are found, return null.
*/
@SuppressWarnings({"unchecked", "rawtypes"})
private static Map<String, String> getPom(JarInputStream jarFile) throws IOException {
Map<String, String> pom = null;
private static Optional<Map<String, String>> getPom(JarInputStream jarFile) throws IOException {
Optional<Map<String, String>> out = Optional.empty();

for (JarEntry entry = jarFile.getNextJarEntry();
entry != null;
entry = jarFile.getNextJarEntry()) {
if (entry.getName().startsWith("META-INF/maven")
&& entry.getName().endsWith("pom.properties")) {
if (pom != null) {
if (out.isPresent()) {
// we've found multiple pom files. bail!
return null;
return Optional.empty();
}
Properties props = new Properties();
props.load(jarFile);

pom = (Map) props;
out = Optional.of((Map) props);
}
}

return pom;
return out;
}

static void getExtraAttributes(JarInputStream jarFile, Map<String, String> map) {
Expand All @@ -200,13 +199,13 @@ static void getExtraAttributes(JarInputStream jarFile, Map<String, String> map)
}
}

static String getVersion(JarInputStream jarFile) {
static Optional<String> getVersion(JarInputStream jarFile) {
Manifest manifest = jarFile.getManifest();
if (manifest == null) {
return null;
return Optional.empty();
}

return JarUtils.getVersionFromManifest(manifest);
return Optional.of(JarUtils.getVersionFromManifest(manifest));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.jspecify.annotations.Nullable;

/**
* Base class that collects basic information and delegates to the subreport for product-specific
Expand Down Expand Up @@ -84,7 +85,7 @@ public abstract class AbstractTopLevelReportBase implements InsightsReport {
private final JsonSerializer<InsightsReport> serializer;
private final InsightsLogger logger;
private final InsightsConfiguration config;
private byte[] subReport;
private byte @Nullable [] subReport;

// Can't be set properly until after report has been generated
private String idHash = "";
Expand Down
8 changes: 6 additions & 2 deletions api/src/main/java/com/redhat/insights/reports/Utils.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/* Copyright (C) Red Hat 2023-2024 */
package com.redhat.insights.reports;

import static java.util.Collections.emptyMap;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand All @@ -11,11 +13,13 @@ private Utils() {}
@SuppressWarnings("unchecked")
public static Map<String, Object> defaultMasking(final Map<String, Object> inArgs) {
List<String> jvmArgs = new ArrayList<>();
for (String arg : (List<String>) (inArgs.get("jvm.args"))) {
List<String> args = (List<String>) (inArgs.getOrDefault("jvm.args", emptyMap()));
for (String arg : args) {
jvmArgs.add(sanitizeJavaParameters(arg));
}
inArgs.put("jvm.args", jvmArgs);
inArgs.put("java.command", sanitizeJavaParameters((String) inArgs.get("java.command")));
String command = (String) inArgs.getOrDefault("java.command", "");
inArgs.put("java.command", sanitizeJavaParameters(command));
return inArgs;
}

Expand Down
Loading

0 comments on commit c742557

Please sign in to comment.