diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/auth/NoopCredentialFactory.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/auth/NoopCredentialFactory.java index 5035c6e9bcc4..48817cf21e47 100644 --- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/auth/NoopCredentialFactory.java +++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/auth/NoopCredentialFactory.java @@ -23,14 +23,12 @@ import java.util.List; import java.util.Map; import org.apache.beam.sdk.options.PipelineOptions; +import org.checkerframework.checker.nullness.qual.Nullable; /** * Construct an oauth credential to be used by the SDK and the SDK workers. Always returns a null * Credential object. */ -@SuppressWarnings({ - "nullness" // TODO(https://github.com/apache/beam/issues/20497) -}) public class NoopCredentialFactory implements CredentialFactory { private static final NoopCredentialFactory INSTANCE = new NoopCredentialFactory(); private static final NoopCredentials NOOP_CREDENTIALS = new NoopCredentials(); @@ -45,13 +43,18 @@ public Credentials getCredential() throws IOException { } private static class NoopCredentials extends Credentials { + // google-auth Credentials is not annotated for nullness; these overrides intentionally + // return null (this is a no-op credential), which its API permits. @Override - public String getAuthenticationType() { + @SuppressWarnings("nullness") + public @Nullable String getAuthenticationType() { return null; } @Override - public Map> getRequestMetadata(URI uri) throws IOException { + @SuppressWarnings("nullness") + public @Nullable Map> getRequestMetadata(@Nullable URI uri) + throws IOException { return null; } diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GcpOptions.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GcpOptions.java index 0b3be3096159..1d65d1c24264 100644 --- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GcpOptions.java +++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GcpOptions.java @@ -17,6 +17,7 @@ */ package org.apache.beam.sdk.extensions.gcp.options; +import static org.apache.beam.sdk.util.Preconditions.checkArgumentNotNull; import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkArgument; import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Strings.isNullOrEmpty; @@ -42,7 +43,6 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Objects; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.beam.sdk.extensions.gcp.auth.CredentialFactory; @@ -80,9 +80,6 @@ * mechanisms for creating credentials. */ @Description("Options used to configure Google Cloud Platform project and credentials.") -@SuppressWarnings({ - "nullness" // TODO(https://github.com/apache/beam/issues/20497) -}) public interface GcpOptions extends GoogleApiDebugOptions, PipelineOptions { /** Project id to use when launching jobs. */ @Description( @@ -248,11 +245,11 @@ public List create(PipelineOptions options) { * Attempts to infer the default project based upon the environment this application is executing * within. Currently this only supports getting the active project from gcloud. */ - class DefaultProjectFactory implements DefaultValueFactory { + class DefaultProjectFactory implements DefaultValueFactory<@Nullable String> { private static final Logger LOG = LoggerFactory.getLogger(DefaultProjectFactory.class); @Override - public String create(PipelineOptions options) { + public @Nullable String create(PipelineOptions options) { try { File configFile; if (getEnvironment().containsKey("CLOUDSDK_CONFIG")) { @@ -293,7 +290,7 @@ public String create(PipelineOptions options) { } else if (section == null || "core".equals(section)) { matcher = projectPattern.matcher(line); if (matcher.matches()) { - String project = matcher.group(1).trim(); + String project = checkArgumentNotNull(matcher.group(1)).trim(); LOG.info( "Inferred default GCP project '{}' from gcloud. If this is the incorrect " + "project, please cancel this Pipeline and specify the command-line " @@ -326,9 +323,9 @@ Map getEnvironment() { * Attempts to load the GCP credentials. See {@link CredentialFactory#getCredential()} for more * details. */ - class GcpUserCredentialsFactory implements DefaultValueFactory { + class GcpUserCredentialsFactory implements DefaultValueFactory<@Nullable Credentials> { @Override - public Credentials create(PipelineOptions options) { + public @Nullable Credentials create(PipelineOptions options) { GcpOptions gcpOptions = options.as(GcpOptions.class); try { CredentialFactory factory = @@ -365,7 +362,7 @@ public Boolean create(PipelineOptions options) { void setGcpTempLocation(String value); /** Returns {@link PipelineOptions#getTempLocation} as the default GCP temp location. */ - class GcpTempLocationFactory implements DefaultValueFactory { + class GcpTempLocationFactory implements DefaultValueFactory<@Nullable String> { private static final FluentBackoff BACKOFF_FACTORY = FluentBackoff.DEFAULT.withMaxRetries(3).withInitialBackoff(Duration.millis(200)); static final String DEFAULT_REGION = "us-central1"; @@ -415,7 +412,7 @@ static boolean isSoftDeletePolicyEnabled(PipelineOptions options, String tempLoc GcsUtil gcsUtil = gcsOptions.getGcsUtil(); try { SoftDeletePolicy policy = - Objects.requireNonNull(gcsUtil.getBucket(GcsPath.fromUri(tempLocation))) + checkArgumentNotNull(gcsUtil.getBucket(GcsPath.fromUri(tempLocation))) .getSoftDeletePolicy(); if (policy != null && policy.getRetentionDurationSeconds() > 0) { return true; diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GoogleApiDebugOptions.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GoogleApiDebugOptions.java index 1577b9a9d089..348e59d2b79d 100644 --- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GoogleApiDebugOptions.java +++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GoogleApiDebugOptions.java @@ -17,6 +17,8 @@ */ package org.apache.beam.sdk.extensions.gcp.options; +import static org.apache.beam.sdk.util.Preconditions.checkArgumentNotNull; + import com.google.api.client.googleapis.services.AbstractGoogleClient; import com.google.api.client.googleapis.services.AbstractGoogleClientRequest; import com.google.api.client.googleapis.services.GoogleClientRequestInitializer; @@ -25,13 +27,11 @@ import java.util.Map; import org.apache.beam.sdk.options.Description; import org.apache.beam.sdk.options.PipelineOptions; +import org.checkerframework.checker.nullness.qual.Nullable; /** * These options configure debug settings for Google API clients created within the Apache Beam SDK. */ -@SuppressWarnings({ - "nullness" // TODO(https://github.com/apache/beam/issues/20497) -}) public interface GoogleApiDebugOptions extends PipelineOptions { /** * This option enables tracing of API calls to Google services used within the Apache Beam SDK. @@ -67,7 +67,7 @@ class GoogleApiTracer extends HashMap implements GoogleClientReq * given client type. */ public GoogleApiTracer addTraceFor(AbstractGoogleClient client, String traceDestination) { - put(client.getClass().getCanonicalName(), traceDestination); + put(checkArgumentNotNull(client.getClass().getCanonicalName()), traceDestination); return this; } @@ -77,14 +77,15 @@ public GoogleApiTracer addTraceFor(AbstractGoogleClient client, String traceDest */ public GoogleApiTracer addTraceFor( AbstractGoogleClientRequest request, String traceDestination) { - put(request.getClass().getCanonicalName(), traceDestination); + put(checkArgumentNotNull(request.getClass().getCanonicalName()), traceDestination); return this; } @Override public void initialize(AbstractGoogleClientRequest request) throws IOException { + @Nullable String requestClassName = request.getClass().getCanonicalName(); for (Map.Entry entry : this.entrySet()) { - if (request.getClass().getCanonicalName().contains(entry.getKey())) { + if (requestClassName != null && requestClassName.contains(entry.getKey())) { request.set("$trace", entry.getValue()); } } diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystem.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystem.java index 5eca4e9e2c2f..319ff4ce8028 100644 --- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystem.java +++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystem.java @@ -18,6 +18,8 @@ package org.apache.beam.sdk.extensions.gcp.storage; import static org.apache.beam.sdk.io.FileSystemUtils.wildcardToRegexp; +import static org.apache.beam.sdk.util.Preconditions.checkArgumentNotNull; +import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull; import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.MoreObjects.firstNonNull; import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkArgument; import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkNotNull; @@ -69,9 +71,6 @@ * href="https://github.com/apache/beam/blob/master/sdks/java/extensions/google-cloud-platform-core/OWNERS"> * here. */ -@SuppressWarnings({ - "nullness" // TODO(https://github.com/apache/beam/issues/20497) -}) class GcsFileSystem extends FileSystem { private static final Logger LOG = LoggerFactory.getLogger(GcsFileSystem.class); @@ -183,7 +182,7 @@ protected WritableByteChannel create(GcsResourceId resourceId, CreateOptions cre if (createOptions instanceof GcsCreateOptions) { builder = builder.setUploadBufferSizeBytes( - ((GcsCreateOptions) createOptions).gcsUploadBufferSizeBytes()); + checkArgumentNotNull(((GcsCreateOptions) createOptions).gcsUploadBufferSizeBytes())); } Stopwatch stopwatch = Stopwatch.createStarted(); try { @@ -376,8 +375,9 @@ private MatchResult toMatchResult(StorageObjectOrIOException objectOrException) } else if (exception != null) { return MatchResult.create(Status.ERROR, exception); } else { - StorageObject object = objectOrException.storageObject(); - assert object != null; // fix a warning; guaranteed by StorageObjectOrIOException semantics. + // Guaranteed non-null by StorageObjectOrIOException semantics: exactly one of + // storageObject/ioException is set, and ioException was null above. + StorageObject object = checkStateNotNull(objectOrException.storageObject()); return MatchResult.create(Status.OK, ImmutableList.of(toMetadata(object))); } } diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsResourceId.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsResourceId.java index 845822774673..eac46f4d3f29 100644 --- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsResourceId.java +++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsResourceId.java @@ -17,6 +17,7 @@ */ package org.apache.beam.sdk.extensions.gcp.storage; +import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull; import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkArgument; import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkNotNull; import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkState; @@ -72,10 +73,9 @@ public GcsResourceId getCurrentDirectory() { if (isDirectory()) { return this; } else { - GcsPath parent = gcsPath.getParent(); - checkState( - parent != null, - String.format("Failed to get the current directory for path: [%s].", gcsPath)); + GcsPath parent = + checkStateNotNull( + gcsPath.getParent(), "Failed to get the current directory for path: [%s].", gcsPath); return fromGcsPath(parent); } } diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/CustomHttpErrors.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/CustomHttpErrors.java index a70ed3a253b7..beb5d889956b 100644 --- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/CustomHttpErrors.java +++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/CustomHttpErrors.java @@ -21,6 +21,7 @@ import java.io.Serializable; import java.util.ArrayList; import java.util.List; +import org.checkerframework.checker.nullness.qual.Nullable; /** * An optional component to use with the {@code RetryHttpRequestInitializer} in order to provide @@ -63,9 +64,6 @@ * *

*/ -@SuppressWarnings({ - "nullness" // TODO(https://github.com/apache/beam/issues/20497) -}) public class CustomHttpErrors { /** @@ -133,7 +131,7 @@ private CustomHttpErrors(List matchersAndLogs) { } /** Returns the first custom error for the failing request and response to match, or null. */ - public String getCustomError(HttpRequestWrapper req, HttpResponseWrapper res) { + public @Nullable String getCustomError(HttpRequestWrapper req, HttpResponseWrapper res) { for (MatcherAndError m : matchersAndLogs) { if (m.getMatcher().matchResponse(req, res)) { return m.getCustomError().customError(req, res); diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcpSecret.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcpSecret.java index 083298305997..0822dda31274 100644 --- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcpSecret.java +++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcpSecret.java @@ -108,6 +108,8 @@ static String resolveGcpProjectId(@Nullable String projectId, @Nullable String c try { Class clazz = Class.forName("com.google.cloud.ServiceOptions"); java.lang.reflect.Method method = clazz.getMethod("getDefaultProjectId"); + // getDefaultProjectId is static, so passing a null receiver to Method.invoke is correct; + // the checker's stub for Method.invoke conservatively requires a non-null receiver. @SuppressWarnings("nullness") Object result = method.invoke(null); if (result != null && !Strings.isNullOrEmpty(result.toString())) { diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV1.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV1.java index a04f688ec7a3..51f211f36b51 100644 --- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV1.java +++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV1.java @@ -19,6 +19,7 @@ import static org.apache.beam.sdk.io.FileSystemUtils.wildcardToRegexp; import static org.apache.beam.sdk.options.ExperimentalOptions.hasExperiment; +import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull; import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkArgument; import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkNotNull; @@ -110,9 +111,6 @@ import org.slf4j.LoggerFactory; /** Provides operations on GCS. */ -@SuppressWarnings({ - "nullness" // TODO(https://github.com/apache/beam/issues/20497) -}) class GcsUtilV1 { /** Describes which GCS counters this {@link GcsUtilV1} emits. */ @@ -290,8 +288,12 @@ public boolean shouldRetry(IOException e) { } googleCloudStorageOptions = optionsBuilder.build(); try { - googleCloudStorage = + // createGoogleCloudStorage is an overridable instance method; the checker flags calling it on + // a still-initializing receiver, but every field it reads is already assigned above. + @SuppressWarnings("nullness") + GoogleCloudStorage gcs = createGoogleCloudStorage(googleCloudStorageOptions, storageClient, credentials); + googleCloudStorage = gcs; } catch (IOException e) { throw new RuntimeException(e); } @@ -301,6 +303,10 @@ public boolean shouldRetry(IOException e) { // are used. GcsUtilV1 util = this; return new BatchInterface() { + // The supplier reads storageClient/httpRequestInitializer through the captured + // instance when invoked, well after construction completes; the checker conservatively + // treats the captured receiver as still-initializing. + @SuppressWarnings("nullness") final BatchRequest batch = util.storageClient.batch(util.httpRequestInitializer); @Override @@ -477,7 +483,9 @@ public Objects listObjects( Storage.Objects.List listObject = storageClient.objects().list(bucket); listObject.setMaxResults(MAX_LIST_ITEMS_PER_CALL); listObject.setPrefix(prefix); - listObject.setDelimiter(delimiter); + if (delimiter != null) { + listObject.setDelimiter(delimiter); + } if (pageToken != null) { listObject.setPageToken(pageToken); @@ -509,10 +517,13 @@ List fileSizes(List paths) throws IOException { private Long toFileSize(StorageObjectOrIOException storageObjectOrIOException) throws IOException { - if (storageObjectOrIOException.ioException() != null) { - throw storageObjectOrIOException.ioException(); + IOException ioException = storageObjectOrIOException.ioException(); + if (ioException != null) { + throw ioException; } else { - return storageObjectOrIOException.storageObject().getSize().longValue(); + // Exactly one of storageObject/ioException is set; ioException was null above. + StorageObject storageObject = checkStateNotNull(storageObjectOrIOException.storageObject()); + return checkStateNotNull(storageObject.getSize()).longValue(); } } @@ -788,8 +799,9 @@ public WritableByteChannel create(GcsPath path, CreateOptions options) throws IO options.getExpectFileToNotExist() ? 0L : StorageResourceId.UNKNOWN_GENERATION_ID); CreateObjectOptions.Builder createBuilder = CreateObjectOptions.builder().setOverwriteExisting(true); - if (options.getContentType() != null) { - createBuilder = createBuilder.setContentType(options.getContentType()); + @Nullable String contentType = options.getContentType(); + if (contentType != null) { + createBuilder = createBuilder.setContentType(contentType); } HashMap baseLabels = new HashMap<>(); @@ -868,7 +880,9 @@ public boolean bucketAccessible(GcsPath path) throws IOException { * exist, an exception will be thrown. */ public long bucketOwner(GcsPath path) throws IOException { - return getBucket(path, createBackOff(), Sleeper.DEFAULT).getProjectNumber().longValue(); + // getBucket throws (rather than returning null) when the bucket is missing or inaccessible. + Bucket bucket = checkStateNotNull(getBucket(path, createBackOff(), Sleeper.DEFAULT)); + return checkStateNotNull(bucket.getProjectNumber()).longValue(); } /** @@ -880,8 +894,7 @@ public void createBucket(String projectId, Bucket bucket) throws IOException { } /** Get the {@link Bucket} from Cloud Storage path or propagates an exception. */ - @Nullable - public Bucket getBucket(GcsPath path) throws IOException { + public @Nullable Bucket getBucket(GcsPath path) throws IOException { return getBucket(path, createBackOff(), Sleeper.DEFAULT); } @@ -913,6 +926,27 @@ void verifyBucketAccessible(GcsPath path, BackOff backoff, Sleeper sleeper) thro getBucket(path, backoff, sleeper); } + // The JDK constructor stubs for these exceptions aren't annotated for nullness, but a null + // "other" file argument is permitted. Concentrating the suppression in these helpers. + @SuppressWarnings("nullness") + private static AccessDeniedException createAccessDeniedException( + String file, @Nullable String reason) { + return new AccessDeniedException(file, null, reason); + } + + @SuppressWarnings("nullness") + private static FileAlreadyExistsException createFileAlreadyExistsException( + String file, @Nullable String reason) { + return new FileAlreadyExistsException(file, null, reason); + } + + // FileNotFoundException's message argument accepts null, but the JDK constructor stub isn't + // annotated for nullness. + @SuppressWarnings("nullness") + private static FileNotFoundException createFileNotFoundException(@Nullable String message) { + return new FileNotFoundException(message); + } + @VisibleForTesting @Nullable Bucket getBucket(GcsPath path, BackOff backoff, Sleeper sleeper) throws IOException { Storage.Buckets.Get getBucket = storageClient.buckets().get(path.getBucket()); @@ -934,10 +968,10 @@ public boolean shouldRetry(IOException e) { sleeper); } catch (GoogleJsonResponseException e) { if (errorExtractor.accessDenied(e)) { - throw new AccessDeniedException(path.toString(), null, e.getMessage()); + throw createAccessDeniedException(path.toString(), e.getMessage()); } if (errorExtractor.itemNotFound(e)) { - throw new FileNotFoundException(e.getMessage()); + throw createFileNotFoundException(e.getMessage()); } throw e; } catch (InterruptedException e) { @@ -974,10 +1008,10 @@ public boolean shouldRetry(IOException e) { return; } catch (GoogleJsonResponseException e) { if (errorExtractor.accessDenied(e)) { - throw new AccessDeniedException(bucket.getName(), null, e.getMessage()); + throw createAccessDeniedException(bucket.getName(), e.getMessage()); } if (errorExtractor.itemAlreadyExists(e)) { - throw new FileAlreadyExistsException(bucket.getName(), null, e.getMessage()); + throw createFileAlreadyExistsException(bucket.getName(), e.getMessage()); } throw e; } catch (InterruptedException e) { @@ -1011,10 +1045,10 @@ public boolean shouldRetry(IOException e) { sleeper); } catch (GoogleJsonResponseException e) { if (errorExtractor.accessDenied(e)) { - throw new AccessDeniedException(bucket.getName(), null, e.getMessage()); + throw createAccessDeniedException(bucket.getName(), e.getMessage()); } if (errorExtractor.itemNotFound(e)) { - throw new FileNotFoundException(e.getMessage()); + throw createFileNotFoundException(e.getMessage()); } throw e; } catch (InterruptedException e) { @@ -1177,6 +1211,9 @@ public void onFailure(GoogleJsonError e, HttpHeaders responseHeaders) }); } + // Storage.Objects.rewrite permits a null content body (no object-metadata change); the + // generated client stub is not annotated for nullness. + @SuppressWarnings("nullness") public RewriteOp(GcsPath from, GcsPath to, boolean deleteSource, boolean ignoreMissingSource) throws IOException { this.from = from; @@ -1237,8 +1274,10 @@ public void onFailure(GoogleJsonError e, HttpHeaders responseHeaders) throws IOE && e.getErrors().size() == 1 && e.getErrors().get(0).getReason().equals("retentionPolicyNotMet")) { List srcAndDestObjects = getObjects(Arrays.asList(from, to)); - String srcHash = srcAndDestObjects.get(0).storageObject().getMd5Hash(); - String destHash = srcAndDestObjects.get(1).storageObject().getMd5Hash(); + @Nullable String srcHash = + checkStateNotNull(srcAndDestObjects.get(0).storageObject()).getMd5Hash(); + @Nullable String destHash = + checkStateNotNull(srcAndDestObjects.get(1).storageObject()).getMd5Hash(); if (srcHash != null && srcHash.equals(destHash)) { // Source and destination are identical. Treat this as a successful rewrite LOG.warn( diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV2.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV2.java index 9119dd79652e..bbdab0982b5f 100644 --- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV2.java +++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV2.java @@ -100,10 +100,10 @@ public GcsUtilV2 create(PipelineOptions options) { uploadBufferSizeBytes = options.as(GcsOptions.class).getGcsUploadBufferSizeBytes(); } - @SuppressWarnings({ - "nullness" // For Creating AccessDeniedException FileNotFoundException, and - // FileAlreadyExistsException with null. - }) + // AccessDeniedException/FileAlreadyExistsException permit a null "other" argument, and these + // exceptions permit a null detail message, but the JDK constructor stubs are not annotated for + // nullness. + @SuppressWarnings("nullness") private static IOException translateStorageException(GcsPath gcsPath, StorageException e) { switch (e.getCode()) { case 403: diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/RetryHttpRequestInitializer.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/RetryHttpRequestInitializer.java index e3d4eb6ee363..13462db2e3a7 100644 --- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/RetryHttpRequestInitializer.java +++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/RetryHttpRequestInitializer.java @@ -45,9 +45,6 @@ * *

Also can take an HttpResponseInterceptor to be applied to the responses. */ -@SuppressWarnings({ - "nullness" // TODO(https://github.com/apache/beam/issues/20497) -}) public class RetryHttpRequestInitializer implements HttpRequestInitializer { private static final Logger LOG = LoggerFactory.getLogger(RetryHttpRequestInitializer.class); @@ -198,9 +195,10 @@ private boolean retryOnStatusCode(int statusCode) { } } - private final HttpResponseInterceptor responseInterceptor; // response Interceptor to use + private final @Nullable HttpResponseInterceptor + responseInterceptor; // response Interceptor to use - private CustomHttpErrors customHttpErrors = null; + private @Nullable CustomHttpErrors customHttpErrors = null; private final NanoClock nanoClock; // used for testing @@ -208,7 +206,7 @@ private boolean retryOnStatusCode(int statusCode) { private Set ignoredResponseCodes = new HashSet<>(DEFAULT_IGNORED_RESPONSE_CODES); - private Map httpHeaders = null; + private @Nullable Map httpHeaders = null; public RetryHttpRequestInitializer() { this(Collections.emptyList()); @@ -242,7 +240,7 @@ public RetryHttpRequestInitializer( NanoClock nanoClock, Sleeper sleeper, Collection additionalIgnoredResponseCodes, - HttpResponseInterceptor responseInterceptor) { + @Nullable HttpResponseInterceptor responseInterceptor) { this.nanoClock = nanoClock; this.sleeper = sleeper; this.ignoredResponseCodes.addAll(additionalIgnoredResponseCodes); @@ -273,8 +271,9 @@ public void initialize(HttpRequest request) throws IOException { request.setUnsuccessfulResponseHandler(loggingHttpBackOffHandler); request.setIOExceptionHandler(loggingHttpBackOffHandler); - if (this.httpHeaders != null) { - request.getHeaders().putAll(this.httpHeaders); + @Nullable Map localHttpHeaders = this.httpHeaders; + if (localHttpHeaders != null) { + request.getHeaders().putAll(localHttpHeaders); } // Set response initializer diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/UploadIdResponseInterceptor.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/UploadIdResponseInterceptor.java index 8610b7f5f76b..7baad4a60e6b 100644 --- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/UploadIdResponseInterceptor.java +++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/UploadIdResponseInterceptor.java @@ -29,9 +29,6 @@ * is the first request (does not have upload_id parameter in the request). Only logs if debug level * is enabled. */ -@SuppressWarnings({ - "nullness" // TODO(https://github.com/apache/beam/issues/20497) -}) public class UploadIdResponseInterceptor implements HttpResponseInterceptor { private static final Logger LOG = LoggerFactory.getLogger(UploadIdResponseInterceptor.class); diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/gcsfs/GcsPath.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/gcsfs/GcsPath.java index e242bd136b1a..67a78c88f165 100644 --- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/gcsfs/GcsPath.java +++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/gcsfs/GcsPath.java @@ -17,6 +17,8 @@ */ package org.apache.beam.sdk.extensions.gcp.util.gcsfs; +import static org.apache.beam.sdk.util.Preconditions.checkArgumentNotNull; +import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull; import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkArgument; import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Strings.isNullOrEmpty; @@ -65,9 +67,6 @@ * @see Java Tutorials: * Path Operations */ -@SuppressWarnings({ - "nullness" // TODO(https://github.com/apache/beam/issues/20497) -}) public class GcsPath implements Path, Serializable { public static final String SCHEME = "gs"; @@ -118,7 +117,10 @@ public static GcsPath fromUri(String uri) { Matcher m = GCS_URI.matcher(uri); checkArgument(m.matches(), "Invalid GCS URI: %s", uri); - checkArgument(m.group("SCHEME").equalsIgnoreCase(SCHEME), "URI: %s is not a GCS URI", uri); + checkArgument( + checkArgumentNotNull(m.group("SCHEME")).equalsIgnoreCase(SCHEME), + "URI: %s is not a GCS URI", + uri); return new GcsPath(null, m.group("BUCKET"), m.group("OBJECT")); } @@ -233,7 +235,10 @@ public void setFileSystem(FileSystem fs) { } @Override - public FileSystem getFileSystem() { + // fs is null for paths not attached to a filesystem. @Nullable documents the contract; the + // suppression covers overriding Path.getFileSystem, which the checker treats as @NonNull. + @SuppressWarnings("nullness") + public @Nullable FileSystem getFileSystem() { return fs; } @@ -264,7 +269,7 @@ public GcsPath getFileName() { *

Returns a path that ends in '/', as the parent path always refers to a directory. */ @Override - public GcsPath getParent() { + public @Nullable GcsPath getParent() { if (bucket.isEmpty() && object.isEmpty()) { // The root path has no parent, by definition. return null; @@ -341,7 +346,8 @@ public GcsPath subpath(int beginIndex, int endIndex) { ++beginIndex; } - return path; + // endIndex > beginIndex is checked above, so the loop runs at least once and path is set. + return checkStateNotNull(path); } @Override @@ -403,7 +409,10 @@ public GcsPath resolve(String other) { if (other.startsWith(SCHEME + "://")) { GcsPath path = GcsPath.fromUri(other); - path.setFileSystem(getFileSystem()); + @Nullable FileSystem currentFs = getFileSystem(); + if (currentFs != null) { + path.setFileSystem(currentFs); + } return path; } @@ -473,11 +482,11 @@ public Iterator iterator() { } private static class NameIterator implements Iterator { - private final FileSystem fs; + private final @Nullable FileSystem fs; private boolean fullPath; - private String name; + private @Nullable String name; - NameIterator(FileSystem fs, boolean fullPath, String name) { + NameIterator(@Nullable FileSystem fs, boolean fullPath, String name) { this.fs = fs; this.fullPath = fullPath; this.name = name; @@ -490,13 +499,14 @@ public boolean hasNext() { @Override public GcsPath next() { - int i = name.indexOf('/'); + String currentName = checkStateNotNull(name); + int i = currentName.indexOf('/'); String component; if (i >= 0) { - component = name.substring(0, i); - name = name.substring(i + 1); + component = currentName.substring(0, i); + name = currentName.substring(i + 1); } else { - component = name; + component = currentName; name = null; } if (fullPath) { @@ -593,6 +603,8 @@ public String toResourceName() { } @Override + // URI permits a null fragment; the JDK URI constructor stub is not annotated for nullness. + @SuppressWarnings("nullness") public URI toUri() { try { return new URI(SCHEME, "//" + bucketAndObject(), null); @@ -613,7 +625,7 @@ private String bucketAndObject() { public static String getNonWildcardPrefix(String globExp) { Matcher m = GLOB_PREFIX.matcher(globExp); checkArgument(m.matches(), String.format("Glob expression: [%s] is not expandable.", globExp)); - return m.group("PREFIX"); + return checkArgumentNotNull(m.group("PREFIX")); } /** Returns true if the given {@code spec} contains wildcard. */