From 7d9bbb473da1f206b9642be26ef5d281752b8b58 Mon Sep 17 00:00:00 2001 From: Gabriel Roldan Date: Tue, 14 Oct 2025 10:29:47 -0300 Subject: [PATCH] Improved exception handling for GoogleCloudStorageBlobStore --- .../gcs/GoogleCloudStorageBlobStore.java | 60 +++++++++---- .../gcs/GoogleCloudStorageBlobStoreInfo.java | 24 +++-- .../gcs/GoogleCloudStorageClient.java | 87 +++++++++++-------- .../GoogleCloudStorageConfigProviderTest.java | 2 - 4 files changed, 112 insertions(+), 61 deletions(-) diff --git a/geowebcache/gcsblob/src/main/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageBlobStore.java b/geowebcache/gcsblob/src/main/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageBlobStore.java index bea77deac..4eba498d0 100644 --- a/geowebcache/gcsblob/src/main/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageBlobStore.java +++ b/geowebcache/gcsblob/src/main/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageBlobStore.java @@ -38,6 +38,7 @@ import javax.annotation.Nullable; import org.geotools.util.logging.Logging; import org.geowebcache.GeoWebCacheException; +import org.geowebcache.config.XMLConfiguration; import org.geowebcache.filter.parameters.ParametersUtils; import org.geowebcache.io.ByteArrayResource; import org.geowebcache.io.Resource; @@ -52,6 +53,7 @@ import org.geowebcache.storage.TileObject; import org.geowebcache.storage.TileRange; import org.geowebcache.storage.TileRangeIterator; +import org.geowebcache.storage.UnsuitableStorageException; import org.geowebcache.util.TMSKeyBuilder; /** @@ -72,11 +74,16 @@ public class GoogleCloudStorageBlobStore implements BlobStore { /** * @param client a pre-configured {@link GoogleCloudStorageClient} * @param layers the tile layer dispatcher to build tile keys from - * @throws org.geowebcache.storage.StorageException if the target bucket is not suitable for a cache (e.g. it's not - * empty and does not contain a {@code metadata.properties} marker file. + * @throws org.geowebcache.storage.UnsuitableStorageException if the target bucket is not suitable for a cache (e.g. + * it's not empty and does not contain a {@code metadata.properties} marker file or the bucket + * can't be accessed, for example due to bad credentials. + * @implNote {@link UnsuitableStorageException} will be thrown also if the bucket can't be accessed to account fot + * {@link XMLConfiguration#addBlobStore} and {@link XMLConfiguration#modifyBlobStore} checking for + * {@code instanceof UnsuitableStorageException} to prevent saving a misconfigured blob store. Otherwise the + * blobstore would be saved even with an invalid state and prevent application startup later on. */ public GoogleCloudStorageBlobStore(GoogleCloudStorageClient client, TileLayerDispatcher layers) - throws org.geowebcache.storage.StorageException { + throws org.geowebcache.storage.UnsuitableStorageException { this.client = requireNonNull(client); this.layers = requireNonNull(layers); @@ -84,7 +91,18 @@ public GoogleCloudStorageBlobStore(GoogleCloudStorageClient client, TileLayerDis String prefix = Optional.ofNullable(client.getPrefix()).orElse(""); this.keyBuilder = new TMSKeyBuilder(prefix, layers); - ensureCacheSuitability(prefix); + try { + ensureCacheSuitability(prefix); + } catch (UnsuitableStorageException e) { + throw e; + } catch (org.geowebcache.storage.StorageException somethingElse) { + // throw UnsuitableStorageException instead, which is a subclass of StorageException + // The GeoServer UI checks for instanceof UnsuitableStorageException when saving a blobstore that failed to + // be created. Otherwise it'll save it with the invalid configuration. + UnsuitableStorageException e = new UnsuitableStorageException(somethingElse.getMessage()); + e.addSuppressed(somethingElse); + throw e; + } } void ensureCacheSuitability(String prefix) throws org.geowebcache.storage.StorageException { @@ -213,10 +231,10 @@ public boolean deleteByParametersId(String layerName, String parametersId) Set gridsetAndFormatPrefixes = keyBuilder.forParameters(layerName, parametersId); // for each ///// - boolean prefixExists = gridsetAndFormatPrefixes.stream() - .map(client::deleteDirectory) - .reduce(Boolean::logicalOr) - .orElse(false); + boolean prefixExists = false; + for (String prefix : gridsetAndFormatPrefixes) { + prefixExists |= client.deleteDirectory(prefix); + } if (prefixExists) { listeners.sendParametersDeleted(layerName, parametersId); } @@ -444,18 +462,30 @@ private Properties getLayerMetadata(String layerName) { @Override public boolean layerExists(String layerName) { final String layerPrefix = keyBuilder.forLayer(layerName); - return client.directoryExists(layerPrefix); + try { + return client.directoryExists(layerPrefix); + } catch (org.geowebcache.storage.StorageException e) { + throw new UncheckedIOException(e); + } } + /** + * {@inheritDoc} + * + * @throws UncheckedIOException if {@link GoogleCloudStorageClient#list(String)} throws an + * {@link org.geowebcache.storage.StorageException} + */ @Override public Map>> getParametersMapping(String layerName) { String parametersMetadataPrefix = keyBuilder.parametersMetadataPrefix(layerName); - Stream blobStream = client.list(parametersMetadataPrefix); - - return blobStream - .map(Blob::getName) - .map(this::loadProperties) - .collect(Collectors.toMap(ParametersUtils::getId, Optional::ofNullable)); + try (Stream blobStream = client.list(parametersMetadataPrefix)) { + return blobStream + .map(Blob::getName) + .map(this::loadProperties) + .collect(Collectors.toMap(ParametersUtils::getId, Optional::ofNullable)); + } catch (org.geowebcache.storage.StorageException e) { + throw new UncheckedIOException(e); + } } private Optional findProperties(String key) { diff --git a/geowebcache/gcsblob/src/main/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageBlobStoreInfo.java b/geowebcache/gcsblob/src/main/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageBlobStoreInfo.java index 0c0ed47dc..91be169bd 100644 --- a/geowebcache/gcsblob/src/main/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageBlobStoreInfo.java +++ b/geowebcache/gcsblob/src/main/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageBlobStoreInfo.java @@ -44,7 +44,15 @@ public class GoogleCloudStorageBlobStoreInfo extends BlobStoreInfo { private String prefix; private String endpointUrl; // Custom endpoint for emulators or non-standard GCS endpoints private String apiKey; - private String useDefaultCredentialsChain; + private boolean useDefaultCredentialsChain; + + public GoogleCloudStorageBlobStoreInfo() { + super(); + } + + public GoogleCloudStorageBlobStoreInfo(String id) { + super(id); + } @Override public GoogleCloudStorageBlobStore createInstance(TileLayerDispatcher layers, LockProvider lockProvider) @@ -135,20 +143,20 @@ public void setApiKey(String apiKey) { } /** - * @return {@code "true"} if the default Google Cloud credentials chain should be used for authentication. + * @return {@code true} if the default Google Cloud credentials chain should be used for authentication. * @see com.google.auth.oauth2.GoogleCredentials#getApplicationDefault() */ - public String getUseDefaultCredentialsChain() { + public boolean getUseDefaultCredentialsChain() { return useDefaultCredentialsChain; } /** * Sets whether to use the default Google Cloud credentials chain. * - * @param defaultCredentialsChain {@code "true"} to enable, {@code "false"} to disable. + * @param useDefaultCredentialsChain {@code true} to enable, {@code false} to disable. */ - public void setUseDefaultCredentialsChain(String defaultCredentialsChain) { - this.useDefaultCredentialsChain = defaultCredentialsChain; + public void setUseDefaultCredentialsChain(boolean useDefaultCredentialsChain) { + this.useDefaultCredentialsChain = useDefaultCredentialsChain; } @Override @@ -162,6 +170,7 @@ public boolean equals(Object o) { GoogleCloudStorageBlobStoreInfo other = (GoogleCloudStorageBlobStoreInfo) o; return Objects.equals(projectId, other.projectId) && Objects.equals(bucket, other.bucket) + && Objects.equals(prefix, other.prefix) && Objects.equals(endpointUrl, other.endpointUrl) && Objects.equals(apiKey, other.apiKey) && Objects.equals(useDefaultCredentialsChain, other.useDefaultCredentialsChain) @@ -173,6 +182,7 @@ public boolean equals(Object o) { @Override public int hashCode() { return 31 * super.hashCode() - + Objects.hash(projectId, bucket, endpointUrl, apiKey, useDefaultCredentialsChain, quotaProjectId); + + Objects.hash( + projectId, bucket, prefix, endpointUrl, apiKey, useDefaultCredentialsChain, quotaProjectId); } } diff --git a/geowebcache/gcsblob/src/main/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageClient.java b/geowebcache/gcsblob/src/main/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageClient.java index 6f4fbd577..b671d6998 100644 --- a/geowebcache/gcsblob/src/main/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageClient.java +++ b/geowebcache/gcsblob/src/main/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageClient.java @@ -126,9 +126,7 @@ public static GoogleCloudStorageClient.Builder builder( .resolveValueIfEnabled(config.getApiKey(), String.class) .orElse(null)); - builder.useDefaultCredentialsChain(environment - .resolveValueIfEnabled(config.getUseDefaultCredentialsChain(), Boolean.class) - .orElse(false)); + builder.useDefaultCredentialsChain(config.getUseDefaultCredentialsChain()); return builder; } @@ -187,37 +185,43 @@ public Builder useDefaultCredentialsChain(boolean useDefaultCredentialsChain) { } public GoogleCloudStorageClient build() throws org.geowebcache.storage.StorageException { - StorageOptions.Builder builder = StorageOptions.getDefaultInstance().toBuilder(); + try { + StorageOptions.Builder builder = StorageOptions.getDefaultInstance().toBuilder(); - if (projectId != null) { - builder.setProjectId(projectId); - } - if (quotaProjectId != null) { - builder.setQuotaProjectId(quotaProjectId); - } - if (endpointUrl != null) { - // Set custom endpoint for emulators or non-standard GCS endpoints - builder.setHost(endpointUrl); - } - Credentials credentials = null; - if (apiKey != null) { - credentials = ApiKeyCredentials.create(apiKey); - } else if (useDefaultCredentialsChain) { - try { - credentials = GoogleCredentials.getApplicationDefault(); - } catch (IOException e) { - throw new org.geowebcache.storage.StorageException("Error obtaining default credentials", e); + if (projectId != null) { + builder.setProjectId(projectId); } - } - if (credentials != null) { - // credentials need to be set after projectId and quotaProjectId so its setter will - // check whether projectId is null and get it from credentials if its a ServiceAccountCredentials - // or quotaProjectId is null and get it from credentials if it's a QuotaProjectIdProvider - builder.setCredentials(credentials); - } - Storage storageClient = builder.build().getService(); + if (quotaProjectId != null) { + builder.setQuotaProjectId(quotaProjectId); + } + if (endpointUrl != null) { + // Set custom endpoint for emulators or non-standard GCS endpoints + builder.setHost(endpointUrl); + } + Credentials credentials = null; + if (apiKey != null) { + credentials = ApiKeyCredentials.create(apiKey); + } else if (useDefaultCredentialsChain) { + try { + credentials = GoogleCredentials.getApplicationDefault(); + } catch (IOException e) { + throw new org.geowebcache.storage.StorageException( + "Error obtaining default credentials: " + e.getMessage(), e); + } + } + if (credentials != null) { + // credentials need to be set after projectId and quotaProjectId so its setter will + // check whether projectId is null and get it from credentials if its a ServiceAccountCredentials + // or quotaProjectId is null and get it from credentials if it's a QuotaProjectIdProvider + builder.setCredentials(credentials); + } + Storage storageClient = builder.build().getService(); - return new GoogleCloudStorageClient(storageClient, bucket, prefix); + return new GoogleCloudStorageClient(storageClient, bucket, prefix); + } catch (StorageException gcse) { + throw new org.geowebcache.storage.StorageException( + "Error creating GCS client: " + gcse.getMessage(), gcse); + } } } @@ -265,9 +269,13 @@ public boolean blobExists(String key) throws org.geowebcache.storage.StorageExce * @param prefix The prefix to filter blobs by. * @return A stream of matching {@link Blob} objects. */ - public Stream list(String prefix) { - return storage.list(bucket, BlobListOption.prefix(requireNonNull(prefix))) - .streamAll(); + public Stream list(String prefix) throws org.geowebcache.storage.StorageException { + try { + return storage.list(bucket, BlobListOption.prefix(requireNonNull(prefix))) + .streamAll(); + } catch (StorageException gcse) { + throw new org.geowebcache.storage.StorageException(gcse.getMessage(), gcse); + } } /** @@ -279,10 +287,15 @@ public Stream list(String prefix) { * @param path The directory path to check. * @return {@code true} if at least one blob exists with this path prefix, {@code false} otherwise. */ - public boolean directoryExists(final String path) { + public boolean directoryExists(final String path) throws org.geowebcache.storage.StorageException { requireNonNull(path); String dirPrefix = dirPrefix(path); - Page blobs = storage.list(bucket, BlobListOption.prefix(dirPrefix), BlobListOption.pageSize(1)); + Page blobs; + try { + blobs = storage.list(bucket, BlobListOption.prefix(dirPrefix), BlobListOption.pageSize(1)); + } catch (StorageException gcse) { + throw new org.geowebcache.storage.StorageException(gcse.getMessage(), gcse); + } Iterator iterator = blobs.getValues().iterator(); boolean hasNext = iterator.hasNext(); if (hasNext) { @@ -307,7 +320,7 @@ public boolean directoryExists(final String path) { * @return {@code true} if the directory existed and the delete task was submitted, {@code false} if the directory * did not exist. */ - public boolean deleteDirectory(String path) { + public boolean deleteDirectory(String path) throws org.geowebcache.storage.StorageException { requireNonNull(path); if (directoryExists(path)) { String dirPrefix = dirPrefix(path); diff --git a/geowebcache/gcsblob/src/test/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageConfigProviderTest.java b/geowebcache/gcsblob/src/test/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageConfigProviderTest.java index c01846e6f..e08ea612b 100644 --- a/geowebcache/gcsblob/src/test/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageConfigProviderTest.java +++ b/geowebcache/gcsblob/src/test/java/org/geowebcache/storage/blobstore/gcs/GoogleCloudStorageConfigProviderTest.java @@ -94,7 +94,6 @@ public void testValuesFromEnvironment() throws StorageException { assertEquals("${GCS_PREFIX}", config.getPrefix()); assertEquals("${GCS_ENDPOINT}", config.getEndpointUrl()); assertEquals("${GCS_APIKEY}", config.getApiKey()); - assertEquals("${GCS_DEFAULT_CREDENTIALS}", config.getUseDefaultCredentialsChain()); setProperty("GCS_PROJECT_ID", "my-project"); setProperty("GCS_QUOTA_PROJECT_ID", "bills-go-here"); @@ -112,6 +111,5 @@ public void testValuesFromEnvironment() throws StorageException { assertEquals("gwc", builder.prefix); assertEquals("https://goog.compatible.storage/api", builder.endpointUrl); assertEquals("goog-fake-api-key", builder.apiKey); - assertTrue(builder.useDefaultCredentialsChain); } }