From 7d5facb749c6eb1fdfefebce67e3712b6e57e532 Mon Sep 17 00:00:00 2001 From: Shinsuke Sugaya Date: Thu, 10 Sep 2026 10:48:12 +0900 Subject: [PATCH 1/2] refactor!: move the S3 storage backend to the fess-storage-s3 plugin The AWS SDK for S3 is 30 jars the distribution carried for every installation, on its own release cadence, to serve a backend most installations never configure. It ships in the fess-storage-s3 plugin now, alongside the S3 crawler client registration, the way fess-storage-gcs already ships the Google Cloud Storage SDK. S3StorageClient moves to org.codelibs.fess.storage.s3 in that plugin, so fess_storage.xml declares no component of its own. The file and the fess.xml include stay: fess_storage++.xml merges into it, and without a base file the plugins would have nowhere to contribute to. The plugin registers both s3StorageClient and s3_compatStorageClient, which is what keeps storage.type=auto working for MinIO and other S3-compatible endpoints once it is installed. Keeping the AWS jars out of the war is the exclusion on fess-crawler-lasta, not the removal of the direct declaration: fess-crawler still compiles S3Client and the s3: URL handler against the SDK. The netty-nio-client exclusion the direct declaration carried is not repeated because it had nothing to remove -- awssdk:s3 2.54.2 uses apache5-client for its synchronous transport and does not depend on netty-nio-client at any scope. Measured with mvn dependency:list -DincludeScope=runtime before and after: 286 runtime artifacts to 256. Exactly 30 removals, no additions, and no version changes, so nothing that stays behind fell back to a different version. The 30 jars are 8,799,602 bytes. crawler.file.protocols loses s3 from its shipped default in all three places that spell it out: fess_config.properties, the two FessConfig javadoc comments, and the compiled defaultMap. Missing that last one is what #3410 had to fix for the removed storage protocol. The plugin adds the protocol back through ProtocolHelper.addFileProtocol when it is installed. The s3: literals in ProtocolHelper itself stay: they are string comparisons with no class reference behind them, and dropping them would break /go/ links, the crawling wizard and file name decoding. StorageClientFactory keeps the storage.type-to-component-name mapping and the endpoint detection, which is now all core contributes toward reaching a plugin. Its exception names both plugins instead of only fess-storage-gcs. StorageClientFactoryTest asserted that every type the distribution serves has a component, which inverts: test_noTypeIsServedByCore now walks StorageType.values() and auto, and requires hasComponent to be false for each. test_app.xml includes the shipped fess_storage.xml, so re-adding a client to core -- and with it an SDK to the war -- fails a test rather than only showing up in the artifact. test_detectStorageType_mapsEndpointsToTypes is new and fixes the detection table directly. The prototype assertion is dropped here because core has no component left to assert it against; that contract belongs to the plugins now. Pairs with the fess-crawler change that stops registering s3Client, which has to be merged and published first: crawler/client.xml still naming the component while the SDK is gone makes every CrawlerClientFactory fail to build, and the unit suite does not catch it because test_app.xml does not include crawler/client.xml. --- pom.xml | 24 +- .../fess/mylasta/direction/FessConfig.java | 6 +- .../fess/storage/S3StorageClient.java | 298 ------------------ .../fess/storage/StorageClientFactory.java | 14 +- src/main/resources/fess_config.properties | 2 +- src/main/resources/fess_storage.xml | 27 +- .../admin/wizard/AdminWizardActionTest.java | 2 +- .../fess/app/web/go/GoActionTest.java | 2 +- .../transformer/FessFileTransformerTest.java | 2 +- .../fess/helper/ProtocolHelperTest.java | 11 +- .../storage/StorageClientFactoryTest.java | 67 ++-- src/test/resources/test_app.xml | 11 +- 12 files changed, 92 insertions(+), 374 deletions(-) delete mode 100644 src/main/java/org/codelibs/fess/storage/S3StorageClient.java diff --git a/pom.xml b/pom.xml index 51cf9a344..0c0ca1c39 100644 --- a/pom.xml +++ b/pom.xml @@ -1278,17 +1278,6 @@ oauth2-oidc-sdk ${oauth2.oidc.sdk.version} - - software.amazon.awssdk - s3 - ${s3.version} - - - software.amazon.awssdk - netty-nio-client - - - org.apache.httpcomponents.client5 httpclient5 @@ -1478,6 +1467,19 @@ com.google.cloud google-cloud-storage + + + software.amazon.awssdk + s3 + diff --git a/src/main/java/org/codelibs/fess/mylasta/direction/FessConfig.java b/src/main/java/org/codelibs/fess/mylasta/direction/FessConfig.java index 549b46ae8..cb1f98545 100644 --- a/src/main/java/org/codelibs/fess/mylasta/direction/FessConfig.java +++ b/src/main/java/org/codelibs/fess/mylasta/direction/FessConfig.java @@ -443,7 +443,7 @@ public interface FessConfig extends FessEnv, org.codelibs.fess.mylasta.direction /** The key of the configuration. e.g. http,https */ String CRAWLER_WEB_PROTOCOLS = "crawler.web.protocols"; - /** The key of the configuration. e.g. file,smb,smb1,ftp,s3 */ + /** The key of the configuration. e.g. file,smb,smb1,ftp */ String CRAWLER_FILE_PROTOCOLS = "crawler.file.protocols"; /** The key of the configuration. e.g. ^FESS_ENV_.* */ @@ -3571,7 +3571,7 @@ public interface FessConfig extends FessEnv, org.codelibs.fess.mylasta.direction /** * Get the value for the key 'crawler.file.protocols'.
- * The value is, e.g. file,smb,smb1,ftp,s3
+ * The value is, e.g. file,smb,smb1,ftp
* comment: Supported file protocols for crawling. * @return The value of found property. (NotNull: if not found, exception but basically no way) */ @@ -14560,7 +14560,7 @@ protected java.util.Map prepareGeneratedDefaultMap() { defaultMap.put(FessConfig.CRAWLER_DOCUMENT_FULLSTOP_CHARS, "u002eu06d4u2e3cu3002"); defaultMap.put(FessConfig.CRAWLER_CRAWLING_DATA_ENCODING, "UTF-8"); defaultMap.put(FessConfig.CRAWLER_WEB_PROTOCOLS, "http,https"); - defaultMap.put(FessConfig.CRAWLER_FILE_PROTOCOLS, "file,smb,smb1,ftp,s3"); + defaultMap.put(FessConfig.CRAWLER_FILE_PROTOCOLS, "file,smb,smb1,ftp"); defaultMap.put(FessConfig.CRAWLER_DATA_ENV_PARAM_KEY_PATTERN, "^FESS_ENV_.*"); defaultMap.put(FessConfig.CRAWLER_IGNORE_ROBOTS_TXT, "false"); defaultMap.put(FessConfig.CRAWLER_IGNORE_ROBOTS_TAGS, "false"); diff --git a/src/main/java/org/codelibs/fess/storage/S3StorageClient.java b/src/main/java/org/codelibs/fess/storage/S3StorageClient.java deleted file mode 100644 index b05ae6e7f..000000000 --- a/src/main/java/org/codelibs/fess/storage/S3StorageClient.java +++ /dev/null @@ -1,298 +0,0 @@ -/* - * Copyright 2012-2025 CodeLibs Project and the Others. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, - * either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ -package org.codelibs.fess.storage; - -import java.io.InputStream; -import java.io.OutputStream; -import java.net.URI; -import java.time.ZonedDateTime; -import java.util.ArrayList; -import java.util.Base64; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.codelibs.core.lang.StringUtil; -import org.codelibs.fess.crawler.Constants; -import org.codelibs.fess.exception.StorageException; -import org.codelibs.fess.mylasta.direction.FessConfig; -import org.codelibs.fess.util.ComponentUtil; - -import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; -import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; -import software.amazon.awssdk.core.sync.RequestBody; -import software.amazon.awssdk.regions.Region; -import software.amazon.awssdk.services.s3.S3Client; -import software.amazon.awssdk.services.s3.S3ClientBuilder; -import software.amazon.awssdk.services.s3.model.CommonPrefix; -import software.amazon.awssdk.services.s3.model.CreateBucketRequest; -import software.amazon.awssdk.services.s3.model.DeleteObjectRequest; -import software.amazon.awssdk.services.s3.model.GetObjectRequest; -import software.amazon.awssdk.services.s3.model.GetObjectTaggingRequest; -import software.amazon.awssdk.services.s3.model.GetObjectTaggingResponse; -import software.amazon.awssdk.services.s3.model.HeadBucketRequest; -import software.amazon.awssdk.services.s3.model.ListObjectsV2Request; -import software.amazon.awssdk.services.s3.model.ListObjectsV2Response; -import software.amazon.awssdk.services.s3.model.NoSuchBucketException; -import software.amazon.awssdk.services.s3.model.PutObjectRequest; -import software.amazon.awssdk.services.s3.model.PutObjectTaggingRequest; -import software.amazon.awssdk.services.s3.model.S3Object; -import software.amazon.awssdk.services.s3.model.Tag; -import software.amazon.awssdk.services.s3.model.Tagging; - -/** - * S3-compatible storage client implementation using AWS SDK v2. - * Supports Amazon S3, MinIO, and other S3-compatible storage systems. - */ -public class S3StorageClient implements StorageClient { - - private static final Logger logger = LogManager.getLogger(S3StorageClient.class); - - private S3Client s3Client; - private String bucket; - - /** - * Creates an unconfigured client, which is how LastaDi builds the prototype component. - * {@link #init()} reads the configuration and opens the connection. - */ - public S3StorageClient() { - // configured by init() - } - - /** - * Creates a new S3StorageClient instance. - * - * @param endpoint the S3 endpoint URL (null for AWS default) - * @param accessKey the AWS access key - * @param secretKey the AWS secret key - * @param bucket the bucket name - * @param region the AWS region - */ - public S3StorageClient(final String endpoint, final String accessKey, final String secretKey, final String bucket, - final String region) { - configure(endpoint, accessKey, secretKey, bucket, region); - } - - @Override - public void init() { - final FessConfig fessConfig = ComponentUtil.getFessConfig(); - configure(fessConfig.getStorageEndpoint(), fessConfig.getStorageAccessKey(), fessConfig.getStorageSecretKey(), - fessConfig.getStorageBucket(), fessConfig.getStorageRegion()); - } - - private void configure(final String endpoint, final String accessKey, final String secretKey, final String bucket, - final String region) { - this.bucket = bucket; - - final AwsBasicCredentials credentials = AwsBasicCredentials.create(accessKey, secretKey); - final S3ClientBuilder builder = - S3Client.builder().credentialsProvider(StaticCredentialsProvider.create(credentials)).region(Region.of(getRegion(region))); - - // For non-AWS endpoints (MinIO, etc.), set custom endpoint with path-style access - if (StringUtil.isNotBlank(endpoint)) { - builder.endpointOverride(URI.create(endpoint)).forcePathStyle(true); - } - - this.s3Client = builder.build(); - } - - private String getRegion(final String region) { - return StringUtil.isNotBlank(region) ? region : "us-east-1"; - } - - @Override - public void uploadObject(final String objectName, final InputStream inputStream, final long size, final String contentType) { - try { - final PutObjectRequest request = - PutObjectRequest.builder().bucket(bucket).key(objectName).contentType(contentType).contentLength(size).build(); - s3Client.putObject(request, RequestBody.fromInputStream(inputStream, size)); - } catch (final Exception e) { - throw new StorageException("Failed to upload " + objectName, e); - } - } - - @Override - public void downloadObject(final String objectName, final OutputStream outputStream) { - try { - final GetObjectRequest request = GetObjectRequest.builder().bucket(bucket).key(objectName).build(); - try (InputStream in = s3Client.getObject(request)) { - in.transferTo(outputStream); - } - } catch (final Exception e) { - throw new StorageException("Failed to download " + objectName, e); - } - } - - @Override - public void deleteObject(final String objectName) { - try { - final DeleteObjectRequest request = DeleteObjectRequest.builder().bucket(bucket).key(objectName).build(); - s3Client.deleteObject(request); - } catch (final Exception e) { - throw new StorageException("Failed to delete " + objectName, e); - } - } - - @Override - public List listObjects(final String prefix, final int maxItems) { - final List items = new ArrayList<>(); - - try { - final ListObjectsV2Request.Builder requestBuilder = - ListObjectsV2Request.builder().bucket(bucket).delimiter("/").maxKeys(maxItems); - - if (StringUtil.isNotBlank(prefix)) { - final String normalizedPrefix = prefix.endsWith("/") ? prefix : prefix + "/"; - requestBuilder.prefix(normalizedPrefix); - } - - final ListObjectsV2Response response = s3Client.listObjectsV2(requestBuilder.build()); - - // Process common prefixes (directories) - for (final CommonPrefix commonPrefix : response.commonPrefixes()) { - final String dirName = getName(commonPrefix.prefix()); - if (StringUtil.isNotBlank(dirName)) { - items.add(new StorageItem(dirName, prefix, true, 0, null, encodeId(commonPrefix.prefix()))); - } - } - - // Process objects (files) - for (final S3Object s3Object : response.contents()) { - final String objectKey = s3Object.key(); - // Skip directory markers (objects ending with /) - if (!objectKey.endsWith("/")) { - final String fileName = getName(objectKey); - final ZonedDateTime lastModified = - s3Object.lastModified() != null ? s3Object.lastModified().atZone(java.time.ZoneId.systemDefault()) : null; - items.add(new StorageItem(fileName, prefix, false, s3Object.size(), lastModified, encodeId(objectKey))); - } - } - } catch (final NoSuchBucketException e) { - logger.info("Bucket does not exist: {}", bucket); - } catch (final Exception e) { - if (logger.isDebugEnabled()) { - logger.debug("Failed to list objects in {}", bucket, e); - } - } - - return items; - } - - @Override - public Map getObjectTags(final String objectName) { - try { - final GetObjectTaggingRequest request = GetObjectTaggingRequest.builder().bucket(bucket).key(objectName).build(); - final GetObjectTaggingResponse response = s3Client.getObjectTagging(request); - return response.tagSet().stream().collect(Collectors.toMap(Tag::key, Tag::value)); - } catch (final Exception e) { - throw new StorageException("Failed to get tags from " + objectName, e); - } - } - - @Override - public void setObjectTags(final String objectName, final Map tags) { - try { - final List tagList = tags.entrySet() - .stream() - .map(e -> Tag.builder().key(e.getKey()).value(e.getValue()).build()) - .collect(Collectors.toList()); - - final PutObjectTaggingRequest request = PutObjectTaggingRequest.builder() - .bucket(bucket) - .key(objectName) - .tagging(Tagging.builder().tagSet(tagList).build()) - .build(); - s3Client.putObjectTagging(request); - } catch (final Exception e) { - throw new StorageException("Failed to update tags for " + objectName, e); - } - } - - @Override - public void ensureBucketExists() { - try { - final HeadBucketRequest request = HeadBucketRequest.builder().bucket(bucket).build(); - s3Client.headBucket(request); - } catch (final NoSuchBucketException e) { - try { - final CreateBucketRequest createRequest = CreateBucketRequest.builder().bucket(bucket).build(); - s3Client.createBucket(createRequest); - logger.info("Created storage bucket: {}", bucket); - } catch (final Exception e1) { - logger.warn("Failed to create storage bucket: {}", bucket, e1); - } - } catch (final Exception e) { - if (logger.isDebugEnabled()) { - logger.debug("Failed to check bucket: {}", bucket, e); - } - } - } - - @Override - public boolean isAvailable() { - try { - final HeadBucketRequest request = HeadBucketRequest.builder().bucket(bucket).build(); - s3Client.headBucket(request); - return true; - } catch (final Exception e) { - return false; - } - } - - @Override - public void close() { - if (s3Client != null) { - s3Client.close(); - } - } - - /** - * Extracts the file/directory name from a full object path. - * - * @param objectName the full object path - * @return the name portion of the path - */ - private String getName(final String objectName) { - if (StringUtil.isBlank(objectName)) { - return StringUtil.EMPTY; - } - // Remove trailing slash if present - String name = objectName; - if (name.endsWith("/")) { - name = name.substring(0, name.length() - 1); - } - final String[] values = name.split("/"); - if (values.length == 0) { - return StringUtil.EMPTY; - } - return values[values.length - 1]; - } - - /** - * Encodes an object name to a URL-safe base64 string. - * - * @param objectName the object name to encode - * @return base64 encoded string - */ - private String encodeId(final String objectName) { - if (objectName == null) { - return StringUtil.EMPTY; - } - return new String(Base64.getUrlEncoder().encode(objectName.getBytes(Constants.UTF_8_CHARSET)), Constants.UTF_8_CHARSET); - } -} diff --git a/src/main/java/org/codelibs/fess/storage/StorageClientFactory.java b/src/main/java/org/codelibs/fess/storage/StorageClientFactory.java index 14f371178..021e3c0f5 100644 --- a/src/main/java/org/codelibs/fess/storage/StorageClientFactory.java +++ b/src/main/java/org/codelibs/fess/storage/StorageClientFactory.java @@ -72,8 +72,10 @@ public static StorageType detectStorageType(final String endpoint) { public static StorageClient createClient(final FessConfig fessConfig) { final String componentName = componentName(fessConfig.getStorageType(), fessConfig.getStorageEndpoint()); if (!ComponentUtil.hasComponent(componentName)) { - throw new StorageException("No storage client is registered as " + componentName + " for storage.type=" - + fessConfig.getStorageType() + ". Install the plugin that provides it, such as fess-storage-gcs for gcs."); + throw new StorageException( + "No storage client is registered as " + componentName + " for storage.type=" + fessConfig.getStorageType() + + ". Every backend ships as a fess-storage-* plugin: install fess-storage-s3 for s3 and s3_compat, " + + "or fess-storage-gcs for gcs."); } if (logger.isDebugEnabled()) { logger.debug("Creating {} for endpoint: {}", componentName, fessConfig.getStorageEndpoint()); @@ -87,10 +89,10 @@ public static StorageClient createClient(final FessConfig fessConfig) { * Returns the name of the DI component that serves a storage type. * *

The mapping from a {@code storage.type} value to an implementation lives in the DI - * definition rather than here, which is what lets the clients ship as plugins: core no longer - * names GcsStorageClient or S3StorageClient, and a plugin registering - * {@code StorageClient} is reachable by setting {@code storage.type=}. The - * components are prototypes because every caller closes the client it was handed.

+ * definition rather than here, which is what lets the clients ship as plugins: core names no + * implementation at all, and a plugin registering {@code StorageClient} is reachable by + * setting {@code storage.type=}. The components are prototypes because every caller + * closes the client it was handed.

* * @param typeStr the configured type, blank or {@code auto} to detect from the endpoint * @param endpoint the storage endpoint, used only when detecting diff --git a/src/main/resources/fess_config.properties b/src/main/resources/fess_config.properties index 930757415..066b66cda 100644 --- a/src/main/resources/fess_config.properties +++ b/src/main/resources/fess_config.properties @@ -383,7 +383,7 @@ crawler.crawling.data.encoding=UTF-8 # Supported web protocols for crawling. crawler.web.protocols=http,https # Supported file protocols for crawling. -crawler.file.protocols=file,smb,smb1,ftp,s3 +crawler.file.protocols=file,smb,smb1,ftp # Pattern for environment variable keys in crawling data. crawler.data.env.param.key.pattern=^FESS_ENV_.* # Whether to ignore robots.txt during crawling. diff --git a/src/main/resources/fess_storage.xml b/src/main/resources/fess_storage.xml index 00846fbc8..faae617b6 100644 --- a/src/main/resources/fess_storage.xml +++ b/src/main/resources/fess_storage.xml @@ -2,21 +2,18 @@ - - - - - - diff --git a/src/test/java/org/codelibs/fess/app/web/admin/wizard/AdminWizardActionTest.java b/src/test/java/org/codelibs/fess/app/web/admin/wizard/AdminWizardActionTest.java index 3c7568eb3..190ebf75c 100644 --- a/src/test/java/org/codelibs/fess/app/web/admin/wizard/AdminWizardActionTest.java +++ b/src/test/java/org/codelibs/fess/app/web/admin/wizard/AdminWizardActionTest.java @@ -42,7 +42,7 @@ public String getCrawlerWebProtocols() { @Override public String getCrawlerFileProtocols() { - return "file,smb,smb1,ftp,s3"; + return "file,smb,smb1,ftp"; } }); final ProtocolHelper protocolHelper = new ProtocolHelper(); diff --git a/src/test/java/org/codelibs/fess/app/web/go/GoActionTest.java b/src/test/java/org/codelibs/fess/app/web/go/GoActionTest.java index 97166ee3f..114a6205b 100644 --- a/src/test/java/org/codelibs/fess/app/web/go/GoActionTest.java +++ b/src/test/java/org/codelibs/fess/app/web/go/GoActionTest.java @@ -48,7 +48,7 @@ public String getCrawlerWebProtocols() { @Override public String getCrawlerFileProtocols() { - return "file,smb,smb1,ftp,s3"; + return "file,smb,smb1,ftp"; } }); final ProtocolHelper protocolHelper = new ProtocolHelper(); diff --git a/src/test/java/org/codelibs/fess/crawler/transformer/FessFileTransformerTest.java b/src/test/java/org/codelibs/fess/crawler/transformer/FessFileTransformerTest.java index ae37d5ab2..000eb934e 100644 --- a/src/test/java/org/codelibs/fess/crawler/transformer/FessFileTransformerTest.java +++ b/src/test/java/org/codelibs/fess/crawler/transformer/FessFileTransformerTest.java @@ -46,7 +46,7 @@ public String getCrawlerWebProtocols() { @Override public String getCrawlerFileProtocols() { - return "file,smb,smb1,ftp,s3"; + return "file,smb,smb1,ftp"; } @Override diff --git a/src/test/java/org/codelibs/fess/helper/ProtocolHelperTest.java b/src/test/java/org/codelibs/fess/helper/ProtocolHelperTest.java index eb09cb026..08bf70936 100644 --- a/src/test/java/org/codelibs/fess/helper/ProtocolHelperTest.java +++ b/src/test/java/org/codelibs/fess/helper/ProtocolHelperTest.java @@ -738,24 +738,25 @@ public String getCrawlerWebProtocols() { @Override public String getCrawlerFileProtocols() { - return "file,smb,smb1,ftp,s3"; + return "file,smb,smb1,ftp"; } }); final ProtocolHelper protocolHelper = new ProtocolHelper(); protocolHelper.init(); - assertEquals(5, protocolHelper.getFileProtocols().length); + assertEquals(4, protocolHelper.getFileProtocols().length); // All file protocols should be valid assertTrue(protocolHelper.isValidFileProtocol("file:///path/to/file")); assertTrue(protocolHelper.isValidFileProtocol("smb://server/share")); assertTrue(protocolHelper.isValidFileProtocol("smb1://server/share")); assertTrue(protocolHelper.isValidFileProtocol("ftp://ftp.example.com/file")); - assertTrue(protocolHelper.isValidFileProtocol("s3://bucket/key")); - // gcs is not in the shipped default: fess-storage-gcs adds it with addFileProtocol when the - // plugin is installed, which test_s3_gcs_protocols_add_dynamically covers. + // Neither object-storage protocol is in the shipped default: fess-storage-s3 and + // fess-storage-gcs add theirs with addFileProtocol when the plugin is installed, which + // test_s3_gcs_protocols_add_dynamically covers. + assertFalse(protocolHelper.isValidFileProtocol("s3://bucket/key")); assertFalse(protocolHelper.isValidFileProtocol("gcs://bucket/object")); // Web protocols should NOT be valid as file protocols diff --git a/src/test/java/org/codelibs/fess/storage/StorageClientFactoryTest.java b/src/test/java/org/codelibs/fess/storage/StorageClientFactoryTest.java index 92c7e6502..c998bac65 100644 --- a/src/test/java/org/codelibs/fess/storage/StorageClientFactoryTest.java +++ b/src/test/java/org/codelibs/fess/storage/StorageClientFactoryTest.java @@ -18,6 +18,12 @@ import org.codelibs.fess.unit.UnitFessTestCase; import org.junit.jupiter.api.Test; +/** + * What core still owns is the type-to-name mapping: {@link StorageClientFactory#componentName} + * and the endpoint detection behind it. The implementations moved to fess-storage-s3 and + * fess-storage-gcs together with their SDKs, so the component definitions and the prototype + * contract are verified in those plugins, not here. + */ public class StorageClientFactoryTest extends UnitFessTestCase { @Test @@ -49,42 +55,49 @@ public void test_componentName_detectsFromTheEndpoint() { } /** - * Every name {@link #test_componentName_detectsFromTheEndpoint} and the storage.type values - * the distribution serves can produce has to exist in fess_storage.xml, or the admin storage - * screen fails at runtime for a configuration the UI offers. + * The distribution on its own serves no storage type: every backend ships as a + * fess-storage-* plugin that contributes its component through fess_storage++.xml, so + * fess_storage.xml declares nothing and storage.type is a configuration error until the + * matching plugin is installed. This asserts the absence, because registering any of these + * names in core again would pull an SDK back into the war - the AWS one for s3 and s3_compat, + * the Google Cloud Storage one for gcs. + * + *

test_app.xml includes fess_storage.xml, so this runs against the file the distribution + * ships rather than against an empty container.

*/ @Test - public void test_everyTypeTheDistributionServesHasAComponent() { - for (final StorageType type : new StorageType[] { StorageType.S3, StorageType.S3_COMPAT }) { + public void test_noTypeIsServedByCore() { + for (final StorageType type : StorageType.values()) { final String name = StorageClientFactory.componentName(type.name(), null); - assertTrue(org.codelibs.fess.util.ComponentUtil.hasComponent(name), name + " is not registered in fess_storage.xml"); + assertFalse(org.codelibs.fess.util.ComponentUtil.hasComponent(name), + name + " belongs to a fess-storage-* plugin and must not be declared in fess_storage.xml"); } - assertTrue(org.codelibs.fess.util.ComponentUtil.hasComponent(StorageClientFactory.componentName("auto", null))); + // auto with no endpoint resolves to s3, which is a plugin as well. + assertFalse(org.codelibs.fess.util.ComponentUtil.hasComponent(StorageClientFactory.componentName("auto", null))); } /** - * GCS is deliberately not in that list. Its client ships in fess-storage-gcs, together with the - * Google Cloud Storage SDK the distribution no longer carries, so what core keeps is the name - * the plugin registers under and the endpoint detection that produces it. Registering a - * component here again would pull the SDK back into the war. + * With the clients gone, the mapping from an endpoint to a type name is what core contributes + * to reaching them, so it is fixed here as well as through {@link #componentName} above: a + * type that came out wrong would resolve a component name no installed plugin registers. */ @Test - public void test_gcsIsServedByAPluginRatherThanCore() { - assertEquals("gcsStorageClient", StorageClientFactory.componentName(StorageType.GCS.name(), null)); - assertFalse(org.codelibs.fess.util.ComponentUtil.hasComponent("gcsStorageClient"), - "gcsStorageClient belongs to fess-storage-gcs, which contributes it through fess_storage++.xml"); - } + public void test_detectStorageType_mapsEndpointsToTypes() { + // No endpoint at all means AWS, which supplies its own. + assertEquals(StorageType.S3, StorageClientFactory.detectStorageType(null)); + assertEquals(StorageType.S3, StorageClientFactory.detectStorageType("")); + assertEquals(StorageType.S3, StorageClientFactory.detectStorageType(" ")); - /** - * A prototype, not a singleton: every caller closes the client it was handed, so a shared - * instance would be shut down for everybody by the first caller to finish. - */ - @Test - public void test_theComponentsArePrototypes() { - final Object first = org.codelibs.fess.util.ComponentUtil.getComponent("s3StorageClient"); - final Object second = org.codelibs.fess.util.ComponentUtil.getComponent("s3StorageClient"); - assertNotNull(first); - assertNotNull(second); - assertFalse(first == second, "s3StorageClient must be instance=\"prototype\""); + assertEquals(StorageType.GCS, StorageClientFactory.detectStorageType("https://storage.googleapis.com")); + assertEquals(StorageType.GCS, StorageClientFactory.detectStorageType("https://STORAGE.GOOGLEAPIS.COM")); + assertEquals(StorageType.GCS, StorageClientFactory.detectStorageType("https://bucket.storage.cloud.google.com")); + + assertEquals(StorageType.S3, StorageClientFactory.detectStorageType("https://s3.us-east-1.amazonaws.com")); + assertEquals(StorageType.S3, StorageClientFactory.detectStorageType("https://bucket.s3.amazonaws.com")); + assertEquals(StorageType.S3, StorageClientFactory.detectStorageType("https://s3-accelerate.amazonaws.com")); + + // No vendor host: MinIO and the like, served by the same client as s3 under another name. + assertEquals(StorageType.S3_COMPAT, StorageClientFactory.detectStorageType("http://minio.internal:9000")); + assertEquals(StorageType.S3_COMPAT, StorageClientFactory.detectStorageType("https://objects.example.com")); } } diff --git a/src/test/resources/test_app.xml b/src/test/resources/test_app.xml index ec499dfe6..a2171bded 100644 --- a/src/test/resources/test_app.xml +++ b/src/test/resources/test_app.xml @@ -5,11 +5,12 @@ - + From 109d2ccda5854b30a44d1a59ce37f1efad766b7c Mon Sep 17 00:00:00 2001 From: Shinsuke Sugaya Date: Thu, 10 Sep 2026 10:48:36 +0900 Subject: [PATCH 2/2] test(storage): run the storage unit tests JUnit 5 was skipping StorageTypeTest and StorageItemTest are written in the JUnit 3 style the suite used before: public void test_* on a UnitFessTestCase subclass, with no annotation. UnitFessTestCase runs on JUnit 5 through utflute, which discovers tests by annotation, so neither class contributed a single test -- they are absent from all 462 surefire reports of a full mvn test run. Annotating them makes 17 tests run, and they pass as written. This matters here because StorageType is one of the two things core still contributes toward reaching a storage plugin, now that the clients themselves ship as fess-storage-*: StorageTypeTest is what pins its constants, and it was pinning nothing. 27 test classes in this repository have the same shape, roughly 65 test methods; only the two in the storage package are annotated here. --- .../org/codelibs/fess/storage/StorageItemTest.java | 11 +++++++++++ .../org/codelibs/fess/storage/StorageTypeTest.java | 8 ++++++++ 2 files changed, 19 insertions(+) diff --git a/src/test/java/org/codelibs/fess/storage/StorageItemTest.java b/src/test/java/org/codelibs/fess/storage/StorageItemTest.java index 7f6f0fcb5..4f3d6f90b 100644 --- a/src/test/java/org/codelibs/fess/storage/StorageItemTest.java +++ b/src/test/java/org/codelibs/fess/storage/StorageItemTest.java @@ -19,9 +19,11 @@ import java.time.ZonedDateTime; import org.codelibs.fess.unit.UnitFessTestCase; +import org.junit.jupiter.api.Test; public class StorageItemTest extends UnitFessTestCase { + @Test public void test_constructorAndGetters() { final String name = "test-file.txt"; final String path = "/documents/"; @@ -40,6 +42,7 @@ public void test_constructorAndGetters() { assertEquals(encodedId, item.getEncodedId()); } + @Test public void test_directoryItem() { final String name = "documents"; final String path = "/"; @@ -57,6 +60,7 @@ public void test_directoryItem() { assertEquals(encodedId, item.getEncodedId()); } + @Test public void test_fileItem() { final ZonedDateTime now = ZonedDateTime.now(); final StorageItem item = new StorageItem("report.pdf", "/reports/2025/", false, 2048576L, now, "cmVwb3J0LnBkZg=="); @@ -68,6 +72,7 @@ public void test_fileItem() { assertEquals(now, item.getLastModified()); } + @Test public void test_nullValues() { final StorageItem item = new StorageItem(null, null, false, 0L, null, null); @@ -79,6 +84,7 @@ public void test_nullValues() { assertNull(item.getEncodedId()); } + @Test public void test_emptyName() { final StorageItem item = new StorageItem("", "/path/", false, 100L, null, ""); @@ -87,6 +93,7 @@ public void test_emptyName() { assertEquals("", item.getEncodedId()); } + @Test public void test_largeFileSize() { final long largeSize = 10737418240L; // 10 GB final StorageItem item = new StorageItem("large-file.zip", "/backups/", false, largeSize, null, "bGFyZ2UtZmlsZS56aXA="); @@ -94,6 +101,7 @@ public void test_largeFileSize() { assertEquals(largeSize, item.getSize()); } + @Test public void test_rootPath() { final StorageItem item = new StorageItem("root-file.txt", "", false, 512L, null, "cm9vdC1maWxlLnR4dA=="); @@ -101,6 +109,7 @@ public void test_rootPath() { assertEquals("", item.getPath()); } + @Test public void test_deeplyNestedPath() { final String deepPath = "/level1/level2/level3/level4/level5/"; final StorageItem item = new StorageItem("nested.txt", deepPath, false, 256L, null, "bmVzdGVkLnR4dA=="); @@ -108,6 +117,7 @@ public void test_deeplyNestedPath() { assertEquals(deepPath, item.getPath()); } + @Test public void test_specialCharactersInName() { final String specialName = "file with spaces & symbols!@#.txt"; final StorageItem item = new StorageItem(specialName, "/special/", false, 100L, null, "c3BlY2lhbA=="); @@ -115,6 +125,7 @@ public void test_specialCharactersInName() { assertEquals(specialName, item.getName()); } + @Test public void test_unicodeInName() { final String unicodeName = "ファイル名.txt"; final StorageItem item = new StorageItem(unicodeName, "/unicode/", false, 200L, null, "dW5pY29kZQ=="); diff --git a/src/test/java/org/codelibs/fess/storage/StorageTypeTest.java b/src/test/java/org/codelibs/fess/storage/StorageTypeTest.java index 73901c7a0..917f6729b 100644 --- a/src/test/java/org/codelibs/fess/storage/StorageTypeTest.java +++ b/src/test/java/org/codelibs/fess/storage/StorageTypeTest.java @@ -16,39 +16,47 @@ package org.codelibs.fess.storage; import org.codelibs.fess.unit.UnitFessTestCase; +import org.junit.jupiter.api.Test; public class StorageTypeTest extends UnitFessTestCase { + @Test public void test_enumValues() { final StorageType[] types = StorageType.values(); assertNotNull(types); assertEquals(3, types.length); } + @Test public void test_s3Type() { assertEquals(StorageType.S3, StorageType.valueOf("S3")); } + @Test public void test_gcsType() { assertEquals(StorageType.GCS, StorageType.valueOf("GCS")); } + @Test public void test_s3CompatType() { assertEquals(StorageType.S3_COMPAT, StorageType.valueOf("S3_COMPAT")); } + @Test public void test_valueOf() { for (final StorageType type : StorageType.values()) { assertEquals(type, StorageType.valueOf(type.name())); } } + @Test public void test_ordinal() { assertEquals(0, StorageType.S3.ordinal()); assertEquals(1, StorageType.GCS.ordinal()); assertEquals(2, StorageType.S3_COMPAT.ordinal()); } + @Test public void test_name() { assertEquals("S3", StorageType.S3.name()); assertEquals("GCS", StorageType.GCS.name());