From d731591b7c1124cedd9fe063a838c70786dddf6a Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Wed, 2 Sep 2026 14:34:46 +0300 Subject: [PATCH 01/10] fix: map the platform's system buckets in the tenant-rooted layout #1863 TenantLayoutTransform knew four bucket locations; the codebase builds descriptors against four more. Each threw under the tenant-rooted layout, and because these paths are composed off the request path the failures took out a subsystem rather than a request: deployment_cost_stats/ per-request cost accounting, on the /v1/deployments path background_jobs/ background job scanning response_mappings/ the response id index api_key_data/ per-request keys, so every application caller They now map to a reserved .system/ segment at the root, above any tenant, which is what preserves today's behaviour: the job scheduler scans its bucket whole and all four are keyed by globally unique ids. Whether this state should become tenant-scoped is a question for the phase that turns multi-tenancy on. The locations move to ResourceDescriptor so a layout can enumerate them, and their callers reference those constants - a second literal next to the caller is how these came to be missed. An unregistered location still throws rather than falling back silently. Separately, invitation ids encoded getAbsoluteFilePath(), which the seam made layout-dependent, so getInvitationResource could not parse them back and share-by-link returned 404. They now use getStableFilePath(), which is the legacy shape whichever layout is active - an id handed to a user has to keep resolving across a layout change. Co-Authored-By: Claude Opus 5 (1M context) --- .../core/server/security/ApiKeyStore.java | 5 +- .../server/service/InvitationService.java | 5 +- .../core/server/token/TokenStatsTracker.java | 7 +- .../core/server/util/ResponseIdUtil.java | 10 +- .../server/util/SystemBucketLayoutTest.java | 107 ++++++++++++++++++ .../storage/resource/ResourceDescriptor.java | 56 +++++++-- .../resource/TenantLayoutTransform.java | 24 ++++ .../resource/TenantLayoutTransformTest.java | 36 ++++++ .../TenantRootedStorageLayoutTest.java | 26 +++++ 9 files changed, 256 insertions(+), 20 deletions(-) create mode 100644 server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java diff --git a/server/src/main/java/com/epam/aidial/core/server/security/ApiKeyStore.java b/server/src/main/java/com/epam/aidial/core/server/security/ApiKeyStore.java index 1421fc4cb..937598043 100644 --- a/server/src/main/java/com/epam/aidial/core/server/security/ApiKeyStore.java +++ b/server/src/main/java/com/epam/aidial/core/server/security/ApiKeyStore.java @@ -40,8 +40,9 @@ @Slf4j public class ApiKeyStore { - public static final String API_KEY_DATA_BUCKET = "api_key_data"; - public static final String API_KEY_DATA_LOCATION = API_KEY_DATA_BUCKET + PATH_SEPARATOR; + // Declared in ResourceDescriptor so a storage layout can enumerate the system buckets. + public static final String API_KEY_DATA_BUCKET = ResourceDescriptor.API_KEY_DATA_BUCKET; + public static final String API_KEY_DATA_LOCATION = ResourceDescriptor.API_KEY_DATA_LOCATION; private final AsyncTaskExecutor taskExecutor; private final RedissonClient redis; diff --git a/server/src/main/java/com/epam/aidial/core/server/service/InvitationService.java b/server/src/main/java/com/epam/aidial/core/server/service/InvitationService.java index ebbae1122..8d438642d 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/InvitationService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/InvitationService.java @@ -269,7 +269,10 @@ public ResourceDescriptor getInvitationResource(String invitationId) { return ResourceDescriptorFactory.fromDecoded(resourceType, bucket, location, INVITATION_RESOURCE_FILENAME); } + // The id is handed out as a link and stored inside the invitations map, and getInvitationResource parses + // the location back out of it. It must therefore not carry the physical path, which the storage layout is + // free to change: an invitation issued before a layout change has to keep resolving after it. private String generateInvitationId(ResourceDescriptor resource) { - return encryptionService.encrypt(resource.getAbsoluteFilePath() + ResourceDescriptor.PATH_SEPARATOR + ApiKeyGenerator.generateKey()); + return encryptionService.encrypt(resource.getStableFilePath() + ResourceDescriptor.PATH_SEPARATOR + ApiKeyGenerator.generateKey()); } } diff --git a/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java b/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java index 36780fcf7..ab577e381 100644 --- a/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java +++ b/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java @@ -21,13 +21,14 @@ import java.util.List; import java.util.Map; -import static com.epam.aidial.core.storage.resource.ResourceDescriptor.PATH_SEPARATOR; @Slf4j @RequiredArgsConstructor public class TokenStatsTracker { - public static final String DEPLOYMENT_COST_STATS_BUCKET = "deployment_cost_stats"; - public static final String DEPLOYMENT_COST_STATS_LOCATION = DEPLOYMENT_COST_STATS_BUCKET + PATH_SEPARATOR; + // Declared in ResourceDescriptor so a storage layout can enumerate the system buckets; a second literal + // here is how the tenant-rooted layout came to reject this location in the first place. + public static final String DEPLOYMENT_COST_STATS_BUCKET = ResourceDescriptor.DEPLOYMENT_COST_STATS_BUCKET; + public static final String DEPLOYMENT_COST_STATS_LOCATION = ResourceDescriptor.DEPLOYMENT_COST_STATS_LOCATION; private final AsyncTaskExecutor taskExecutor; private final ResourceService resourceService; diff --git a/server/src/main/java/com/epam/aidial/core/server/util/ResponseIdUtil.java b/server/src/main/java/com/epam/aidial/core/server/util/ResponseIdUtil.java index 6c7476173..d812d4246 100644 --- a/server/src/main/java/com/epam/aidial/core/server/util/ResponseIdUtil.java +++ b/server/src/main/java/com/epam/aidial/core/server/util/ResponseIdUtil.java @@ -6,10 +6,12 @@ @UtilityClass public class ResponseIdUtil { - public static final String RESPONSE_MAPPINGS_BUCKET = "response_mappings"; - public static final String RESPONSE_MAPPINGS_BUCKET_LOCATION = RESPONSE_MAPPINGS_BUCKET + "/"; - public static final String BACKGROUND_JOB_BUCKET = "background_jobs"; - public static final String BACKGROUND_JOB_BUCKET_LOCATION = BACKGROUND_JOB_BUCKET + "/"; + // Declared in ResourceDescriptor so a storage layout can enumerate the system buckets; a second literal + // here is how the tenant-rooted layout came to reject these locations in the first place. + public static final String RESPONSE_MAPPINGS_BUCKET = ResourceDescriptor.RESPONSE_MAPPINGS_BUCKET; + public static final String RESPONSE_MAPPINGS_BUCKET_LOCATION = ResourceDescriptor.RESPONSE_MAPPINGS_LOCATION; + public static final String BACKGROUND_JOB_BUCKET = ResourceDescriptor.BACKGROUND_JOB_BUCKET; + public static final String BACKGROUND_JOB_BUCKET_LOCATION = ResourceDescriptor.BACKGROUND_JOB_LOCATION; public static final String RESPONSE_ID_PREFIX = "dial_"; public String createResponseId(String deploymentName, String uuid) { diff --git a/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java b/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java new file mode 100644 index 000000000..0a0bf8ed2 --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java @@ -0,0 +1,107 @@ +package com.epam.aidial.core.server.util; + +import com.epam.aidial.core.server.security.ApiKeyStore; +import com.epam.aidial.core.server.token.TokenStatsTracker; +import com.epam.aidial.core.storage.resource.LegacyStorageLayout; +import com.epam.aidial.core.storage.resource.ResourceDescriptor; +import com.epam.aidial.core.storage.resource.ResourceTypes; +import com.epam.aidial.core.storage.resource.StorageLayouts; +import com.epam.aidial.core.storage.resource.TenantRootedStorageLayout; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Every descriptor the platform builds against a system bucket has to compose a path under the tenant-rooted + * layout, not just the ones the resource API can reach. + * + *

These paths are built off the request path — cost accounting runs per span, the job scheduler on a timer, + * response mappings on every completion — so an unmapped location does not surface as a failed request. It + * throws inside a background task and takes the subsystem out silently, which is how all three of these came + * to be broken under the tenant-rooted layout without a single test going red. + */ +public class SystemBucketLayoutTest { + + private static final String TENANT = "acme"; + + @AfterEach + public void restoreLegacyLayout() { + StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); + } + + @Test + public void testResponseMappingPath() { + StorageLayouts.useLayout(new TenantRootedStorageLayout(TENANT)); + + ResourceDescriptor descriptor = ResponseIdUtil.getResponseMappingDescriptor("dial_gpt-4_abc123"); + + assertEquals(".system/response_mappings/.response_mappings/gpt-4/abc123", descriptor.getAbsoluteFilePath()); + assertEquals("response_mappings/response_mappings/gpt-4/abc123", descriptor.getStableFilePath()); + } + + @Test + public void testBackgroundJobPath() { + StorageLayouts.useLayout(new TenantRootedStorageLayout(TENANT)); + + ResourceDescriptor descriptor = ResponseIdUtil.getBackgroundJobDescriptor("job-1"); + + assertEquals(".system/background_jobs/.background_jobs/job-1", descriptor.getAbsoluteFilePath()); + } + + /** + * The scheduler scans the whole bucket, and cost accounting deletes by trace id — both address the folder + * itself, which is a separate composition path from an item. + */ + @Test + public void testBackgroundJobRootFolderPath() { + StorageLayouts.useLayout(new TenantRootedStorageLayout(TENANT)); + + ResourceDescriptor root = ResponseIdUtil.getBackgroundJobDescriptor(null); + + assertEquals(".system/background_jobs/.background_jobs/", root.getAbsoluteFilePath()); + } + + // Per-request keys are how an application caller is represented, so this one breaks four of the eleven + // permission rules rather than a background task. + @Test + public void testApiKeyDataPath() { + StorageLayouts.useLayout(new TenantRootedStorageLayout(TENANT)); + + ResourceDescriptor descriptor = ResourceDescriptorFactory.fromDecoded(ResourceTypes.API_KEY_DATA, + ApiKeyStore.API_KEY_DATA_BUCKET, ApiKeyStore.API_KEY_DATA_LOCATION, "some-key"); + + assertEquals(".system/api_key_data/.api_key_data/some-key", descriptor.getAbsoluteFilePath()); + } + + @Test + public void testDeploymentCostStatsPath() { + StorageLayouts.useLayout(new TenantRootedStorageLayout(TENANT)); + + ResourceDescriptor descriptor = ResourceDescriptorFactory.fromDecoded(ResourceTypes.DEPLOYMENT_COST_STATS, + TokenStatsTracker.DEPLOYMENT_COST_STATS_BUCKET, TokenStatsTracker.DEPLOYMENT_COST_STATS_LOCATION, + "trace-id"); + + assertEquals(".system/deployment_cost_stats/.deployment_cost_stats/trace-id", descriptor.getAbsoluteFilePath()); + } + + /** + * A system bucket added elsewhere and not registered in {@link ResourceDescriptor#SYSTEM_LOCATIONS} is + * exactly the defect this class exists for, so pin that the locations these utilities use are the + * registered ones rather than parallel literals. + */ + @Test + public void testCallersUseRegisteredLocations() { + for (String location : List.of( + ResponseIdUtil.RESPONSE_MAPPINGS_BUCKET_LOCATION, + ResponseIdUtil.BACKGROUND_JOB_BUCKET_LOCATION, + TokenStatsTracker.DEPLOYMENT_COST_STATS_LOCATION, + ApiKeyStore.API_KEY_DATA_LOCATION)) { + assertTrue(ResourceDescriptor.SYSTEM_LOCATIONS.contains(location), + () -> location + " is not registered in ResourceDescriptor.SYSTEM_LOCATIONS"); + } + } +} diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java index caa6a8c98..9a6a0907c 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java @@ -23,6 +23,27 @@ public class ResourceDescriptor { public static final String PLATFORM_BUCKET = "platform"; public static final String PLATFORM_LOCATION = PLATFORM_BUCKET + PATH_SEPARATOR; + public static final String DEPLOYMENT_COST_STATS_BUCKET = "deployment_cost_stats"; + public static final String DEPLOYMENT_COST_STATS_LOCATION = DEPLOYMENT_COST_STATS_BUCKET + PATH_SEPARATOR; + public static final String BACKGROUND_JOB_BUCKET = "background_jobs"; + public static final String BACKGROUND_JOB_LOCATION = BACKGROUND_JOB_BUCKET + PATH_SEPARATOR; + public static final String RESPONSE_MAPPINGS_BUCKET = "response_mappings"; + public static final String RESPONSE_MAPPINGS_LOCATION = RESPONSE_MAPPINGS_BUCKET + PATH_SEPARATOR; + public static final String API_KEY_DATA_BUCKET = "api_key_data"; + public static final String API_KEY_DATA_LOCATION = API_KEY_DATA_BUCKET + PATH_SEPARATOR; + + /** + * Buckets holding platform-internal runtime state rather than anyone's content. They belong to no + * principal, so a layout has to place them somewhere other than the branches it uses for users and + * projects, and they are listed here so a layout can enumerate them. + * + *

A location that is not a principal, not public, not the platform and not in this set is rejected + * rather than passed through: a silent fallback would let the next such bucket reach production unmapped. + */ + public static final List SYSTEM_LOCATIONS = List.of( + DEPLOYMENT_COST_STATS_LOCATION, BACKGROUND_JOB_LOCATION, RESPONSE_MAPPINGS_LOCATION, + API_KEY_DATA_LOCATION); + ResourceType type; /** @@ -106,8 +127,32 @@ public String getDecodedUrl() { * Returns an absolute path to the resource in a persistent storage. */ public String getAbsoluteFilePath() { + return getStoragePrefix() + getPathWithinType(); + } + + /** + * Returns the same path as {@link #getAbsoluteFilePath()} would under the legacy layout, whichever layout + * is active. + * + *

Callers that bake a resource path into something durable — an identifier handed to a user, an + * encryption AAD — must use this rather than the physical path. A physical path is free to change when the + * layout changes; anything derived from one and then stored is not, or the stored thing stops resolving. + */ + public String getStableFilePath() { + return bucketLocation + type.group() + PATH_SEPARATOR + getPathWithinType(); + } + + /** + * Returns the layout-dependent prefix every physical path of this resource starts with: the bucket + * location followed by the resource-type folder. + */ + private String getStoragePrefix() { + StorageLayout layout = StorageLayouts.resolveActive(); + return layout.resolveLocationPrefix(bucketLocation) + layout.resolveTypeFolder(type.group()) + PATH_SEPARATOR; + } + + private String getPathWithinType() { StringBuilder builder = new StringBuilder(); - builder.append(getStoragePrefix()); if (!parentFolders.isEmpty()) { builder.append(getParentPath()) @@ -125,15 +170,6 @@ public String getAbsoluteFilePath() { return builder.toString(); } - /** - * Returns the layout-dependent prefix every physical path of this resource starts with: the bucket - * location followed by the resource-type folder. - */ - private String getStoragePrefix() { - StorageLayout layout = StorageLayouts.resolveActive(); - return layout.resolveLocationPrefix(bucketLocation) + layout.resolveTypeFolder(type.group()) + PATH_SEPARATOR; - } - /** * Returns the parent resource if any. */ diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java index b9a0b41bc..9005f3ce8 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java @@ -30,6 +30,18 @@ public class TenantLayoutTransform { */ private static final String PLATFORM_LOCATION = ""; + /** + * Where the system buckets of {@link ResourceDescriptor#SYSTEM_LOCATIONS} land — at the root, above any + * tenant, keeping the bucket name so the mapping stays reversible. + * + *

Above rather than inside a tenant because that is what preserves today's behaviour: the background + * job scheduler scans its bucket whole, and cost stats and response mappings are keyed by globally unique + * trace, job and response ids. Whether this state should become tenant-scoped is a real question — per + * tenant billing would want it to be — but it is a design decision for the phase that turns multi-tenancy + * on, not something to settle silently inside a path transform. + */ + private static final String SYSTEM_SEGMENT = ".system/"; + private static final char TYPE_FOLDER_MARKER = '.'; public String toTenantLocation(String legacyLocation, String tenantId) { @@ -37,6 +49,10 @@ public String toTenantLocation(String legacyLocation, String tenantId) { return PLATFORM_LOCATION; } + if (ResourceDescriptor.SYSTEM_LOCATIONS.contains(legacyLocation)) { + return SYSTEM_SEGMENT + legacyLocation; + } + String tenantRoot = tenantRoot(tenantId); if (ResourceDescriptor.PUBLIC_LOCATION.equals(legacyLocation)) { return tenantRoot; @@ -60,6 +76,14 @@ public String toLegacyLocation(String tenantLocation, String tenantId) { return ResourceDescriptor.PLATFORM_LOCATION; } + if (tenantLocation.startsWith(SYSTEM_SEGMENT)) { + String system = tenantLocation.substring(SYSTEM_SEGMENT.length()); + if (!ResourceDescriptor.SYSTEM_LOCATIONS.contains(system)) { + throw new IllegalArgumentException("Unknown system bucket location: " + tenantLocation); + } + return system; + } + String tenantRoot = tenantRoot(tenantId); if (!tenantLocation.startsWith(tenantRoot)) { throw new IllegalArgumentException("Location does not belong to tenant " + tenantId + ": " + tenantLocation); diff --git a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java index 34560c63f..4632d339f 100644 --- a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java +++ b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java @@ -50,6 +50,42 @@ public void testLocationRoundTrip() { } } + @Test + public void testSystemLocations() { + assertEquals(".system/background_jobs/", TenantLayoutTransform.toTenantLocation("background_jobs/", TENANT)); + assertEquals("background_jobs/", TenantLayoutTransform.toLegacyLocation(".system/background_jobs/", TENANT)); + } + + /** + * Every system bucket has to be mapped, not just the ones a test happened to name: an unmapped one throws + * at the point a path is composed, which takes out whatever subsystem owns it — that is how the background + * job scheduler and per-request cost accounting were found broken under this layout. + */ + @Test + public void testEverySystemLocationRoundTrips() { + for (String legacy : ResourceDescriptor.SYSTEM_LOCATIONS) { + String tenant = TenantLayoutTransform.toTenantLocation(legacy, TENANT); + assertEquals(legacy, TenantLayoutTransform.toLegacyLocation(tenant, TENANT)); + } + } + + /** + * System buckets sit above the tenant, so they must not move when the tenant does. + */ + @Test + public void testSystemLocationsAreTenantIndependent() { + for (String legacy : ResourceDescriptor.SYSTEM_LOCATIONS) { + assertEquals(TenantLayoutTransform.toTenantLocation(legacy, TENANT), + TenantLayoutTransform.toTenantLocation(legacy, "another-tenant")); + } + } + + @Test + public void testUnknownSystemLocationRejected() { + assertThrows(IllegalArgumentException.class, + () -> TenantLayoutTransform.toLegacyLocation(".system/not_a_system_bucket/", TENANT)); + } + @Test public void testUnsupportedLegacyLocation() { assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Unknown/u1/", TENANT)); diff --git a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java index bc87cab9a..1b693f35e 100644 --- a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java +++ b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java @@ -23,6 +23,32 @@ public void testTypeFolderIsReserved() { assertEquals(".conversations", layout.resolveTypeFolder("conversations")); } + @Test + public void testSystemLocationsResolve() { + assertEquals(".system/deployment_cost_stats/", layout.resolveLocationPrefix("deployment_cost_stats/")); + assertEquals(".system/background_jobs/", layout.resolveLocationPrefix("background_jobs/")); + assertEquals(".system/response_mappings/", layout.resolveLocationPrefix("response_mappings/")); + } + + /** + * The composed path for a system bucket, end to end. These buckets name themselves twice — the location + * and the resource type carry the same word — which the layout has to preserve rather than tidy up. + */ + @Test + public void testSystemBucketPath() { + StorageLayouts.useLayout(layout); + try { + ResourceDescriptor job = new ResourceDescriptor(ResourceTypes.BACKGROUND_JOB, "job-1", + java.util.List.of(), ResourceDescriptor.BACKGROUND_JOB_BUCKET, + ResourceDescriptor.BACKGROUND_JOB_LOCATION, false); + + assertEquals(".system/background_jobs/.background_jobs/job-1", job.getAbsoluteFilePath()); + assertEquals("background_jobs/background_jobs/job-1", job.getStableFilePath()); + } finally { + StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); + } + } + @Test public void testUnsupportedLocationRejected() { assertThrows(IllegalArgumentException.class, () -> layout.resolveLocationPrefix("Unknown/u1/")); From 7d67175307d5bef6dd6faa5c8829b6e18e541d74 Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Wed, 2 Sep 2026 17:30:19 +0300 Subject: [PATCH 02/10] fix: derive encrypted-field AAD from the stable path #1863 The AAD lives on inside stored ciphertext, so binding it to the physical path means a storage-layout change silently voids every stored secret: config entities drop out of the merged config as invalid, external-service and background-job keys stop decrypting. Bind it to the stable path instead, which is byte-identical under the legacy layout. Proven with real AES-GCM: a secret encrypted under the legacy layout decrypts after the flip, and physical-path ciphertext does not. Co-Authored-By: Claude Fable 5 --- .../server/config/SecretFieldProcessor.java | 15 +-- .../server/service/BackgroundJobService.java | 6 +- .../service/UserExternalServiceService.java | 6 +- .../SecretFieldProcessorLayoutTest.java | 97 +++++++++++++++++++ 4 files changed, 114 insertions(+), 10 deletions(-) create mode 100644 server/src/test/java/com/epam/aidial/core/server/config/SecretFieldProcessorLayoutTest.java diff --git a/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java b/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java index 02a4b7109..49cc30d73 100644 --- a/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java +++ b/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java @@ -40,16 +40,14 @@ public void encryptFields(Object entity, ResourceDescriptor descriptor) { if (entity == null) { return; } - byte[] aad = descriptor.getAbsoluteFilePath().getBytes(StandardCharsets.UTF_8); - walk(entity, aad, true); + walk(entity, aad(descriptor), true); } public void decryptFields(Object entity, ResourceDescriptor descriptor) { if (entity == null) { return; } - byte[] aad = descriptor.getAbsoluteFilePath().getBytes(StandardCharsets.UTF_8); - walk(entity, aad, false); + walk(entity, aad(descriptor), false); } public String resolveSecret(String value, ResourceDescriptor descriptor) { @@ -57,12 +55,17 @@ public String resolveSecret(String value, ResourceDescriptor descriptor) { return null; } if (value.startsWith(ENC_PREFIX) && value.endsWith(ENC_SUFFIX)) { - byte[] aad = descriptor.getAbsoluteFilePath().getBytes(StandardCharsets.UTF_8); - return decryptEnvelope(value, aad, "value"); + return decryptEnvelope(value, aad(descriptor), "value"); } return value; } + // The AAD outlives the process inside the stored ciphertext, so it must not depend on the active + // storage layout: physical paths move when the layout changes, the stable path never does. + private static byte[] aad(ResourceDescriptor descriptor) { + return descriptor.getStableFilePath().getBytes(StandardCharsets.UTF_8); + } + /** * Strip every {@link EncryptedField}-annotated value (and any nested array elements that carry * the annotation) from {@code payload}. Used to project invalid-entity payloads on the admin diff --git a/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java b/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java index 6bd5c69ba..15cb98983 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java @@ -272,16 +272,18 @@ private Future processResult( return Future.succeededFuture(); } + // Stable rather than physical path for the AAD: a job record outlives restarts, so its ciphertext + // must survive a storage-layout change. private String encryptKey(ResourceDescriptor descriptor, String key) { BucketInfo bucketInfo = new BucketInfo(descriptor.getBucketName(), descriptor.getBucketLocation()); - byte[] aad = descriptor.getAbsoluteFilePath().getBytes(StandardCharsets.UTF_8); + byte[] aad = descriptor.getStableFilePath().getBytes(StandardCharsets.UTF_8); byte[] cipher = encryptionService.encrypt(bucketInfo, key.getBytes(StandardCharsets.UTF_8), aad); return Base64.getEncoder().encodeToString(cipher); } private String decryptKey(ResourceDescriptor descriptor, String key) { BucketInfo bucketInfo = new BucketInfo(descriptor.getBucketName(), descriptor.getBucketLocation()); - byte[] aad = descriptor.getAbsoluteFilePath().getBytes(StandardCharsets.UTF_8); + byte[] aad = descriptor.getStableFilePath().getBytes(StandardCharsets.UTF_8); byte[] raw = Base64.getDecoder().decode(key); return new String(encryptionService.decrypt(bucketInfo, raw, aad), StandardCharsets.UTF_8); } diff --git a/server/src/main/java/com/epam/aidial/core/server/service/UserExternalServiceService.java b/server/src/main/java/com/epam/aidial/core/server/service/UserExternalServiceService.java index e88382d84..a3de30568 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/UserExternalServiceService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/UserExternalServiceService.java @@ -78,7 +78,9 @@ private static List applicationSegments(String appPart) { public ExternalService put(String ownerUserId, String appPart, String serviceId, ExternalService service, String author) { ResourceDescriptor resource = descriptor(ownerUserId, appPart, serviceId); BucketInfo bucket = new BucketInfo(resource.getBucketName(), resource.getBucketLocation()); - String aad = resource.getAbsoluteFilePath(); + // Stable rather than physical path: the AAD is baked into the stored ciphertext and must + // survive a storage-layout change. + String aad = resource.getStableFilePath(); MutableObject result = new MutableObject<>(); PersistedSecret persisted = new PersistedSecret(); resourceService.computeResource(resource, EtagHeader.ANY, author, json -> { @@ -104,7 +106,7 @@ public ExternalService get(String ownerUserId, String appPart, String serviceId) return null; } ExternalService service = ProxyUtil.convertToObject(stored.getValue(), ExternalService.class); - decryptSecret(resource.getAbsoluteFilePath(), new BucketInfo(resource.getBucketName(), resource.getBucketLocation()), service); + decryptSecret(resource.getStableFilePath(), new BucketInfo(resource.getBucketName(), resource.getBucketLocation()), service); return service; } diff --git a/server/src/test/java/com/epam/aidial/core/server/config/SecretFieldProcessorLayoutTest.java b/server/src/test/java/com/epam/aidial/core/server/config/SecretFieldProcessorLayoutTest.java new file mode 100644 index 000000000..ea07b837b --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/config/SecretFieldProcessorLayoutTest.java @@ -0,0 +1,97 @@ +package com.epam.aidial.core.server.config; + +import com.epam.aidial.core.config.Key; +import com.epam.aidial.core.credentials.data.configuration.EncryptionSettings; +import com.epam.aidial.core.credentials.data.credentials.BucketInfo; +import com.epam.aidial.core.credentials.encryption.ContentEncryptionKeyService; +import com.epam.aidial.core.credentials.encryption.CredentialEncryptionService; +import com.epam.aidial.core.credentials.encryption.DataEncryptionService; +import com.epam.aidial.core.server.util.ResourceDescriptorFactory; +import com.epam.aidial.core.storage.resource.LegacyStorageLayout; +import com.epam.aidial.core.storage.resource.ResourceDescriptor; +import com.epam.aidial.core.storage.resource.ResourceTypes; +import com.epam.aidial.core.storage.resource.StorageLayouts; +import com.epam.aidial.core.storage.resource.TenantRootedStorageLayout; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; +import java.security.SecureRandom; +import java.util.Base64; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * The AAD of an encrypted field lives on inside the stored ciphertext, so it must not change when the + * storage layout does. These tests use real AES-GCM — a mocked cipher ignores the AAD and would pass + * regardless of which path it was derived from. + */ +public class SecretFieldProcessorLayoutTest { + + private static final BucketInfo BUCKET = new BucketInfo("platform", "platform/"); + + private CredentialEncryptionService encryptionService; + private SecretFieldProcessor processor; + private ResourceDescriptor descriptor; + + @BeforeEach + public void setUp() { + EncryptionSettings settings = EncryptionSettings.builder() + .algorithm("AES") + .keySize(256) + .cipherTransformation("AES/GCM/NoPadding") + .ivLengthBytes(12) + .gcmTagLengthBits(128) + .build(); + SecureRandom random = new SecureRandom(); + byte[] contentEncryptionKey = new byte[32]; + random.nextBytes(contentEncryptionKey); + ContentEncryptionKeyService keyService = mock(ContentEncryptionKeyService.class); + when(keyService.getOrCreateKey(any(BucketInfo.class))).thenReturn(contentEncryptionKey); + + encryptionService = new CredentialEncryptionService(keyService, new DataEncryptionService(settings, random)); + processor = new SecretFieldProcessor(encryptionService, BUCKET); + descriptor = ResourceDescriptorFactory.fromDecoded( + ResourceTypes.PROJECT_KEY, "platform", "platform/", "test-key"); + } + + @AfterEach + public void restoreLegacyLayout() { + StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); + } + + @Test + public void testSecretEncryptedUnderLegacyLayoutDecryptsUnderTenantRootedLayout() { + Key key = new Key(); + key.setKey("plain-secret"); + processor.encryptFields(key, descriptor); + assertTrue(key.getKey().startsWith(SecretFieldProcessor.ENC_PREFIX)); + + StorageLayouts.useLayout(new TenantRootedStorageLayout("acme")); + processor.decryptFields(key, descriptor); + + assertEquals("plain-secret", key.getKey()); + } + + // Under the legacy layout the physical and stable paths coincide, so the divergence only exists on the + // tenant-rooted side: ciphertext bound to the tenant-shaped physical path must not decrypt against the + // stable AAD. This is also the guard proving the AAD participates at all — with a cipher that ignored + // it, the round-trip test above would pass for any path. + @Test + public void testPhysicalPathAadDoesNotMatchTheStableAad() { + StorageLayouts.useLayout(new TenantRootedStorageLayout("acme")); + + byte[] physicalPathAad = descriptor.getAbsoluteFilePath().getBytes(StandardCharsets.UTF_8); + byte[] cipher = encryptionService.encrypt(BUCKET, "plain-secret".getBytes(StandardCharsets.UTF_8), physicalPathAad); + Key key = new Key(); + key.setKey(SecretFieldProcessor.ENC_PREFIX + Base64.getEncoder().encodeToString(cipher) + SecretFieldProcessor.ENC_SUFFIX); + + assertThrows(SecurityException.class, () -> processor.decryptFields(key, descriptor)); + } +} From aee53baa9d27c74735fc013d7cca72a7d869da74 Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Wed, 2 Sep 2026 18:06:11 +0300 Subject: [PATCH 03/10] fix: map public sub-bucket locations in the tenant-rooted layout #1863 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ApplicationService keys a public function app's deployment folder as a synthesized sub-bucket, public/deployments//, which the transform rejected — every deploy of a public app threw under the tenant-rooted layout. Public sub-buckets now keep their legacy suffix under the tenant root; dotted segments are rejected so the mapping stays reversible, since dotted names there are reserved for principal branches and type folders. The private and review variants already worked via multi-segment principal ids and are pinned by the new test alongside the public one. Co-Authored-By: Claude Fable 5 --- .../ApplicationDeploymentLayoutTest.java | 57 +++++++++++++++++++ .../resource/TenantLayoutTransform.java | 31 +++++++++- .../resource/TenantLayoutTransformTest.java | 27 ++++++++- 3 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 server/src/test/java/com/epam/aidial/core/server/service/ApplicationDeploymentLayoutTest.java diff --git a/server/src/test/java/com/epam/aidial/core/server/service/ApplicationDeploymentLayoutTest.java b/server/src/test/java/com/epam/aidial/core/server/service/ApplicationDeploymentLayoutTest.java new file mode 100644 index 000000000..5be21198c --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/service/ApplicationDeploymentLayoutTest.java @@ -0,0 +1,57 @@ +package com.epam.aidial.core.server.service; + +import com.epam.aidial.core.server.util.ResourceDescriptorFactory; +import com.epam.aidial.core.storage.resource.LegacyStorageLayout; +import com.epam.aidial.core.storage.resource.ResourceDescriptor; +import com.epam.aidial.core.storage.resource.ResourceTypes; +import com.epam.aidial.core.storage.resource.StorageLayouts; +import com.epam.aidial.core.storage.resource.TenantRootedStorageLayout; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * {@code ApplicationService} keys a function app's deployment folder as a synthesized sub-bucket of the + * owner's location, so these shapes never pass through the bucket builder. Each of them has to compose a + * physical path under the tenant-rooted layout — the public one is how every deploy of a public function + * app came to throw under it. + */ +public class ApplicationDeploymentLayoutTest { + + @AfterEach + public void restoreLegacyLayout() { + StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); + } + + @Test + public void testPublicDeploymentFolder() { + StorageLayouts.useLayout(new TenantRootedStorageLayout("acme")); + + ResourceDescriptor folder = ResourceDescriptorFactory.fromDecoded( + ResourceTypes.FILE, "bucket", "public/deployments/fn-1/", null); + + assertEquals(".org/acme/deployments/fn-1/.files/", folder.getAbsoluteFilePath()); + assertEquals("public/deployments/fn-1/files/", folder.getStableFilePath()); + } + + @Test + public void testPrivateDeploymentFolder() { + StorageLayouts.useLayout(new TenantRootedStorageLayout("acme")); + + ResourceDescriptor folder = ResourceDescriptorFactory.fromDecoded( + ResourceTypes.FILE, "bucket", "Users/u1/deployments/fn-1/", null); + + assertEquals(".org/acme/.users/u1/deployments/fn-1/.files/", folder.getAbsoluteFilePath()); + } + + @Test + public void testReviewDeploymentFolder() { + StorageLayouts.useLayout(new TenantRootedStorageLayout("acme")); + + ResourceDescriptor folder = ResourceDescriptorFactory.fromDecoded( + ResourceTypes.FILE, "bucket", "Users/u1/publications/p1/deployments/fn-1/", null); + + assertEquals(".org/acme/.users/u1/publications/p1/deployments/fn-1/.files/", folder.getAbsoluteFilePath()); + } +} diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java index 9005f3ce8..91010c99f 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java @@ -54,8 +54,13 @@ public String toTenantLocation(String legacyLocation, String tenantId) { } String tenantRoot = tenantRoot(tenantId); - if (ResourceDescriptor.PUBLIC_LOCATION.equals(legacyLocation)) { - return tenantRoot; + if (legacyLocation.startsWith(ResourceDescriptor.PUBLIC_LOCATION)) { + // Not only "public/" itself: the platform synthesizes sub-buckets under it, e.g. a public + // function app's source folder is keyed as "public/deployments//". The suffix keeps its + // legacy shape under the tenant root. + String scope = legacyLocation.substring(ResourceDescriptor.PUBLIC_LOCATION.length()); + requirePublicScope(scope, legacyLocation); + return tenantRoot + scope; } String userId = principalId(legacyLocation, LEGACY_USERS_PREFIX); @@ -104,9 +109,31 @@ public String toLegacyLocation(String tenantLocation, String tenantId) { return LEGACY_KEYS_PREFIX + project; } + if (scope.charAt(0) != TYPE_FOLDER_MARKER) { + requirePublicScope(scope, tenantLocation); + return ResourceDescriptor.PUBLIC_LOCATION + scope; + } + throw new IllegalArgumentException("Unsupported tenant bucket location: " + tenantLocation); } + /** + * A public sub-bucket suffix must end at a path boundary, and none of its segments may start with + * the marker character: dotted names under the tenant root are reserved for principal branches and + * resource-type folders, so a dotted segment here would make the mapping irreversible. + */ + private void requirePublicScope(String scope, String location) { + if (scope.isEmpty()) { + return; + } + + if (!scope.endsWith(ResourceDescriptor.PATH_SEPARATOR) + || scope.charAt(0) == TYPE_FOLDER_MARKER + || scope.contains(ResourceDescriptor.PATH_SEPARATOR + TYPE_FOLDER_MARKER)) { + throw new IllegalArgumentException("Unsupported public bucket location: " + location); + } + } + public String toTenantTypeFolder(String legacyTypeFolder) { if (legacyTypeFolder.isEmpty() || legacyTypeFolder.charAt(0) == TYPE_FOLDER_MARKER) { throw new IllegalArgumentException("Unsupported legacy resource type folder: " + legacyTypeFolder); diff --git a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java index 4632d339f..4533ef83e 100644 --- a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java +++ b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java @@ -42,9 +42,34 @@ public void testMultiSegmentKeyLocation() { assertEquals(legacy, TenantLayoutTransform.toLegacyLocation(tenant, TENANT)); } + /** + * The platform synthesizes sub-buckets of {@code public/}: a public function app's source and target + * folder is keyed as {@code public/deployments//}. The suffix keeps its legacy shape under the + * tenant root — it stays parseable because reserved names there are dotted and the suffix may not be. + */ + @Test + public void testPublicDeploymentsLocation() { + assertEquals(".org/default-tenant/deployments/abc123/", + TenantLayoutTransform.toTenantLocation("public/deployments/abc123/", TENANT)); + assertEquals("public/deployments/abc123/", + TenantLayoutTransform.toLegacyLocation(".org/default-tenant/deployments/abc123/", TENANT)); + } + + @Test + public void testDottedOrUnterminatedPublicScopeRejected() { + assertThrows(IllegalArgumentException.class, + () -> TenantLayoutTransform.toTenantLocation("public/.deployments/abc123/", TENANT)); + assertThrows(IllegalArgumentException.class, + () -> TenantLayoutTransform.toTenantLocation("public/deployments/.abc123/", TENANT)); + assertThrows(IllegalArgumentException.class, + () -> TenantLayoutTransform.toTenantLocation("public/deployments/abc123", TENANT)); + assertThrows(IllegalArgumentException.class, + () -> TenantLayoutTransform.toLegacyLocation(".org/default-tenant/deployments/abc123", TENANT)); + } + @Test public void testLocationRoundTrip() { - for (String legacy : new String[] {"platform/", "public/", "Users/u1/", "Keys/proj/", "Keys/applications/abc/app/"}) { + for (String legacy : new String[] {"platform/", "public/", "public/deployments/abc123/", "Users/u1/", "Keys/proj/", "Keys/applications/abc/app/"}) { String tenant = TenantLayoutTransform.toTenantLocation(legacy, TENANT); assertEquals(legacy, TenantLayoutTransform.toLegacyLocation(tenant, TENANT)); } From 5e4ade558598fc44274023d839182c717bba2dcf Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Wed, 2 Sep 2026 18:22:39 +0300 Subject: [PATCH 04/10] fix: reject resource type folders that collide with reserved layout names #1863 The tenant-rooted tree gives .org/.system/.users/.keys and dotted type folders one namespace, and full-path parsing depends on the reserved names staying unambiguous. No ResourceTypes group takes one today, but nothing kept a future one off them; now the transform rejects the collision and the every-type round-trip test fails at build time. Co-Authored-By: Claude Fable 5 --- .../resource/TenantLayoutTransform.java | 20 ++++++++++++++++++- .../resource/TenantLayoutTransformTest.java | 12 +++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java index 91010c99f..fd377523c 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java @@ -2,6 +2,7 @@ import lombok.experimental.UtilityClass; +import java.util.Set; import javax.annotation.Nullable; /** @@ -44,6 +45,14 @@ public class TenantLayoutTransform { private static final char TYPE_FOLDER_MARKER = '.'; + /** + * The dotted names with a structural meaning in the tenant-rooted tree. A resource type whose group + * produced one of these as its type folder would make full paths unparseable — nothing distinguishes + * the public {@code .users} type folder from the {@code .users} principal branch — so such a group is + * rejected outright. Nothing else keeps a future {@code ResourceTypes} entry off these names. + */ + private static final Set RESERVED_SEGMENTS = Set.of(ORG_PREFIX, USERS_SEGMENT, KEYS_SEGMENT, SYSTEM_SEGMENT); + public String toTenantLocation(String legacyLocation, String tenantId) { if (ResourceDescriptor.PLATFORM_LOCATION.equals(legacyLocation)) { return PLATFORM_LOCATION; @@ -139,7 +148,9 @@ public String toTenantTypeFolder(String legacyTypeFolder) { throw new IllegalArgumentException("Unsupported legacy resource type folder: " + legacyTypeFolder); } - return TYPE_FOLDER_MARKER + legacyTypeFolder; + String folder = TYPE_FOLDER_MARKER + legacyTypeFolder; + requireUnreserved(folder, legacyTypeFolder); + return folder; } public String toLegacyTypeFolder(String tenantTypeFolder) { @@ -147,9 +158,16 @@ public String toLegacyTypeFolder(String tenantTypeFolder) { throw new IllegalArgumentException("Unsupported tenant resource type folder: " + tenantTypeFolder); } + requireUnreserved(tenantTypeFolder, tenantTypeFolder); return tenantTypeFolder.substring(1); } + private void requireUnreserved(String dottedFolder, String input) { + if (RESERVED_SEGMENTS.contains(dottedFolder + ResourceDescriptor.PATH_SEPARATOR)) { + throw new IllegalArgumentException("Resource type folder collides with a reserved name: " + input); + } + } + private String tenantRoot(String tenantId) { if (tenantId.isEmpty()) { throw new IllegalArgumentException("Tenant id must not be empty"); diff --git a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java index 4533ef83e..7922bc2b2 100644 --- a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java +++ b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java @@ -151,6 +151,18 @@ public void testTypeFolderRoundTripsForEveryResourceType() { } } + /** + * With the guard in place, {@link #testTypeFolderRoundTripsForEveryResourceType} doubles as the + * build-time tripwire: a future {@code ResourceTypes} group taking a reserved name fails that test. + */ + @Test + public void testReservedTypeFolderNamesRejected() { + for (String reserved : new String[] {"org", "system", "users", "keys"}) { + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantTypeFolder(reserved)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toLegacyTypeFolder("." + reserved)); + } + } + @Test public void testUnsupportedTypeFolder() { assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantTypeFolder("")); From ef6c0e53107c5f56e914b236819bc52671139423 Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Thu, 3 Sep 2026 13:46:08 +0300 Subject: [PATCH 05/10] refactor: one vocabulary for locations, one rule for path composition #1863 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on the seam, applied in place: - The Users/ and Keys/ prefixes lived twice — formatted by BucketBuilder, re-declared by the transform "because this module cannot depend on it". Both now reference ResourceDescriptor, the same cure the system-bucket literals got. - getStableFilePath re-inlined the composition rule the seam centralises; both paths now go through getStoragePrefix(layout), so the stable path is the legacy layout's composition by construction, not by comment. - Principal ids now reject dotted segments the way public scopes already did: ".." escapes the tree, and dotted names are reserved. - BackgroundJobService derives its AAD through one helper, mirroring SecretFieldProcessor; SYSTEM_LOCATIONS is a Set (membership, no order); stray blank lines dropped. Co-Authored-By: Claude Fable 5 --- .../server/service/BackgroundJobService.java | 14 +++++----- .../core/server/token/TokenStatsTracker.java | 1 - .../core/server/util/BucketBuilder.java | 7 +++-- .../storage/resource/ResourceDescriptor.java | 25 ++++++++++------- .../resource/TenantLayoutTransform.java | 27 ++++++++++++------- .../resource/TenantLayoutTransformTest.java | 14 ++++++++++ 6 files changed, 61 insertions(+), 27 deletions(-) diff --git a/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java b/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java index 15cb98983..447c6936a 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java @@ -272,20 +272,22 @@ private Future processResult( return Future.succeededFuture(); } - // Stable rather than physical path for the AAD: a job record outlives restarts, so its ciphertext - // must survive a storage-layout change. private String encryptKey(ResourceDescriptor descriptor, String key) { BucketInfo bucketInfo = new BucketInfo(descriptor.getBucketName(), descriptor.getBucketLocation()); - byte[] aad = descriptor.getStableFilePath().getBytes(StandardCharsets.UTF_8); - byte[] cipher = encryptionService.encrypt(bucketInfo, key.getBytes(StandardCharsets.UTF_8), aad); + byte[] cipher = encryptionService.encrypt(bucketInfo, key.getBytes(StandardCharsets.UTF_8), aad(descriptor)); return Base64.getEncoder().encodeToString(cipher); } private String decryptKey(ResourceDescriptor descriptor, String key) { BucketInfo bucketInfo = new BucketInfo(descriptor.getBucketName(), descriptor.getBucketLocation()); - byte[] aad = descriptor.getStableFilePath().getBytes(StandardCharsets.UTF_8); byte[] raw = Base64.getDecoder().decode(key); - return new String(encryptionService.decrypt(bucketInfo, raw, aad), StandardCharsets.UTF_8); + return new String(encryptionService.decrypt(bucketInfo, raw, aad(descriptor)), StandardCharsets.UTF_8); + } + + // Stable rather than physical path for the AAD: a job record outlives restarts, so its ciphertext + // must survive a storage-layout change. + private static byte[] aad(ResourceDescriptor descriptor) { + return descriptor.getStableFilePath().getBytes(StandardCharsets.UTF_8); } private Future completeAndProcess( diff --git a/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java b/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java index ab577e381..2dcff91bc 100644 --- a/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java +++ b/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java @@ -21,7 +21,6 @@ import java.util.List; import java.util.Map; - @Slf4j @RequiredArgsConstructor public class TokenStatsTracker { diff --git a/server/src/main/java/com/epam/aidial/core/server/util/BucketBuilder.java b/server/src/main/java/com/epam/aidial/core/server/util/BucketBuilder.java index b9a3124c7..febe449e9 100644 --- a/server/src/main/java/com/epam/aidial/core/server/util/BucketBuilder.java +++ b/server/src/main/java/com/epam/aidial/core/server/util/BucketBuilder.java @@ -3,6 +3,7 @@ import com.epam.aidial.core.server.ProxyContext; import com.epam.aidial.core.server.data.AuthBucket; import com.epam.aidial.core.server.security.EncryptionService; +import com.epam.aidial.core.storage.resource.ResourceDescriptor; import lombok.experimental.UtilityClass; import java.util.Objects; @@ -12,8 +13,10 @@ public class BucketBuilder { public static final String APPDATA_PATTERN = "appdata/%s"; - public static final String USER_BUCKET_PATTERN = "Users/%s/"; - public static final String API_KEY_BUCKET_PATTERN = "Keys/%s/"; + // Prefixes declared in ResourceDescriptor: a storage layout recognizes principal locations by the + // same literals this builder formats them with. + public static final String USER_BUCKET_PATTERN = ResourceDescriptor.USERS_LOCATION_PREFIX + "%s" + ResourceDescriptor.PATH_SEPARATOR; + public static final String API_KEY_BUCKET_PATTERN = ResourceDescriptor.KEYS_LOCATION_PREFIX + "%s" + ResourceDescriptor.PATH_SEPARATOR; public String buildUserBucket(ProxyContext context) { if (context.getApiKeyData().getPerRequestKey() == null) { diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java index 9a6a0907c..46d99dd31 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java @@ -8,6 +8,7 @@ import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nullable; @@ -23,6 +24,14 @@ public class ResourceDescriptor { public static final String PLATFORM_BUCKET = "platform"; public static final String PLATFORM_LOCATION = PLATFORM_BUCKET + PATH_SEPARATOR; + /** + * Prefixes of the two principal bucket-location shapes, {@code Users//} and {@code Keys//}. + * The server-side bucket builder formats locations with them, and a storage layout recognizes principal + * locations by them — one set of literals for both, or the two drift. + */ + public static final String USERS_LOCATION_PREFIX = "Users" + PATH_SEPARATOR; + public static final String KEYS_LOCATION_PREFIX = "Keys" + PATH_SEPARATOR; + public static final String DEPLOYMENT_COST_STATS_BUCKET = "deployment_cost_stats"; public static final String DEPLOYMENT_COST_STATS_LOCATION = DEPLOYMENT_COST_STATS_BUCKET + PATH_SEPARATOR; public static final String BACKGROUND_JOB_BUCKET = "background_jobs"; @@ -40,11 +49,10 @@ public class ResourceDescriptor { *

A location that is not a principal, not public, not the platform and not in this set is rejected * rather than passed through: a silent fallback would let the next such bucket reach production unmapped. */ - public static final List SYSTEM_LOCATIONS = List.of( + public static final Set SYSTEM_LOCATIONS = Set.of( DEPLOYMENT_COST_STATS_LOCATION, BACKGROUND_JOB_LOCATION, RESPONSE_MAPPINGS_LOCATION, API_KEY_DATA_LOCATION); - ResourceType type; /** * Resource's name or empty if the resource is a folder @@ -127,11 +135,11 @@ public String getDecodedUrl() { * Returns an absolute path to the resource in a persistent storage. */ public String getAbsoluteFilePath() { - return getStoragePrefix() + getPathWithinType(); + return getStoragePrefix(StorageLayouts.resolveActive()) + getPathWithinType(); } /** - * Returns the same path as {@link #getAbsoluteFilePath()} would under the legacy layout, whichever layout + * Returns the path {@link #getAbsoluteFilePath()} produces under the legacy layout, whichever layout * is active. * *

Callers that bake a resource path into something durable — an identifier handed to a user, an @@ -139,15 +147,14 @@ public String getAbsoluteFilePath() { * layout changes; anything derived from one and then stored is not, or the stored thing stops resolving. */ public String getStableFilePath() { - return bucketLocation + type.group() + PATH_SEPARATOR + getPathWithinType(); + return getStoragePrefix(LegacyStorageLayout.INSTANCE) + getPathWithinType(); } /** - * Returns the layout-dependent prefix every physical path of this resource starts with: the bucket + * Returns the prefix every path of this resource starts with under the given layout: the bucket * location followed by the resource-type folder. */ - private String getStoragePrefix() { - StorageLayout layout = StorageLayouts.resolveActive(); + private String getStoragePrefix(StorageLayout layout) { return layout.resolveLocationPrefix(bucketLocation) + layout.resolveTypeFolder(type.group()) + PATH_SEPARATOR; } @@ -254,7 +261,7 @@ public ResourceDescriptor resolveByUrl(String url) { * @param path - to the resource with decrypted bucket */ public ResourceDescriptor resolveByPath(String path) { - String prefix = getStoragePrefix(); + String prefix = getStoragePrefix(StorageLayouts.resolveActive()); if (!isFolder) { throw new IllegalStateException("Resource must be a folder"); } diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java index fd377523c..779016c17 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java @@ -12,15 +12,12 @@ * *

The conversion is total and reversible in both directions, so a migrated path can always be mapped * back to its origin. - * - *

Legacy locations are produced by the server-side bucket builder; the prefixes are repeated here - * because this module cannot depend on it. */ @UtilityClass public class TenantLayoutTransform { - private static final String LEGACY_USERS_PREFIX = "Users/"; - private static final String LEGACY_KEYS_PREFIX = "Keys/"; + private static final String LEGACY_USERS_PREFIX = ResourceDescriptor.USERS_LOCATION_PREFIX; + private static final String LEGACY_KEYS_PREFIX = ResourceDescriptor.KEYS_LOCATION_PREFIX; private static final String ORG_PREFIX = ".org/"; private static final String USERS_SEGMENT = ".users/"; @@ -136,9 +133,7 @@ private void requirePublicScope(String scope, String location) { return; } - if (!scope.endsWith(ResourceDescriptor.PATH_SEPARATOR) - || scope.charAt(0) == TYPE_FOLDER_MARKER - || scope.contains(ResourceDescriptor.PATH_SEPARATOR + TYPE_FOLDER_MARKER)) { + if (!scope.endsWith(ResourceDescriptor.PATH_SEPARATOR) || hasDottedSegment(scope)) { throw new IllegalArgumentException("Unsupported public bucket location: " + location); } } @@ -188,6 +183,20 @@ private String principalId(String location, String prefix) { } String id = location.substring(prefix.length()); - return id.length() > 1 && id.endsWith(ResourceDescriptor.PATH_SEPARATOR) ? id : null; + if (id.length() <= 1 || !id.endsWith(ResourceDescriptor.PATH_SEPARATOR)) { + return null; + } + + // A dotted segment would escape the tree ("..") or collide with the dotted names reserved for + // type folders and principal branches; no legitimate producer emits one. + if (hasDottedSegment(id)) { + throw new IllegalArgumentException("Unsupported principal id in location: " + location); + } + return id; + } + + private boolean hasDottedSegment(String path) { + return path.charAt(0) == TYPE_FOLDER_MARKER + || path.contains(ResourceDescriptor.PATH_SEPARATOR + TYPE_FOLDER_MARKER); } } diff --git a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java index 7922bc2b2..4059679d1 100644 --- a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java +++ b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java @@ -111,6 +111,20 @@ public void testUnknownSystemLocationRejected() { () -> TenantLayoutTransform.toLegacyLocation(".system/not_a_system_bucket/", TENANT)); } + /** + * A dotted principal-id segment would escape the tree ("..") or collide with the dotted names + * reserved for type folders and principal branches. No legitimate producer emits one, so the + * transform rejects rather than composes. + */ + @Test + public void testDottedPrincipalSegmentsRejected() { + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Users/../", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Users/.evil/", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Keys/applications/../app/", TENANT)); + assertThrows(IllegalArgumentException.class, + () -> TenantLayoutTransform.toLegacyLocation(".org/default-tenant/.users/../", TENANT)); + } + @Test public void testUnsupportedLegacyLocation() { assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Unknown/u1/", TENANT)); From 5743da278509078d6033b026cccbf70c69ba802a Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Thu, 3 Sep 2026 14:52:00 +0300 Subject: [PATCH 06/10] =?UTF-8?q?refactor:=20second=20review=20round=20on?= =?UTF-8?q?=20the=20seam=20=E2=80=94=20validation,=20aliases,=20test=20gap?= =?UTF-8?q?s=20#1863?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production: - Tenant ids are now validated by the same rule the transform applies to everything else: a separator would nest one tenant's root inside another's, a leading dot collides with reserved names. Checked at construction and on every composition. - The constant re-export aliases are gone — a pattern with no precedent here, two public names per value, and the naming had already drifted (_BUCKET_LOCATION vs _LOCATION). Callers reference ResourceDescriptor. - The transform's PLATFORM_LOCATION ("") no longer shadows ResourceDescriptor.PLATFORM_LOCATION ("platform/"): renamed to TENANT_TREE_ROOT. - One vocabulary for the type-group parameter across the interface and both layouts; javadoc trimmed to contracts; the rejection policy moved onto toTenantLocation, where it is enforced. Tests: - ApplicationDeploymentLayoutTest now composes locations through ApplicationService.deploymentFolderLocation instead of pinning literals that could drift from what production synthesizes. - Closed branch gaps: dotted rejections in the tenant-to-legacy direction, the Users// boundary, resolveByPath under the tenant layout, invalid tenant ids both at the transform and the constructor. - Layout flips moved to @BeforeEach, the lone try/finally restore aligned with the @AfterEach pattern, dead marker indirection inlined, war-story comments trimmed to their invariants. Co-Authored-By: Claude Fable 5 --- .../server/config/SecretFieldProcessor.java | 2 - .../core/server/security/ApiKeyStore.java | 7 +-- .../server/service/ApplicationService.java | 13 +++-- .../server/service/BackgroundJobService.java | 2 - .../service/ResponseMappingService.java | 4 +- .../service/UserExternalServiceService.java | 2 - .../core/server/token/TokenStatsTracker.java | 7 +-- .../core/server/util/ResponseIdUtil.java | 10 +--- .../server/TenantRootedLayoutApiTest.java | 11 ++-- .../ApplicationDeploymentLayoutTest.java | 30 ++++++----- .../core/server/util/ResponseIdUtilTest.java | 4 +- .../server/util/SystemBucketLayoutTest.java | 49 ++++-------------- .../storage/resource/LegacyStorageLayout.java | 4 +- .../storage/resource/ResourceDescriptor.java | 16 ++---- .../core/storage/resource/StorageLayout.java | 2 +- .../resource/TenantLayoutTransform.java | 50 ++++++++++-------- .../resource/TenantRootedStorageLayout.java | 6 ++- .../resource/TenantLayoutTransformTest.java | 21 +++++++- .../TenantRootedStorageLayoutTest.java | 51 +++++++++++++++---- 19 files changed, 147 insertions(+), 144 deletions(-) diff --git a/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java b/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java index 49cc30d73..15fee1e59 100644 --- a/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java +++ b/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java @@ -60,8 +60,6 @@ public String resolveSecret(String value, ResourceDescriptor descriptor) { return value; } - // The AAD outlives the process inside the stored ciphertext, so it must not depend on the active - // storage layout: physical paths move when the layout changes, the stable path never does. private static byte[] aad(ResourceDescriptor descriptor) { return descriptor.getStableFilePath().getBytes(StandardCharsets.UTF_8); } diff --git a/server/src/main/java/com/epam/aidial/core/server/security/ApiKeyStore.java b/server/src/main/java/com/epam/aidial/core/server/security/ApiKeyStore.java index 937598043..c82757163 100644 --- a/server/src/main/java/com/epam/aidial/core/server/security/ApiKeyStore.java +++ b/server/src/main/java/com/epam/aidial/core/server/security/ApiKeyStore.java @@ -28,7 +28,6 @@ import java.util.function.Function; import static com.epam.aidial.core.server.security.ApiKeyGenerator.generateKey; -import static com.epam.aidial.core.storage.resource.ResourceDescriptor.PATH_SEPARATOR; /** * The store keeps per request and project API key data. @@ -40,10 +39,6 @@ @Slf4j public class ApiKeyStore { - // Declared in ResourceDescriptor so a storage layout can enumerate the system buckets. - public static final String API_KEY_DATA_BUCKET = ResourceDescriptor.API_KEY_DATA_BUCKET; - public static final String API_KEY_DATA_LOCATION = ResourceDescriptor.API_KEY_DATA_LOCATION; - private final AsyncTaskExecutor taskExecutor; private final RedissonClient redis; private final String prefix; @@ -272,7 +267,7 @@ private void validateProjectKey(Key key) { private String toRedisKey(String apiKey) { ResourceDescriptor resource = ResourceDescriptorFactory.fromDecoded( - ResourceTypes.API_KEY_DATA, API_KEY_DATA_BUCKET, API_KEY_DATA_LOCATION, apiKey); + ResourceTypes.API_KEY_DATA, ResourceDescriptor.API_KEY_DATA_BUCKET, ResourceDescriptor.API_KEY_DATA_LOCATION, apiKey); return RedisUtil.redisKey(resource, prefix); } diff --git a/server/src/main/java/com/epam/aidial/core/server/service/ApplicationService.java b/server/src/main/java/com/epam/aidial/core/server/service/ApplicationService.java index dd4fbbdd8..e6b792050 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/ApplicationService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/ApplicationService.java @@ -814,14 +814,19 @@ private String deploymentLockKey(ResourceDescriptor resource) { } private String encodeTargetFolder(ResourceDescriptor resource, String id) { - String location = resource.getBucketLocation() - + DEPLOYMENTS_NAME + ResourceDescriptor.PATH_SEPARATOR - + id + ResourceDescriptor.PATH_SEPARATOR; - + String location = deploymentFolderLocation(resource.getBucketLocation(), id); String name = encryptionService.encrypt(location); return ResourceDescriptorFactory.fromDecoded(ResourceTypes.FILE, name, location, null).getUrl(); } + /** + * The synthesized sub-bucket location a function's deployment folder is keyed by. Package-visible so the + * layout test composes the same shape the service does rather than pinning a literal that can drift. + */ + static String deploymentFolderLocation(String bucketLocation, String id) { + return bucketLocation + DEPLOYMENTS_NAME + ResourceDescriptor.PATH_SEPARATOR + id + ResourceDescriptor.PATH_SEPARATOR; + } + public static boolean isActive(Application application) { return application != null && application.getFunction() != null && application.getFunction().getStatus().isActive(); } diff --git a/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java b/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java index 447c6936a..b460971b9 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java @@ -284,8 +284,6 @@ private String decryptKey(ResourceDescriptor descriptor, String key) { return new String(encryptionService.decrypt(bucketInfo, raw, aad(descriptor)), StandardCharsets.UTF_8); } - // Stable rather than physical path for the AAD: a job record outlives restarts, so its ciphertext - // must survive a storage-layout change. private static byte[] aad(ResourceDescriptor descriptor) { return descriptor.getStableFilePath().getBytes(StandardCharsets.UTF_8); } diff --git a/server/src/main/java/com/epam/aidial/core/server/service/ResponseMappingService.java b/server/src/main/java/com/epam/aidial/core/server/service/ResponseMappingService.java index 45a860073..227495864 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/ResponseMappingService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/ResponseMappingService.java @@ -67,7 +67,7 @@ private Void cleanExpiredMappings() { log.debug("Housekeeping: scanning for expired response mappings"); try { ResourceDescriptor root = ResourceDescriptorFactory.fromDecoded( - ResourceTypes.RESPONSE_MAPPING, ResponseIdUtil.RESPONSE_MAPPINGS_BUCKET, ResponseIdUtil.RESPONSE_MAPPINGS_BUCKET_LOCATION, null); + ResourceTypes.RESPONSE_MAPPING, ResourceDescriptor.RESPONSE_MAPPINGS_BUCKET, ResourceDescriptor.RESPONSE_MAPPINGS_LOCATION, null); cleanDeploymentSubfolders(root); } catch (Throwable e) { log.warn("Housekeeping: failed to clean expired response mappings", e); @@ -96,7 +96,7 @@ private void cleanDeploymentSubfolders(ResourceDescriptor root) { private void cleanItemsInDeploymentFolder(String deploymentName) { ResourceDescriptor subfolder = ResourceDescriptorFactory.fromDecoded( - ResourceTypes.RESPONSE_MAPPING, ResponseIdUtil.RESPONSE_MAPPINGS_BUCKET, ResponseIdUtil.RESPONSE_MAPPINGS_BUCKET_LOCATION, deploymentName + "/"); + ResourceTypes.RESPONSE_MAPPING, ResourceDescriptor.RESPONSE_MAPPINGS_BUCKET, ResourceDescriptor.RESPONSE_MAPPINGS_LOCATION, deploymentName + "/"); long now = System.currentTimeMillis(); String token = null; diff --git a/server/src/main/java/com/epam/aidial/core/server/service/UserExternalServiceService.java b/server/src/main/java/com/epam/aidial/core/server/service/UserExternalServiceService.java index a3de30568..f3b5afa8e 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/UserExternalServiceService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/UserExternalServiceService.java @@ -78,8 +78,6 @@ private static List applicationSegments(String appPart) { public ExternalService put(String ownerUserId, String appPart, String serviceId, ExternalService service, String author) { ResourceDescriptor resource = descriptor(ownerUserId, appPart, serviceId); BucketInfo bucket = new BucketInfo(resource.getBucketName(), resource.getBucketLocation()); - // Stable rather than physical path: the AAD is baked into the stored ciphertext and must - // survive a storage-layout change. String aad = resource.getStableFilePath(); MutableObject result = new MutableObject<>(); PersistedSecret persisted = new PersistedSecret(); diff --git a/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java b/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java index 2dcff91bc..0465f60a7 100644 --- a/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java +++ b/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java @@ -24,11 +24,6 @@ @Slf4j @RequiredArgsConstructor public class TokenStatsTracker { - // Declared in ResourceDescriptor so a storage layout can enumerate the system buckets; a second literal - // here is how the tenant-rooted layout came to reject this location in the first place. - public static final String DEPLOYMENT_COST_STATS_BUCKET = ResourceDescriptor.DEPLOYMENT_COST_STATS_BUCKET; - public static final String DEPLOYMENT_COST_STATS_LOCATION = ResourceDescriptor.DEPLOYMENT_COST_STATS_LOCATION; - private final AsyncTaskExecutor taskExecutor; private final ResourceService resourceService; @@ -216,6 +211,6 @@ public record UsageStats(TokenUsage total, List usagePerModel) { private static ResourceDescriptor toResource(String traceId) { return ResourceDescriptorFactory.fromDecoded( - ResourceTypes.DEPLOYMENT_COST_STATS, DEPLOYMENT_COST_STATS_BUCKET, DEPLOYMENT_COST_STATS_LOCATION, traceId); + ResourceTypes.DEPLOYMENT_COST_STATS, ResourceDescriptor.DEPLOYMENT_COST_STATS_BUCKET, ResourceDescriptor.DEPLOYMENT_COST_STATS_LOCATION, traceId); } } diff --git a/server/src/main/java/com/epam/aidial/core/server/util/ResponseIdUtil.java b/server/src/main/java/com/epam/aidial/core/server/util/ResponseIdUtil.java index d812d4246..1c7a58946 100644 --- a/server/src/main/java/com/epam/aidial/core/server/util/ResponseIdUtil.java +++ b/server/src/main/java/com/epam/aidial/core/server/util/ResponseIdUtil.java @@ -6,12 +6,6 @@ @UtilityClass public class ResponseIdUtil { - // Declared in ResourceDescriptor so a storage layout can enumerate the system buckets; a second literal - // here is how the tenant-rooted layout came to reject these locations in the first place. - public static final String RESPONSE_MAPPINGS_BUCKET = ResourceDescriptor.RESPONSE_MAPPINGS_BUCKET; - public static final String RESPONSE_MAPPINGS_BUCKET_LOCATION = ResourceDescriptor.RESPONSE_MAPPINGS_LOCATION; - public static final String BACKGROUND_JOB_BUCKET = ResourceDescriptor.BACKGROUND_JOB_BUCKET; - public static final String BACKGROUND_JOB_BUCKET_LOCATION = ResourceDescriptor.BACKGROUND_JOB_LOCATION; public static final String RESPONSE_ID_PREFIX = "dial_"; public String createResponseId(String deploymentName, String uuid) { @@ -30,11 +24,11 @@ public ResourceDescriptor getResponseMappingDescriptor(String dialResponseId) { String uuid = dialResponseId.substring(underscore + 1); String relativePath = deploymentName + "/" + uuid; return ResourceDescriptorFactory.fromDecoded( - ResourceTypes.RESPONSE_MAPPING, RESPONSE_MAPPINGS_BUCKET, RESPONSE_MAPPINGS_BUCKET_LOCATION, relativePath); + ResourceTypes.RESPONSE_MAPPING, ResourceDescriptor.RESPONSE_MAPPINGS_BUCKET, ResourceDescriptor.RESPONSE_MAPPINGS_LOCATION, relativePath); } public ResourceDescriptor getBackgroundJobDescriptor(String jobId) { return ResourceDescriptorFactory.fromDecoded( - ResourceTypes.BACKGROUND_JOB, BACKGROUND_JOB_BUCKET, BACKGROUND_JOB_BUCKET_LOCATION, jobId); + ResourceTypes.BACKGROUND_JOB, ResourceDescriptor.BACKGROUND_JOB_BUCKET, ResourceDescriptor.BACKGROUND_JOB_LOCATION, jobId); } } diff --git a/server/src/test/java/com/epam/aidial/core/server/TenantRootedLayoutApiTest.java b/server/src/test/java/com/epam/aidial/core/server/TenantRootedLayoutApiTest.java index 72c3d4589..fc0c2def1 100644 --- a/server/src/test/java/com/epam/aidial/core/server/TenantRootedLayoutApiTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/TenantRootedLayoutApiTest.java @@ -14,6 +14,7 @@ import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -66,21 +67,19 @@ public void testBlobIsStoredUnderTenantRoot() throws IOException { Response flushed = resourceRequest(HttpMethod.GET, "/folder/conversation"); assertEquals(200, flushed.status()); - List storedPaths = findStoredPaths(""); + List storedPaths = findStoredPaths(); List tenantRootedPaths = storedPaths.stream() .filter(path -> path.toString().contains(".org/" + TENANT)) .toList(); - assertTrue(!tenantRootedPaths.isEmpty(), + assertFalse(tenantRootedPaths.isEmpty(), () -> "No blob stored under the tenant root, found: " + storedPaths); assertTrue(tenantRootedPaths.stream().anyMatch(path -> path.toString().contains(".conversations")), () -> "Conversations are not stored in a reserved type folder: " + tenantRootedPaths); } - private List findStoredPaths(String marker) throws IOException { + private List findStoredPaths() throws IOException { try (Stream paths = Files.walk(testDir)) { - return paths.filter(Files::isRegularFile) - .filter(path -> path.toString().contains(marker)) - .toList(); + return paths.filter(Files::isRegularFile).toList(); } } } diff --git a/server/src/test/java/com/epam/aidial/core/server/service/ApplicationDeploymentLayoutTest.java b/server/src/test/java/com/epam/aidial/core/server/service/ApplicationDeploymentLayoutTest.java index 5be21198c..e66158448 100644 --- a/server/src/test/java/com/epam/aidial/core/server/service/ApplicationDeploymentLayoutTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/service/ApplicationDeploymentLayoutTest.java @@ -7,6 +7,7 @@ import com.epam.aidial.core.storage.resource.StorageLayouts; import com.epam.aidial.core.storage.resource.TenantRootedStorageLayout; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -14,11 +15,16 @@ /** * {@code ApplicationService} keys a function app's deployment folder as a synthesized sub-bucket of the * owner's location, so these shapes never pass through the bucket builder. Each of them has to compose a - * physical path under the tenant-rooted layout — the public one is how every deploy of a public function - * app came to throw under it. + * physical path under the tenant-rooted layout, and the locations are composed by the service itself — + * a literal here could drift from what production synthesizes. */ public class ApplicationDeploymentLayoutTest { + @BeforeEach + public void useTenantRootedLayout() { + StorageLayouts.useLayout(new TenantRootedStorageLayout("acme")); + } + @AfterEach public void restoreLegacyLayout() { StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); @@ -26,10 +32,7 @@ public void restoreLegacyLayout() { @Test public void testPublicDeploymentFolder() { - StorageLayouts.useLayout(new TenantRootedStorageLayout("acme")); - - ResourceDescriptor folder = ResourceDescriptorFactory.fromDecoded( - ResourceTypes.FILE, "bucket", "public/deployments/fn-1/", null); + ResourceDescriptor folder = deploymentFolder(ResourceDescriptor.PUBLIC_LOCATION); assertEquals(".org/acme/deployments/fn-1/.files/", folder.getAbsoluteFilePath()); assertEquals("public/deployments/fn-1/files/", folder.getStableFilePath()); @@ -37,21 +40,20 @@ public void testPublicDeploymentFolder() { @Test public void testPrivateDeploymentFolder() { - StorageLayouts.useLayout(new TenantRootedStorageLayout("acme")); - - ResourceDescriptor folder = ResourceDescriptorFactory.fromDecoded( - ResourceTypes.FILE, "bucket", "Users/u1/deployments/fn-1/", null); + ResourceDescriptor folder = deploymentFolder(ResourceDescriptor.USERS_LOCATION_PREFIX + "u1/"); assertEquals(".org/acme/.users/u1/deployments/fn-1/.files/", folder.getAbsoluteFilePath()); } @Test public void testReviewDeploymentFolder() { - StorageLayouts.useLayout(new TenantRootedStorageLayout("acme")); - - ResourceDescriptor folder = ResourceDescriptorFactory.fromDecoded( - ResourceTypes.FILE, "bucket", "Users/u1/publications/p1/deployments/fn-1/", null); + ResourceDescriptor folder = deploymentFolder(ResourceDescriptor.USERS_LOCATION_PREFIX + "u1/publications/p1/"); assertEquals(".org/acme/.users/u1/publications/p1/deployments/fn-1/.files/", folder.getAbsoluteFilePath()); } + + private static ResourceDescriptor deploymentFolder(String ownerLocation) { + String location = ApplicationService.deploymentFolderLocation(ownerLocation, "fn-1"); + return ResourceDescriptorFactory.fromDecoded(ResourceTypes.FILE, "bucket", location, null); + } } diff --git a/server/src/test/java/com/epam/aidial/core/server/util/ResponseIdUtilTest.java b/server/src/test/java/com/epam/aidial/core/server/util/ResponseIdUtilTest.java index 7ad08e1de..c205faabe 100644 --- a/server/src/test/java/com/epam/aidial/core/server/util/ResponseIdUtilTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/util/ResponseIdUtilTest.java @@ -28,8 +28,8 @@ public void testGetResponseMappingDescriptor() { ResourceDescriptor descriptor = ResponseIdUtil.getResponseMappingDescriptor("dial_gpt-4_abc123"); assertEquals(ResourceTypes.RESPONSE_MAPPING, descriptor.getType()); - assertEquals(ResponseIdUtil.RESPONSE_MAPPINGS_BUCKET, descriptor.getBucketName()); - assertEquals(ResponseIdUtil.RESPONSE_MAPPINGS_BUCKET_LOCATION, descriptor.getBucketLocation()); + assertEquals(ResourceDescriptor.RESPONSE_MAPPINGS_BUCKET, descriptor.getBucketName()); + assertEquals(ResourceDescriptor.RESPONSE_MAPPINGS_LOCATION, descriptor.getBucketLocation()); assertEquals("gpt-4", descriptor.getParentPath()); assertEquals("abc123", descriptor.getName()); } diff --git a/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java b/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java index 0a0bf8ed2..2e0d0e1a1 100644 --- a/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java @@ -1,33 +1,32 @@ package com.epam.aidial.core.server.util; -import com.epam.aidial.core.server.security.ApiKeyStore; -import com.epam.aidial.core.server.token.TokenStatsTracker; import com.epam.aidial.core.storage.resource.LegacyStorageLayout; import com.epam.aidial.core.storage.resource.ResourceDescriptor; import com.epam.aidial.core.storage.resource.ResourceTypes; import com.epam.aidial.core.storage.resource.StorageLayouts; import com.epam.aidial.core.storage.resource.TenantRootedStorageLayout; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import java.util.List; - import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; /** * Every descriptor the platform builds against a system bucket has to compose a path under the tenant-rooted * layout, not just the ones the resource API can reach. * - *

These paths are built off the request path — cost accounting runs per span, the job scheduler on a timer, - * response mappings on every completion — so an unmapped location does not surface as a failed request. It - * throws inside a background task and takes the subsystem out silently, which is how all three of these came - * to be broken under the tenant-rooted layout without a single test going red. + *

These paths are built off the request path, so an unmapped location does not surface as a failed + * request — it throws inside a background task and takes the subsystem out silently. */ public class SystemBucketLayoutTest { private static final String TENANT = "acme"; + @BeforeEach + public void useTenantRootedLayout() { + StorageLayouts.useLayout(new TenantRootedStorageLayout(TENANT)); + } + @AfterEach public void restoreLegacyLayout() { StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); @@ -35,8 +34,6 @@ public void restoreLegacyLayout() { @Test public void testResponseMappingPath() { - StorageLayouts.useLayout(new TenantRootedStorageLayout(TENANT)); - ResourceDescriptor descriptor = ResponseIdUtil.getResponseMappingDescriptor("dial_gpt-4_abc123"); assertEquals(".system/response_mappings/.response_mappings/gpt-4/abc123", descriptor.getAbsoluteFilePath()); @@ -45,8 +42,6 @@ public void testResponseMappingPath() { @Test public void testBackgroundJobPath() { - StorageLayouts.useLayout(new TenantRootedStorageLayout(TENANT)); - ResourceDescriptor descriptor = ResponseIdUtil.getBackgroundJobDescriptor("job-1"); assertEquals(".system/background_jobs/.background_jobs/job-1", descriptor.getAbsoluteFilePath()); @@ -58,50 +53,26 @@ public void testBackgroundJobPath() { */ @Test public void testBackgroundJobRootFolderPath() { - StorageLayouts.useLayout(new TenantRootedStorageLayout(TENANT)); - ResourceDescriptor root = ResponseIdUtil.getBackgroundJobDescriptor(null); assertEquals(".system/background_jobs/.background_jobs/", root.getAbsoluteFilePath()); } - // Per-request keys are how an application caller is represented, so this one breaks four of the eleven - // permission rules rather than a background task. @Test public void testApiKeyDataPath() { - StorageLayouts.useLayout(new TenantRootedStorageLayout(TENANT)); - ResourceDescriptor descriptor = ResourceDescriptorFactory.fromDecoded(ResourceTypes.API_KEY_DATA, - ApiKeyStore.API_KEY_DATA_BUCKET, ApiKeyStore.API_KEY_DATA_LOCATION, "some-key"); + ResourceDescriptor.API_KEY_DATA_BUCKET, ResourceDescriptor.API_KEY_DATA_LOCATION, "some-key"); assertEquals(".system/api_key_data/.api_key_data/some-key", descriptor.getAbsoluteFilePath()); } @Test public void testDeploymentCostStatsPath() { - StorageLayouts.useLayout(new TenantRootedStorageLayout(TENANT)); - ResourceDescriptor descriptor = ResourceDescriptorFactory.fromDecoded(ResourceTypes.DEPLOYMENT_COST_STATS, - TokenStatsTracker.DEPLOYMENT_COST_STATS_BUCKET, TokenStatsTracker.DEPLOYMENT_COST_STATS_LOCATION, + ResourceDescriptor.DEPLOYMENT_COST_STATS_BUCKET, ResourceDescriptor.DEPLOYMENT_COST_STATS_LOCATION, "trace-id"); assertEquals(".system/deployment_cost_stats/.deployment_cost_stats/trace-id", descriptor.getAbsoluteFilePath()); } - /** - * A system bucket added elsewhere and not registered in {@link ResourceDescriptor#SYSTEM_LOCATIONS} is - * exactly the defect this class exists for, so pin that the locations these utilities use are the - * registered ones rather than parallel literals. - */ - @Test - public void testCallersUseRegisteredLocations() { - for (String location : List.of( - ResponseIdUtil.RESPONSE_MAPPINGS_BUCKET_LOCATION, - ResponseIdUtil.BACKGROUND_JOB_BUCKET_LOCATION, - TokenStatsTracker.DEPLOYMENT_COST_STATS_LOCATION, - ApiKeyStore.API_KEY_DATA_LOCATION)) { - assertTrue(ResourceDescriptor.SYSTEM_LOCATIONS.contains(location), - () -> location + " is not registered in ResourceDescriptor.SYSTEM_LOCATIONS"); - } - } } diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/LegacyStorageLayout.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/LegacyStorageLayout.java index d124908e3..95fe8177c 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/LegacyStorageLayout.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/LegacyStorageLayout.java @@ -16,7 +16,7 @@ public String resolveLocationPrefix(String bucketLocation) { } @Override - public String resolveTypeFolder(String group) { - return group; + public String resolveTypeFolder(String typeGroup) { + return typeGroup; } } diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java index 46d99dd31..0a0860a55 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java @@ -45,9 +45,6 @@ public class ResourceDescriptor { * Buckets holding platform-internal runtime state rather than anyone's content. They belong to no * principal, so a layout has to place them somewhere other than the branches it uses for users and * projects, and they are listed here so a layout can enumerate them. - * - *

A location that is not a principal, not public, not the platform and not in this set is rejected - * rather than passed through: a silent fallback would let the next such bucket reach production unmapped. */ public static final Set SYSTEM_LOCATIONS = Set.of( DEPLOYMENT_COST_STATS_LOCATION, BACKGROUND_JOB_LOCATION, RESPONSE_MAPPINGS_LOCATION, @@ -139,21 +136,14 @@ public String getAbsoluteFilePath() { } /** - * Returns the path {@link #getAbsoluteFilePath()} produces under the legacy layout, whichever layout - * is active. - * - *

Callers that bake a resource path into something durable — an identifier handed to a user, an - * encryption AAD — must use this rather than the physical path. A physical path is free to change when the - * layout changes; anything derived from one and then stored is not, or the stored thing stops resolving. + * The path {@link #getAbsoluteFilePath()} produces under the legacy layout, whichever layout is active. + * Anything durable derived from a path — an identifier handed to a user, an encryption AAD — must use + * this: a physical path is free to change when the layout does, and the stored artifact is not. */ public String getStableFilePath() { return getStoragePrefix(LegacyStorageLayout.INSTANCE) + getPathWithinType(); } - /** - * Returns the prefix every path of this resource starts with under the given layout: the bucket - * location followed by the resource-type folder. - */ private String getStoragePrefix(StorageLayout layout) { return layout.resolveLocationPrefix(bucketLocation) + layout.resolveTypeFolder(type.group()) + PATH_SEPARATOR; } diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/StorageLayout.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/StorageLayout.java index 7a25bc276..d37fe6ed5 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/StorageLayout.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/StorageLayout.java @@ -9,5 +9,5 @@ public interface StorageLayout { String resolveLocationPrefix(String bucketLocation); - String resolveTypeFolder(String group); + String resolveTypeFolder(String typeGroup); } diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java index 779016c17..8456e5d3c 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java @@ -11,32 +11,25 @@ * stays in {@link ResourceDescriptor#getAbsoluteFilePath()}. * *

The conversion is total and reversible in both directions, so a migrated path can always be mapped - * back to its origin. + * back to its origin. Nothing on the request path converts backwards — the legacy direction exists for + * migration tooling and the layout verifier. */ @UtilityClass public class TenantLayoutTransform { - private static final String LEGACY_USERS_PREFIX = ResourceDescriptor.USERS_LOCATION_PREFIX; - private static final String LEGACY_KEYS_PREFIX = ResourceDescriptor.KEYS_LOCATION_PREFIX; - private static final String ORG_PREFIX = ".org/"; private static final String USERS_SEGMENT = ".users/"; private static final String KEYS_SEGMENT = ".keys/"; /** - * The platform scope is the root of the tenant-rooted tree, above any tenant, so it has no prefix. + * The platform scope maps to the root of the tenant-rooted tree, above any tenant, so it has no prefix. */ - private static final String PLATFORM_LOCATION = ""; + private static final String TENANT_TREE_ROOT = ""; /** * Where the system buckets of {@link ResourceDescriptor#SYSTEM_LOCATIONS} land — at the root, above any - * tenant, keeping the bucket name so the mapping stays reversible. - * - *

Above rather than inside a tenant because that is what preserves today's behaviour: the background - * job scheduler scans its bucket whole, and cost stats and response mappings are keyed by globally unique - * trace, job and response ids. Whether this state should become tenant-scoped is a real question — per - * tenant billing would want it to be — but it is a design decision for the phase that turns multi-tenancy - * on, not something to settle silently inside a path transform. + * tenant, which preserves whole-bucket scans and globally unique keys; the bucket name is kept so the + * mapping stays reversible. */ private static final String SYSTEM_SEGMENT = ".system/"; @@ -50,9 +43,13 @@ public class TenantLayoutTransform { */ private static final Set RESERVED_SEGMENTS = Set.of(ORG_PREFIX, USERS_SEGMENT, KEYS_SEGMENT, SYSTEM_SEGMENT); + /** + * A location that is not the platform, a system bucket, public or a principal is rejected rather than + * passed through: a silent fallback would let the next unmapped bucket reach production unnoticed. + */ public String toTenantLocation(String legacyLocation, String tenantId) { if (ResourceDescriptor.PLATFORM_LOCATION.equals(legacyLocation)) { - return PLATFORM_LOCATION; + return TENANT_TREE_ROOT; } if (ResourceDescriptor.SYSTEM_LOCATIONS.contains(legacyLocation)) { @@ -69,12 +66,12 @@ public String toTenantLocation(String legacyLocation, String tenantId) { return tenantRoot + scope; } - String userId = principalId(legacyLocation, LEGACY_USERS_PREFIX); + String userId = principalId(legacyLocation, ResourceDescriptor.USERS_LOCATION_PREFIX); if (userId != null) { return tenantRoot + USERS_SEGMENT + userId; } - String project = principalId(legacyLocation, LEGACY_KEYS_PREFIX); + String project = principalId(legacyLocation, ResourceDescriptor.KEYS_LOCATION_PREFIX); if (project != null) { return tenantRoot + KEYS_SEGMENT + project; } @@ -83,7 +80,7 @@ public String toTenantLocation(String legacyLocation, String tenantId) { } public String toLegacyLocation(String tenantLocation, String tenantId) { - if (PLATFORM_LOCATION.equals(tenantLocation)) { + if (TENANT_TREE_ROOT.equals(tenantLocation)) { return ResourceDescriptor.PLATFORM_LOCATION; } @@ -107,12 +104,12 @@ public String toLegacyLocation(String tenantLocation, String tenantId) { String userId = principalId(scope, USERS_SEGMENT); if (userId != null) { - return LEGACY_USERS_PREFIX + userId; + return ResourceDescriptor.USERS_LOCATION_PREFIX + userId; } String project = principalId(scope, KEYS_SEGMENT); if (project != null) { - return LEGACY_KEYS_PREFIX + project; + return ResourceDescriptor.KEYS_LOCATION_PREFIX + project; } if (scope.charAt(0) != TYPE_FOLDER_MARKER) { @@ -163,11 +160,20 @@ private void requireUnreserved(String dottedFolder, String input) { } } - private String tenantRoot(String tenantId) { - if (tenantId.isEmpty()) { - throw new IllegalArgumentException("Tenant id must not be empty"); + /** + * A tenant id is a single undotted path segment: a separator would let one tenant's root nest inside + * another's, and a leading marker would collide with the reserved dotted names. + */ + void requireTenantId(String tenantId) { + if (tenantId.isEmpty() + || tenantId.contains(ResourceDescriptor.PATH_SEPARATOR) + || tenantId.charAt(0) == TYPE_FOLDER_MARKER) { + throw new IllegalArgumentException("Unsupported tenant id: " + tenantId); } + } + private String tenantRoot(String tenantId) { + requireTenantId(tenantId); return ORG_PREFIX + tenantId + ResourceDescriptor.PATH_SEPARATOR; } diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayout.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayout.java index ffabeffce..d0c196e85 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayout.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayout.java @@ -13,6 +13,8 @@ public TenantRootedStorageLayout(String tenantId) { throw new IllegalArgumentException("Tenant id must not be blank"); } + // Same rule the transform applies on every composition, but failing here fails at start-up. + TenantLayoutTransform.requireTenantId(tenantId); this.tenantId = tenantId; } @@ -22,7 +24,7 @@ public String resolveLocationPrefix(String bucketLocation) { } @Override - public String resolveTypeFolder(String group) { - return TenantLayoutTransform.toTenantTypeFolder(group); + public String resolveTypeFolder(String typeGroup) { + return TenantLayoutTransform.toTenantTypeFolder(typeGroup); } } diff --git a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java index 4059679d1..30e1fe5fd 100644 --- a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java +++ b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java @@ -65,6 +65,8 @@ public void testDottedOrUnterminatedPublicScopeRejected() { () -> TenantLayoutTransform.toTenantLocation("public/deployments/abc123", TENANT)); assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toLegacyLocation(".org/default-tenant/deployments/abc123", TENANT)); + assertThrows(IllegalArgumentException.class, + () -> TenantLayoutTransform.toLegacyLocation(".org/default-tenant/deployments/.x/", TENANT)); } @Test @@ -83,8 +85,7 @@ public void testSystemLocations() { /** * Every system bucket has to be mapped, not just the ones a test happened to name: an unmapped one throws - * at the point a path is composed, which takes out whatever subsystem owns it — that is how the background - * job scheduler and per-request cost accounting were found broken under this layout. + * at the point a path is composed, which takes out whatever subsystem owns it. */ @Test public void testEverySystemLocationRoundTrips() { @@ -123,6 +124,8 @@ public void testDottedPrincipalSegmentsRejected() { assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Keys/applications/../app/", TENANT)); assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toLegacyLocation(".org/default-tenant/.users/../", TENANT)); + assertThrows(IllegalArgumentException.class, + () -> TenantLayoutTransform.toLegacyLocation(".org/default-tenant/.keys/../", TENANT)); } @Test @@ -130,6 +133,7 @@ public void testUnsupportedLegacyLocation() { assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Unknown/u1/", TENANT)); assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("", TENANT)); assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Users/", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Users//", TENANT)); assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Users/u1", TENANT)); } @@ -151,6 +155,19 @@ public void testEmptyTenantRejected() { assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toLegacyLocation(".org//", "")); } + /** + * A separator would let one tenant's root nest inside another's ("acme/.users" collides with tenant + * acme's users branch), and a leading marker collides with the reserved dotted names. + */ + @Test + public void testInvalidTenantRejected() { + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("public/", "a/b")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("public/", "acme/.users")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("public/", "..")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("public/", ".acme")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toLegacyLocation(".org/a/b/", "a/b")); + } + @Test public void testTypeFolder() { assertEquals(".files", TenantLayoutTransform.toTenantTypeFolder("files")); diff --git a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java index 1b693f35e..42d73fc2e 100644 --- a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java +++ b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java @@ -1,7 +1,10 @@ package com.epam.aidial.core.storage.resource; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import java.util.List; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -9,6 +12,11 @@ public class TenantRootedStorageLayoutTest { private final StorageLayout layout = new TenantRootedStorageLayout("acme"); + @AfterEach + public void restoreLegacyLayout() { + StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); + } + @Test public void testLocationPrefixIsTenantRooted() { assertEquals(".org/acme/.users/u1/", layout.resolveLocationPrefix("Users/u1/")); @@ -37,16 +45,30 @@ public void testSystemLocationsResolve() { @Test public void testSystemBucketPath() { StorageLayouts.useLayout(layout); - try { - ResourceDescriptor job = new ResourceDescriptor(ResourceTypes.BACKGROUND_JOB, "job-1", - java.util.List.of(), ResourceDescriptor.BACKGROUND_JOB_BUCKET, - ResourceDescriptor.BACKGROUND_JOB_LOCATION, false); - assertEquals(".system/background_jobs/.background_jobs/job-1", job.getAbsoluteFilePath()); - assertEquals("background_jobs/background_jobs/job-1", job.getStableFilePath()); - } finally { - StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); - } + ResourceDescriptor job = new ResourceDescriptor(ResourceTypes.BACKGROUND_JOB, "job-1", + List.of(), ResourceDescriptor.BACKGROUND_JOB_BUCKET, + ResourceDescriptor.BACKGROUND_JOB_LOCATION, false); + + assertEquals(".system/background_jobs/.background_jobs/job-1", job.getAbsoluteFilePath()); + assertEquals("background_jobs/background_jobs/job-1", job.getStableFilePath()); + } + + /** + * {@code resolveByPath} re-derives a descriptor from a listed physical path, so it has to parse the + * tenant-rooted shape, not just compose it. + */ + @Test + public void testResolveByPathUnderTenantRootedLayout() { + StorageLayouts.useLayout(layout); + + ResourceDescriptor folder = new ResourceDescriptor(ResourceTypes.CONVERSATION, null, + List.of(), "bucket", "Users/u1/", true); + ResourceDescriptor resolved = folder.resolveByPath(".org/acme/.users/u1/.conversations/chats/chat1"); + + assertEquals("chat1", resolved.getName()); + assertEquals(List.of("chats"), resolved.getParentFolders()); + assertEquals(".org/acme/.users/u1/.conversations/chats/chat1", resolved.getAbsoluteFilePath()); } @Test @@ -59,4 +81,15 @@ public void testBlankTenantRejected() { assertThrows(IllegalArgumentException.class, () -> new TenantRootedStorageLayout(null)); assertThrows(IllegalArgumentException.class, () -> new TenantRootedStorageLayout(" ")); } + + /** + * The constructor applies the same tenant-id rule the transform applies on every composition, so a + * misconfigured tenant fails at start-up rather than on the first request. + */ + @Test + public void testInvalidTenantRejectedAtConstruction() { + assertThrows(IllegalArgumentException.class, () -> new TenantRootedStorageLayout("acme/.users")); + assertThrows(IllegalArgumentException.class, () -> new TenantRootedStorageLayout("..")); + assertThrows(IllegalArgumentException.class, () -> new TenantRootedStorageLayout(".acme")); + } } From 74fe4b033c533c3014d7a9cdfd8cf7e83312870b Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Mon, 7 Sep 2026 14:18:57 +0300 Subject: [PATCH 07/10] refactor: rename TenantLayoutTransform to TenantLayoutTransformer #1863 Agent noun per review on #1866; landed here because every commit above the seam edits this class and a bottom-of-stack rename would conflict through all of them for the same end state. Co-Authored-By: Claude Fable 5 --- ...form.java => TenantLayoutTransformer.java} | 2 +- .../resource/TenantRootedStorageLayout.java | 8 +- ....java => TenantLayoutTransformerTest.java} | 114 +++++++++--------- 3 files changed, 62 insertions(+), 62 deletions(-) rename storage/src/main/java/com/epam/aidial/core/storage/resource/{TenantLayoutTransform.java => TenantLayoutTransformer.java} (99%) rename storage/src/test/java/com/epam/aidial/core/storage/resource/{TenantLayoutTransformTest.java => TenantLayoutTransformerTest.java} (57%) diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformer.java similarity index 99% rename from storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java rename to storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformer.java index 8456e5d3c..eca32dfe8 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransform.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformer.java @@ -15,7 +15,7 @@ * migration tooling and the layout verifier. */ @UtilityClass -public class TenantLayoutTransform { +public class TenantLayoutTransformer { private static final String ORG_PREFIX = ".org/"; private static final String USERS_SEGMENT = ".users/"; diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayout.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayout.java index d0c196e85..6b968df07 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayout.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayout.java @@ -2,7 +2,7 @@ /** * The tenant-rooted layout: every bucket location is placed under its tenant, and resource-type folders - * are reserved names. Conversion rules live in {@link TenantLayoutTransform}. + * are reserved names. Conversion rules live in {@link TenantLayoutTransformer}. */ public final class TenantRootedStorageLayout implements StorageLayout { @@ -14,17 +14,17 @@ public TenantRootedStorageLayout(String tenantId) { } // Same rule the transform applies on every composition, but failing here fails at start-up. - TenantLayoutTransform.requireTenantId(tenantId); + TenantLayoutTransformer.requireTenantId(tenantId); this.tenantId = tenantId; } @Override public String resolveLocationPrefix(String bucketLocation) { - return TenantLayoutTransform.toTenantLocation(bucketLocation, tenantId); + return TenantLayoutTransformer.toTenantLocation(bucketLocation, tenantId); } @Override public String resolveTypeFolder(String typeGroup) { - return TenantLayoutTransform.toTenantTypeFolder(typeGroup); + return TenantLayoutTransformer.toTenantTypeFolder(typeGroup); } } diff --git a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformerTest.java similarity index 57% rename from storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java rename to storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformerTest.java index 30e1fe5fd..086433047 100644 --- a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformTest.java +++ b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformerTest.java @@ -5,32 +5,32 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -public class TenantLayoutTransformTest { +public class TenantLayoutTransformerTest { private static final String TENANT = "default-tenant"; @Test public void testPlatformLocation() { - assertEquals("", TenantLayoutTransform.toTenantLocation("platform/", TENANT)); - assertEquals("platform/", TenantLayoutTransform.toLegacyLocation("", TENANT)); + assertEquals("", TenantLayoutTransformer.toTenantLocation("platform/", TENANT)); + assertEquals("platform/", TenantLayoutTransformer.toLegacyLocation("", TENANT)); } @Test public void testPublicLocation() { - assertEquals(".org/default-tenant/", TenantLayoutTransform.toTenantLocation("public/", TENANT)); - assertEquals("public/", TenantLayoutTransform.toLegacyLocation(".org/default-tenant/", TENANT)); + assertEquals(".org/default-tenant/", TenantLayoutTransformer.toTenantLocation("public/", TENANT)); + assertEquals("public/", TenantLayoutTransformer.toLegacyLocation(".org/default-tenant/", TENANT)); } @Test public void testUserLocation() { - assertEquals(".org/default-tenant/.users/u1/", TenantLayoutTransform.toTenantLocation("Users/u1/", TENANT)); - assertEquals("Users/u1/", TenantLayoutTransform.toLegacyLocation(".org/default-tenant/.users/u1/", TENANT)); + assertEquals(".org/default-tenant/.users/u1/", TenantLayoutTransformer.toTenantLocation("Users/u1/", TENANT)); + assertEquals("Users/u1/", TenantLayoutTransformer.toLegacyLocation(".org/default-tenant/.users/u1/", TENANT)); } @Test public void testKeyLocation() { - assertEquals(".org/default-tenant/.keys/EPM-RTC-GPT/", TenantLayoutTransform.toTenantLocation("Keys/EPM-RTC-GPT/", TENANT)); - assertEquals("Keys/EPM-RTC-GPT/", TenantLayoutTransform.toLegacyLocation(".org/default-tenant/.keys/EPM-RTC-GPT/", TENANT)); + assertEquals(".org/default-tenant/.keys/EPM-RTC-GPT/", TenantLayoutTransformer.toTenantLocation("Keys/EPM-RTC-GPT/", TENANT)); + assertEquals("Keys/EPM-RTC-GPT/", TenantLayoutTransformer.toLegacyLocation(".org/default-tenant/.keys/EPM-RTC-GPT/", TENANT)); } @Test @@ -38,8 +38,8 @@ public void testMultiSegmentKeyLocation() { String legacy = "Keys/applications/abc123/my-app/"; String tenant = ".org/default-tenant/.keys/applications/abc123/my-app/"; - assertEquals(tenant, TenantLayoutTransform.toTenantLocation(legacy, TENANT)); - assertEquals(legacy, TenantLayoutTransform.toLegacyLocation(tenant, TENANT)); + assertEquals(tenant, TenantLayoutTransformer.toTenantLocation(legacy, TENANT)); + assertEquals(legacy, TenantLayoutTransformer.toLegacyLocation(tenant, TENANT)); } /** @@ -50,37 +50,37 @@ public void testMultiSegmentKeyLocation() { @Test public void testPublicDeploymentsLocation() { assertEquals(".org/default-tenant/deployments/abc123/", - TenantLayoutTransform.toTenantLocation("public/deployments/abc123/", TENANT)); + TenantLayoutTransformer.toTenantLocation("public/deployments/abc123/", TENANT)); assertEquals("public/deployments/abc123/", - TenantLayoutTransform.toLegacyLocation(".org/default-tenant/deployments/abc123/", TENANT)); + TenantLayoutTransformer.toLegacyLocation(".org/default-tenant/deployments/abc123/", TENANT)); } @Test public void testDottedOrUnterminatedPublicScopeRejected() { assertThrows(IllegalArgumentException.class, - () -> TenantLayoutTransform.toTenantLocation("public/.deployments/abc123/", TENANT)); + () -> TenantLayoutTransformer.toTenantLocation("public/.deployments/abc123/", TENANT)); assertThrows(IllegalArgumentException.class, - () -> TenantLayoutTransform.toTenantLocation("public/deployments/.abc123/", TENANT)); + () -> TenantLayoutTransformer.toTenantLocation("public/deployments/.abc123/", TENANT)); assertThrows(IllegalArgumentException.class, - () -> TenantLayoutTransform.toTenantLocation("public/deployments/abc123", TENANT)); + () -> TenantLayoutTransformer.toTenantLocation("public/deployments/abc123", TENANT)); assertThrows(IllegalArgumentException.class, - () -> TenantLayoutTransform.toLegacyLocation(".org/default-tenant/deployments/abc123", TENANT)); + () -> TenantLayoutTransformer.toLegacyLocation(".org/default-tenant/deployments/abc123", TENANT)); assertThrows(IllegalArgumentException.class, - () -> TenantLayoutTransform.toLegacyLocation(".org/default-tenant/deployments/.x/", TENANT)); + () -> TenantLayoutTransformer.toLegacyLocation(".org/default-tenant/deployments/.x/", TENANT)); } @Test public void testLocationRoundTrip() { for (String legacy : new String[] {"platform/", "public/", "public/deployments/abc123/", "Users/u1/", "Keys/proj/", "Keys/applications/abc/app/"}) { - String tenant = TenantLayoutTransform.toTenantLocation(legacy, TENANT); - assertEquals(legacy, TenantLayoutTransform.toLegacyLocation(tenant, TENANT)); + String tenant = TenantLayoutTransformer.toTenantLocation(legacy, TENANT); + assertEquals(legacy, TenantLayoutTransformer.toLegacyLocation(tenant, TENANT)); } } @Test public void testSystemLocations() { - assertEquals(".system/background_jobs/", TenantLayoutTransform.toTenantLocation("background_jobs/", TENANT)); - assertEquals("background_jobs/", TenantLayoutTransform.toLegacyLocation(".system/background_jobs/", TENANT)); + assertEquals(".system/background_jobs/", TenantLayoutTransformer.toTenantLocation("background_jobs/", TENANT)); + assertEquals("background_jobs/", TenantLayoutTransformer.toLegacyLocation(".system/background_jobs/", TENANT)); } /** @@ -90,8 +90,8 @@ public void testSystemLocations() { @Test public void testEverySystemLocationRoundTrips() { for (String legacy : ResourceDescriptor.SYSTEM_LOCATIONS) { - String tenant = TenantLayoutTransform.toTenantLocation(legacy, TENANT); - assertEquals(legacy, TenantLayoutTransform.toLegacyLocation(tenant, TENANT)); + String tenant = TenantLayoutTransformer.toTenantLocation(legacy, TENANT); + assertEquals(legacy, TenantLayoutTransformer.toLegacyLocation(tenant, TENANT)); } } @@ -101,15 +101,15 @@ public void testEverySystemLocationRoundTrips() { @Test public void testSystemLocationsAreTenantIndependent() { for (String legacy : ResourceDescriptor.SYSTEM_LOCATIONS) { - assertEquals(TenantLayoutTransform.toTenantLocation(legacy, TENANT), - TenantLayoutTransform.toTenantLocation(legacy, "another-tenant")); + assertEquals(TenantLayoutTransformer.toTenantLocation(legacy, TENANT), + TenantLayoutTransformer.toTenantLocation(legacy, "another-tenant")); } } @Test public void testUnknownSystemLocationRejected() { assertThrows(IllegalArgumentException.class, - () -> TenantLayoutTransform.toLegacyLocation(".system/not_a_system_bucket/", TENANT)); + () -> TenantLayoutTransformer.toLegacyLocation(".system/not_a_system_bucket/", TENANT)); } /** @@ -119,40 +119,40 @@ public void testUnknownSystemLocationRejected() { */ @Test public void testDottedPrincipalSegmentsRejected() { - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Users/../", TENANT)); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Users/.evil/", TENANT)); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Keys/applications/../app/", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("Users/../", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("Users/.evil/", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("Keys/applications/../app/", TENANT)); assertThrows(IllegalArgumentException.class, - () -> TenantLayoutTransform.toLegacyLocation(".org/default-tenant/.users/../", TENANT)); + () -> TenantLayoutTransformer.toLegacyLocation(".org/default-tenant/.users/../", TENANT)); assertThrows(IllegalArgumentException.class, - () -> TenantLayoutTransform.toLegacyLocation(".org/default-tenant/.keys/../", TENANT)); + () -> TenantLayoutTransformer.toLegacyLocation(".org/default-tenant/.keys/../", TENANT)); } @Test public void testUnsupportedLegacyLocation() { - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Unknown/u1/", TENANT)); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("", TENANT)); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Users/", TENANT)); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Users//", TENANT)); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("Users/u1", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("Unknown/u1/", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("Users/", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("Users//", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("Users/u1", TENANT)); } @Test public void testUnsupportedTenantLocation() { - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toLegacyLocation(".org/default-tenant/.other/u1/", TENANT)); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toLegacyLocation(".org/default-tenant/.users/", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toLegacyLocation(".org/default-tenant/.other/u1/", TENANT)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toLegacyLocation(".org/default-tenant/.users/", TENANT)); } @Test public void testForeignTenantRejected() { assertThrows(IllegalArgumentException.class, - () -> TenantLayoutTransform.toLegacyLocation(".org/other-tenant/.users/u1/", TENANT)); + () -> TenantLayoutTransformer.toLegacyLocation(".org/other-tenant/.users/u1/", TENANT)); } @Test public void testEmptyTenantRejected() { - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("public/", "")); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toLegacyLocation(".org//", "")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("public/", "")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toLegacyLocation(".org//", "")); } /** @@ -161,24 +161,24 @@ public void testEmptyTenantRejected() { */ @Test public void testInvalidTenantRejected() { - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("public/", "a/b")); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("public/", "acme/.users")); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("public/", "..")); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantLocation("public/", ".acme")); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toLegacyLocation(".org/a/b/", "a/b")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("public/", "a/b")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("public/", "acme/.users")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("public/", "..")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantLocation("public/", ".acme")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toLegacyLocation(".org/a/b/", "a/b")); } @Test public void testTypeFolder() { - assertEquals(".files", TenantLayoutTransform.toTenantTypeFolder("files")); - assertEquals("files", TenantLayoutTransform.toLegacyTypeFolder(".files")); + assertEquals(".files", TenantLayoutTransformer.toTenantTypeFolder("files")); + assertEquals("files", TenantLayoutTransformer.toLegacyTypeFolder(".files")); } @Test public void testTypeFolderRoundTripsForEveryResourceType() { for (ResourceTypes type : ResourceTypes.values()) { - String tenantFolder = TenantLayoutTransform.toTenantTypeFolder(type.group()); - assertEquals(type.group(), TenantLayoutTransform.toLegacyTypeFolder(tenantFolder)); + String tenantFolder = TenantLayoutTransformer.toTenantTypeFolder(type.group()); + assertEquals(type.group(), TenantLayoutTransformer.toLegacyTypeFolder(tenantFolder)); } } @@ -189,16 +189,16 @@ public void testTypeFolderRoundTripsForEveryResourceType() { @Test public void testReservedTypeFolderNamesRejected() { for (String reserved : new String[] {"org", "system", "users", "keys"}) { - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantTypeFolder(reserved)); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toLegacyTypeFolder("." + reserved)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantTypeFolder(reserved)); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toLegacyTypeFolder("." + reserved)); } } @Test public void testUnsupportedTypeFolder() { - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantTypeFolder("")); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toTenantTypeFolder(".files")); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toLegacyTypeFolder("files")); - assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransform.toLegacyTypeFolder(".")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantTypeFolder("")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toTenantTypeFolder(".files")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toLegacyTypeFolder("files")); + assertThrows(IllegalArgumentException.class, () -> TenantLayoutTransformer.toLegacyTypeFolder(".")); } } From 41f441cab56242bf31d88b2168e901786f18f188 Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Mon, 7 Sep 2026 14:20:04 +0300 Subject: [PATCH 08/10] refactor: move the layout settings under the storage block #1863 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit storage.layout.{tenantRooted,defaultTenant} per review on #1866 — the layout is a storage concern. The block is stripped before the Storage POJO decode because the codec rejects unknown properties. Co-Authored-By: Claude Fable 5 --- README.md | 4 ++-- .../main/java/com/epam/aidial/core/server/AiDial.java | 9 +++++++-- server/src/main/resources/aidial.settings.json | 8 ++++---- .../aidial/core/server/TenantRootedLayoutApiTest.java | 6 +++--- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 79540dfcd..b5a679807 100644 --- a/README.md +++ b/README.md @@ -232,8 +232,8 @@ without putting the raw address in the log. | storage.bucket | - | No | Blob storage bucket. | | storage.overrides.* | - | No | Key-value pairs to override storage settings. `*` might be any specific blob storage setting to be overridden. Refer to [examples](#temporary-credentials-1) in the sections below. | | storage.createBucket | false | No | Indicates whether bucket should be created on start-up. | -| storageLayout.tenantRooted | false | No | Places every bucket under a tenant root instead of storing it at the top level. Changing it on a populated deployment re-addresses existing data and requires migration; leave disabled otherwise. | -| storageLayout.defaultTenant | default | No | Tenant that existing buckets are placed under when `storageLayout.tenantRooted` is enabled. | +| storage.layout.tenantRooted | false | No | Places every bucket under a tenant root instead of storing it at the top level. Changing it on a populated deployment re-addresses existing data and requires migration; leave disabled otherwise. | +| storage.layout.defaultTenant | default | No | Tenant that existing buckets are placed under when `storage.layout.tenantRooted` is enabled. | | storage.prefix | - | No | Base prefix for all stored resources. The purpose to use the same bucket for different environments, e.g. dev, prod, pre-prod. Must not contain path separators or any invalid chars. | | storage.maxUploadedFileSize | 536870912 | No | Maximum size in bytes of uploaded file. If a size of uploaded file exceeds the limit the server returns HTTP code 413 | diff --git a/server/src/main/java/com/epam/aidial/core/server/AiDial.java b/server/src/main/java/com/epam/aidial/core/server/AiDial.java index 445c8e906..e41c80f6a 100644 --- a/server/src/main/java/com/epam/aidial/core/server/AiDial.java +++ b/server/src/main/java/com/epam/aidial/core/server/AiDial.java @@ -210,10 +210,15 @@ void start() throws Exception { accessTokenValidator = new AccessTokenValidator(settings("identityProviders"), vertx, taskExecutor, client, clientOptions, claimsLogLevel); } - StorageLayouts.useLayout(createStorageLayout(settings("storageLayout"))); + StorageLayouts.useLayout(createStorageLayout( + settings("storage").getJsonObject("layout", new JsonObject()))); if (storage == null) { - Storage storageConfig = Json.decodeValue(settings("storage").toBuffer(), Storage.class); + // The layout block configures path composition, not the blob store; it is stripped + // before the decode because the codec rejects unknown properties. + JsonObject storageSettings = settings("storage").copy(); + storageSettings.remove("layout"); + Storage storageConfig = Json.decodeValue(storageSettings.toBuffer(), Storage.class); storage = new BlobStorage(storageConfig); } encryptionService = new EncryptionService(settings("encryption")); diff --git a/server/src/main/resources/aidial.settings.json b/server/src/main/resources/aidial.settings.json index db960d2b3..3b3b8d2f3 100644 --- a/server/src/main/resources/aidial.settings.json +++ b/server/src/main/resources/aidial.settings.json @@ -53,12 +53,12 @@ "createBucket": true, "overrides": { "jclouds.filesystem.basedir": "data" + }, + "layout": { + "tenantRooted": false, + "defaultTenant": "default" } }, - "storageLayout": { - "tenantRooted": false, - "defaultTenant": "default" - }, "resources": { "maxSize" : 67108864, "maxSizeToCache": 1048576, diff --git a/server/src/test/java/com/epam/aidial/core/server/TenantRootedLayoutApiTest.java b/server/src/test/java/com/epam/aidial/core/server/TenantRootedLayoutApiTest.java index fc0c2def1..5d442c20e 100644 --- a/server/src/test/java/com/epam/aidial/core/server/TenantRootedLayoutApiTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/TenantRootedLayoutApiTest.java @@ -18,7 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; /** - * Drives the resource API with {@code storageLayout.tenantRooted} enabled: the whole stack — descriptor, + * Drives the resource API with {@code storage.layout.tenantRooted} enabled: the whole stack — descriptor, * cache and blob store — has to agree on the tenant-rooted paths, which unit tests cannot show. */ public class TenantRootedLayoutApiTest extends ResourceBaseTest { @@ -27,9 +27,9 @@ public class TenantRootedLayoutApiTest extends ResourceBaseTest { @Override protected JsonObject additionalSettingsOverrides() { - return new JsonObject().put("storageLayout", new JsonObject() + return new JsonObject().put("storage", new JsonObject().put("layout", new JsonObject() .put("tenantRooted", true) - .put("defaultTenant", TENANT)); + .put("defaultTenant", TENANT))); } @AfterEach From c69a5c0b8e3e97be9f4b1a8b63dc69d9acc685cd Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Mon, 7 Sep 2026 14:21:25 +0300 Subject: [PATCH 09/10] refactor: rename getStableFilePath to getLegacyFilePath #1863 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review: the name now says what the path is (the legacy layout's composition) rather than the property it has. The contract stays in the javadoc — durable artifacts derive from this path, never the physical one. Co-Authored-By: Claude Fable 5 --- .../aidial/core/server/config/SecretFieldProcessor.java | 2 +- .../aidial/core/server/service/BackgroundJobService.java | 2 +- .../epam/aidial/core/server/service/InvitationService.java | 2 +- .../core/server/service/UserExternalServiceService.java | 4 ++-- .../core/server/config/SecretFieldProcessorLayoutTest.java | 6 +++--- .../server/service/ApplicationDeploymentLayoutTest.java | 2 +- .../aidial/core/server/util/SystemBucketLayoutTest.java | 2 +- .../aidial/core/storage/resource/ResourceDescriptor.java | 2 +- .../storage/resource/TenantRootedStorageLayoutTest.java | 2 +- 9 files changed, 12 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java b/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java index 15fee1e59..3268d3b9f 100644 --- a/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java +++ b/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java @@ -61,7 +61,7 @@ public String resolveSecret(String value, ResourceDescriptor descriptor) { } private static byte[] aad(ResourceDescriptor descriptor) { - return descriptor.getStableFilePath().getBytes(StandardCharsets.UTF_8); + return descriptor.getLegacyFilePath().getBytes(StandardCharsets.UTF_8); } /** diff --git a/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java b/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java index b460971b9..da3c10156 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/BackgroundJobService.java @@ -285,7 +285,7 @@ private String decryptKey(ResourceDescriptor descriptor, String key) { } private static byte[] aad(ResourceDescriptor descriptor) { - return descriptor.getStableFilePath().getBytes(StandardCharsets.UTF_8); + return descriptor.getLegacyFilePath().getBytes(StandardCharsets.UTF_8); } private Future completeAndProcess( diff --git a/server/src/main/java/com/epam/aidial/core/server/service/InvitationService.java b/server/src/main/java/com/epam/aidial/core/server/service/InvitationService.java index 8d438642d..c9de9c506 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/InvitationService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/InvitationService.java @@ -273,6 +273,6 @@ public ResourceDescriptor getInvitationResource(String invitationId) { // the location back out of it. It must therefore not carry the physical path, which the storage layout is // free to change: an invitation issued before a layout change has to keep resolving after it. private String generateInvitationId(ResourceDescriptor resource) { - return encryptionService.encrypt(resource.getStableFilePath() + ResourceDescriptor.PATH_SEPARATOR + ApiKeyGenerator.generateKey()); + return encryptionService.encrypt(resource.getLegacyFilePath() + ResourceDescriptor.PATH_SEPARATOR + ApiKeyGenerator.generateKey()); } } diff --git a/server/src/main/java/com/epam/aidial/core/server/service/UserExternalServiceService.java b/server/src/main/java/com/epam/aidial/core/server/service/UserExternalServiceService.java index f3b5afa8e..f17cc6184 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/UserExternalServiceService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/UserExternalServiceService.java @@ -78,7 +78,7 @@ private static List applicationSegments(String appPart) { public ExternalService put(String ownerUserId, String appPart, String serviceId, ExternalService service, String author) { ResourceDescriptor resource = descriptor(ownerUserId, appPart, serviceId); BucketInfo bucket = new BucketInfo(resource.getBucketName(), resource.getBucketLocation()); - String aad = resource.getStableFilePath(); + String aad = resource.getLegacyFilePath(); MutableObject result = new MutableObject<>(); PersistedSecret persisted = new PersistedSecret(); resourceService.computeResource(resource, EtagHeader.ANY, author, json -> { @@ -104,7 +104,7 @@ public ExternalService get(String ownerUserId, String appPart, String serviceId) return null; } ExternalService service = ProxyUtil.convertToObject(stored.getValue(), ExternalService.class); - decryptSecret(resource.getStableFilePath(), new BucketInfo(resource.getBucketName(), resource.getBucketLocation()), service); + decryptSecret(resource.getLegacyFilePath(), new BucketInfo(resource.getBucketName(), resource.getBucketLocation()), service); return service; } diff --git a/server/src/test/java/com/epam/aidial/core/server/config/SecretFieldProcessorLayoutTest.java b/server/src/test/java/com/epam/aidial/core/server/config/SecretFieldProcessorLayoutTest.java index ea07b837b..50a22d909 100644 --- a/server/src/test/java/com/epam/aidial/core/server/config/SecretFieldProcessorLayoutTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/config/SecretFieldProcessorLayoutTest.java @@ -79,12 +79,12 @@ public void testSecretEncryptedUnderLegacyLayoutDecryptsUnderTenantRootedLayout( assertEquals("plain-secret", key.getKey()); } - // Under the legacy layout the physical and stable paths coincide, so the divergence only exists on the + // Under the legacy layout the physical and legacy paths coincide, so the divergence only exists on the // tenant-rooted side: ciphertext bound to the tenant-shaped physical path must not decrypt against the - // stable AAD. This is also the guard proving the AAD participates at all — with a cipher that ignored + // legacy-path AAD. This is also the guard proving the AAD participates at all — with a cipher that ignored // it, the round-trip test above would pass for any path. @Test - public void testPhysicalPathAadDoesNotMatchTheStableAad() { + public void testPhysicalPathAadDoesNotMatchTheLegacyAad() { StorageLayouts.useLayout(new TenantRootedStorageLayout("acme")); byte[] physicalPathAad = descriptor.getAbsoluteFilePath().getBytes(StandardCharsets.UTF_8); diff --git a/server/src/test/java/com/epam/aidial/core/server/service/ApplicationDeploymentLayoutTest.java b/server/src/test/java/com/epam/aidial/core/server/service/ApplicationDeploymentLayoutTest.java index e66158448..f19546d7d 100644 --- a/server/src/test/java/com/epam/aidial/core/server/service/ApplicationDeploymentLayoutTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/service/ApplicationDeploymentLayoutTest.java @@ -35,7 +35,7 @@ public void testPublicDeploymentFolder() { ResourceDescriptor folder = deploymentFolder(ResourceDescriptor.PUBLIC_LOCATION); assertEquals(".org/acme/deployments/fn-1/.files/", folder.getAbsoluteFilePath()); - assertEquals("public/deployments/fn-1/files/", folder.getStableFilePath()); + assertEquals("public/deployments/fn-1/files/", folder.getLegacyFilePath()); } @Test diff --git a/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java b/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java index 2e0d0e1a1..cd1c693a9 100644 --- a/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java @@ -37,7 +37,7 @@ public void testResponseMappingPath() { ResourceDescriptor descriptor = ResponseIdUtil.getResponseMappingDescriptor("dial_gpt-4_abc123"); assertEquals(".system/response_mappings/.response_mappings/gpt-4/abc123", descriptor.getAbsoluteFilePath()); - assertEquals("response_mappings/response_mappings/gpt-4/abc123", descriptor.getStableFilePath()); + assertEquals("response_mappings/response_mappings/gpt-4/abc123", descriptor.getLegacyFilePath()); } @Test diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java index 0a0860a55..9c4764af6 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java @@ -140,7 +140,7 @@ public String getAbsoluteFilePath() { * Anything durable derived from a path — an identifier handed to a user, an encryption AAD — must use * this: a physical path is free to change when the layout does, and the stored artifact is not. */ - public String getStableFilePath() { + public String getLegacyFilePath() { return getStoragePrefix(LegacyStorageLayout.INSTANCE) + getPathWithinType(); } diff --git a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java index 42d73fc2e..d6a1f9351 100644 --- a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java +++ b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java @@ -51,7 +51,7 @@ public void testSystemBucketPath() { ResourceDescriptor.BACKGROUND_JOB_LOCATION, false); assertEquals(".system/background_jobs/.background_jobs/job-1", job.getAbsoluteFilePath()); - assertEquals("background_jobs/background_jobs/job-1", job.getStableFilePath()); + assertEquals("background_jobs/background_jobs/job-1", job.getLegacyFilePath()); } /** From eaf03d67db0b95a54eba4cf5768fb78c72fc352b Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Mon, 7 Sep 2026 14:25:50 +0300 Subject: [PATCH 10/10] refactor: extract SystemResourceRegistry #1863 Per review: the four system-bucket constant pairs and SYSTEM_LOCATIONS move off ResourceDescriptor into an enum owning the bucket/location vocabulary; the transformer asks isSystemLocation instead of set membership, and callers name their bucket by entry. Also documents resolveTypeFolder's parameter, which the vocabulary rename had left unexplained. Co-Authored-By: Claude Fable 5 --- .../core/server/security/ApiKeyStore.java | 3 +- .../service/ResponseMappingService.java | 5 ++- .../core/server/token/TokenStatsTracker.java | 3 +- .../core/server/util/ResponseIdUtil.java | 5 ++- .../core/server/util/ResponseIdUtilTest.java | 5 ++- .../server/util/SystemBucketLayoutTest.java | 5 ++- .../storage/resource/ResourceDescriptor.java | 19 ---------- .../core/storage/resource/StorageLayout.java | 3 ++ .../resource/SystemResourceRegistry.java | 37 +++++++++++++++++++ .../resource/TenantLayoutTransformer.java | 6 +-- .../resource/TenantLayoutTransformerTest.java | 12 +++--- .../TenantRootedStorageLayoutTest.java | 4 +- 12 files changed, 67 insertions(+), 40 deletions(-) create mode 100644 storage/src/main/java/com/epam/aidial/core/storage/resource/SystemResourceRegistry.java diff --git a/server/src/main/java/com/epam/aidial/core/server/security/ApiKeyStore.java b/server/src/main/java/com/epam/aidial/core/server/security/ApiKeyStore.java index c82757163..1ddabc89d 100644 --- a/server/src/main/java/com/epam/aidial/core/server/security/ApiKeyStore.java +++ b/server/src/main/java/com/epam/aidial/core/server/security/ApiKeyStore.java @@ -11,6 +11,7 @@ import com.epam.aidial.core.storage.http.HttpStatus; import com.epam.aidial.core.storage.resource.ResourceDescriptor; import com.epam.aidial.core.storage.resource.ResourceTypes; +import com.epam.aidial.core.storage.resource.SystemResourceRegistry; import com.epam.aidial.core.storage.util.RedisUtil; import io.vertx.core.Future; import io.vertx.core.json.JsonObject; @@ -267,7 +268,7 @@ private void validateProjectKey(Key key) { private String toRedisKey(String apiKey) { ResourceDescriptor resource = ResourceDescriptorFactory.fromDecoded( - ResourceTypes.API_KEY_DATA, ResourceDescriptor.API_KEY_DATA_BUCKET, ResourceDescriptor.API_KEY_DATA_LOCATION, apiKey); + ResourceTypes.API_KEY_DATA, SystemResourceRegistry.API_KEY_DATA.bucket(), SystemResourceRegistry.API_KEY_DATA.location(), apiKey); return RedisUtil.redisKey(resource, prefix); } diff --git a/server/src/main/java/com/epam/aidial/core/server/service/ResponseMappingService.java b/server/src/main/java/com/epam/aidial/core/server/service/ResponseMappingService.java index 227495864..3084400b7 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/ResponseMappingService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/ResponseMappingService.java @@ -12,6 +12,7 @@ import com.epam.aidial.core.storage.data.ResourceItemMetadata; import com.epam.aidial.core.storage.resource.ResourceDescriptor; import com.epam.aidial.core.storage.resource.ResourceTypes; +import com.epam.aidial.core.storage.resource.SystemResourceRegistry; import com.epam.aidial.core.storage.service.ResourceService; import com.epam.aidial.core.storage.util.EtagHeader; import io.vertx.core.Vertx; @@ -67,7 +68,7 @@ private Void cleanExpiredMappings() { log.debug("Housekeeping: scanning for expired response mappings"); try { ResourceDescriptor root = ResourceDescriptorFactory.fromDecoded( - ResourceTypes.RESPONSE_MAPPING, ResourceDescriptor.RESPONSE_MAPPINGS_BUCKET, ResourceDescriptor.RESPONSE_MAPPINGS_LOCATION, null); + ResourceTypes.RESPONSE_MAPPING, SystemResourceRegistry.RESPONSE_MAPPINGS.bucket(), SystemResourceRegistry.RESPONSE_MAPPINGS.location(), null); cleanDeploymentSubfolders(root); } catch (Throwable e) { log.warn("Housekeeping: failed to clean expired response mappings", e); @@ -96,7 +97,7 @@ private void cleanDeploymentSubfolders(ResourceDescriptor root) { private void cleanItemsInDeploymentFolder(String deploymentName) { ResourceDescriptor subfolder = ResourceDescriptorFactory.fromDecoded( - ResourceTypes.RESPONSE_MAPPING, ResourceDescriptor.RESPONSE_MAPPINGS_BUCKET, ResourceDescriptor.RESPONSE_MAPPINGS_LOCATION, deploymentName + "/"); + ResourceTypes.RESPONSE_MAPPING, SystemResourceRegistry.RESPONSE_MAPPINGS.bucket(), SystemResourceRegistry.RESPONSE_MAPPINGS.location(), deploymentName + "/"); long now = System.currentTimeMillis(); String token = null; diff --git a/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java b/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java index 0465f60a7..44ebbc592 100644 --- a/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java +++ b/server/src/main/java/com/epam/aidial/core/server/token/TokenStatsTracker.java @@ -6,6 +6,7 @@ import com.epam.aidial.core.server.vertx.AsyncTaskExecutor; import com.epam.aidial.core.storage.resource.ResourceDescriptor; import com.epam.aidial.core.storage.resource.ResourceTypes; +import com.epam.aidial.core.storage.resource.SystemResourceRegistry; import com.epam.aidial.core.storage.service.ResourceService; import com.epam.aidial.core.storage.util.EtagHeader; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; @@ -211,6 +212,6 @@ public record UsageStats(TokenUsage total, List usagePerModel) { private static ResourceDescriptor toResource(String traceId) { return ResourceDescriptorFactory.fromDecoded( - ResourceTypes.DEPLOYMENT_COST_STATS, ResourceDescriptor.DEPLOYMENT_COST_STATS_BUCKET, ResourceDescriptor.DEPLOYMENT_COST_STATS_LOCATION, traceId); + ResourceTypes.DEPLOYMENT_COST_STATS, SystemResourceRegistry.DEPLOYMENT_COST_STATS.bucket(), SystemResourceRegistry.DEPLOYMENT_COST_STATS.location(), traceId); } } diff --git a/server/src/main/java/com/epam/aidial/core/server/util/ResponseIdUtil.java b/server/src/main/java/com/epam/aidial/core/server/util/ResponseIdUtil.java index 1c7a58946..9cdd36bba 100644 --- a/server/src/main/java/com/epam/aidial/core/server/util/ResponseIdUtil.java +++ b/server/src/main/java/com/epam/aidial/core/server/util/ResponseIdUtil.java @@ -2,6 +2,7 @@ import com.epam.aidial.core.storage.resource.ResourceDescriptor; import com.epam.aidial.core.storage.resource.ResourceTypes; +import com.epam.aidial.core.storage.resource.SystemResourceRegistry; import lombok.experimental.UtilityClass; @UtilityClass @@ -24,11 +25,11 @@ public ResourceDescriptor getResponseMappingDescriptor(String dialResponseId) { String uuid = dialResponseId.substring(underscore + 1); String relativePath = deploymentName + "/" + uuid; return ResourceDescriptorFactory.fromDecoded( - ResourceTypes.RESPONSE_MAPPING, ResourceDescriptor.RESPONSE_MAPPINGS_BUCKET, ResourceDescriptor.RESPONSE_MAPPINGS_LOCATION, relativePath); + ResourceTypes.RESPONSE_MAPPING, SystemResourceRegistry.RESPONSE_MAPPINGS.bucket(), SystemResourceRegistry.RESPONSE_MAPPINGS.location(), relativePath); } public ResourceDescriptor getBackgroundJobDescriptor(String jobId) { return ResourceDescriptorFactory.fromDecoded( - ResourceTypes.BACKGROUND_JOB, ResourceDescriptor.BACKGROUND_JOB_BUCKET, ResourceDescriptor.BACKGROUND_JOB_LOCATION, jobId); + ResourceTypes.BACKGROUND_JOB, SystemResourceRegistry.BACKGROUND_JOBS.bucket(), SystemResourceRegistry.BACKGROUND_JOBS.location(), jobId); } } diff --git a/server/src/test/java/com/epam/aidial/core/server/util/ResponseIdUtilTest.java b/server/src/test/java/com/epam/aidial/core/server/util/ResponseIdUtilTest.java index c205faabe..756bb42c7 100644 --- a/server/src/test/java/com/epam/aidial/core/server/util/ResponseIdUtilTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/util/ResponseIdUtilTest.java @@ -2,6 +2,7 @@ import com.epam.aidial.core.storage.resource.ResourceDescriptor; import com.epam.aidial.core.storage.resource.ResourceTypes; +import com.epam.aidial.core.storage.resource.SystemResourceRegistry; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -28,8 +29,8 @@ public void testGetResponseMappingDescriptor() { ResourceDescriptor descriptor = ResponseIdUtil.getResponseMappingDescriptor("dial_gpt-4_abc123"); assertEquals(ResourceTypes.RESPONSE_MAPPING, descriptor.getType()); - assertEquals(ResourceDescriptor.RESPONSE_MAPPINGS_BUCKET, descriptor.getBucketName()); - assertEquals(ResourceDescriptor.RESPONSE_MAPPINGS_LOCATION, descriptor.getBucketLocation()); + assertEquals(SystemResourceRegistry.RESPONSE_MAPPINGS.bucket(), descriptor.getBucketName()); + assertEquals(SystemResourceRegistry.RESPONSE_MAPPINGS.location(), descriptor.getBucketLocation()); assertEquals("gpt-4", descriptor.getParentPath()); assertEquals("abc123", descriptor.getName()); } diff --git a/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java b/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java index cd1c693a9..08af21590 100644 --- a/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/util/SystemBucketLayoutTest.java @@ -4,6 +4,7 @@ import com.epam.aidial.core.storage.resource.ResourceDescriptor; import com.epam.aidial.core.storage.resource.ResourceTypes; import com.epam.aidial.core.storage.resource.StorageLayouts; +import com.epam.aidial.core.storage.resource.SystemResourceRegistry; import com.epam.aidial.core.storage.resource.TenantRootedStorageLayout; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -61,7 +62,7 @@ public void testBackgroundJobRootFolderPath() { @Test public void testApiKeyDataPath() { ResourceDescriptor descriptor = ResourceDescriptorFactory.fromDecoded(ResourceTypes.API_KEY_DATA, - ResourceDescriptor.API_KEY_DATA_BUCKET, ResourceDescriptor.API_KEY_DATA_LOCATION, "some-key"); + SystemResourceRegistry.API_KEY_DATA.bucket(), SystemResourceRegistry.API_KEY_DATA.location(), "some-key"); assertEquals(".system/api_key_data/.api_key_data/some-key", descriptor.getAbsoluteFilePath()); } @@ -69,7 +70,7 @@ public void testApiKeyDataPath() { @Test public void testDeploymentCostStatsPath() { ResourceDescriptor descriptor = ResourceDescriptorFactory.fromDecoded(ResourceTypes.DEPLOYMENT_COST_STATS, - ResourceDescriptor.DEPLOYMENT_COST_STATS_BUCKET, ResourceDescriptor.DEPLOYMENT_COST_STATS_LOCATION, + SystemResourceRegistry.DEPLOYMENT_COST_STATS.bucket(), SystemResourceRegistry.DEPLOYMENT_COST_STATS.location(), "trace-id"); assertEquals(".system/deployment_cost_stats/.deployment_cost_stats/trace-id", descriptor.getAbsoluteFilePath()); diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java index 9c4764af6..2cde51925 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceDescriptor.java @@ -8,7 +8,6 @@ import java.util.Arrays; import java.util.List; import java.util.Objects; -import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nullable; @@ -32,24 +31,6 @@ public class ResourceDescriptor { public static final String USERS_LOCATION_PREFIX = "Users" + PATH_SEPARATOR; public static final String KEYS_LOCATION_PREFIX = "Keys" + PATH_SEPARATOR; - public static final String DEPLOYMENT_COST_STATS_BUCKET = "deployment_cost_stats"; - public static final String DEPLOYMENT_COST_STATS_LOCATION = DEPLOYMENT_COST_STATS_BUCKET + PATH_SEPARATOR; - public static final String BACKGROUND_JOB_BUCKET = "background_jobs"; - public static final String BACKGROUND_JOB_LOCATION = BACKGROUND_JOB_BUCKET + PATH_SEPARATOR; - public static final String RESPONSE_MAPPINGS_BUCKET = "response_mappings"; - public static final String RESPONSE_MAPPINGS_LOCATION = RESPONSE_MAPPINGS_BUCKET + PATH_SEPARATOR; - public static final String API_KEY_DATA_BUCKET = "api_key_data"; - public static final String API_KEY_DATA_LOCATION = API_KEY_DATA_BUCKET + PATH_SEPARATOR; - - /** - * Buckets holding platform-internal runtime state rather than anyone's content. They belong to no - * principal, so a layout has to place them somewhere other than the branches it uses for users and - * projects, and they are listed here so a layout can enumerate them. - */ - public static final Set SYSTEM_LOCATIONS = Set.of( - DEPLOYMENT_COST_STATS_LOCATION, BACKGROUND_JOB_LOCATION, RESPONSE_MAPPINGS_LOCATION, - API_KEY_DATA_LOCATION); - ResourceType type; /** * Resource's name or empty if the resource is a folder diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/StorageLayout.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/StorageLayout.java index d37fe6ed5..d0a037587 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/StorageLayout.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/StorageLayout.java @@ -9,5 +9,8 @@ public interface StorageLayout { String resolveLocationPrefix(String bucketLocation); + /** + * @param typeGroup {@link ResourceType#group()} — the storage folder name of the resource type + */ String resolveTypeFolder(String typeGroup); } diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/SystemResourceRegistry.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/SystemResourceRegistry.java new file mode 100644 index 000000000..29e8da7cd --- /dev/null +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/SystemResourceRegistry.java @@ -0,0 +1,37 @@ +package com.epam.aidial.core.storage.resource; + +/** + * The platform-internal buckets holding runtime state rather than anyone's content. They belong to no + * principal, so a layout has to place them somewhere other than the branches it uses for users and + * projects; this registry is how a layout enumerates them. + */ +public enum SystemResourceRegistry { + + DEPLOYMENT_COST_STATS("deployment_cost_stats"), + BACKGROUND_JOBS("background_jobs"), + RESPONSE_MAPPINGS("response_mappings"), + API_KEY_DATA("api_key_data"); + + private final String bucket; + + SystemResourceRegistry(String bucket) { + this.bucket = bucket; + } + + public String bucket() { + return bucket; + } + + public String location() { + return bucket + ResourceDescriptor.PATH_SEPARATOR; + } + + public static boolean isSystemLocation(String location) { + for (SystemResourceRegistry entry : values()) { + if (entry.location().equals(location)) { + return true; + } + } + return false; + } +} diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformer.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformer.java index eca32dfe8..1a0fdecf8 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformer.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformer.java @@ -27,7 +27,7 @@ public class TenantLayoutTransformer { private static final String TENANT_TREE_ROOT = ""; /** - * Where the system buckets of {@link ResourceDescriptor#SYSTEM_LOCATIONS} land — at the root, above any + * Where the system buckets of {@link SystemResourceRegistry} land — at the root, above any * tenant, which preserves whole-bucket scans and globally unique keys; the bucket name is kept so the * mapping stays reversible. */ @@ -52,7 +52,7 @@ public String toTenantLocation(String legacyLocation, String tenantId) { return TENANT_TREE_ROOT; } - if (ResourceDescriptor.SYSTEM_LOCATIONS.contains(legacyLocation)) { + if (SystemResourceRegistry.isSystemLocation(legacyLocation)) { return SYSTEM_SEGMENT + legacyLocation; } @@ -86,7 +86,7 @@ public String toLegacyLocation(String tenantLocation, String tenantId) { if (tenantLocation.startsWith(SYSTEM_SEGMENT)) { String system = tenantLocation.substring(SYSTEM_SEGMENT.length()); - if (!ResourceDescriptor.SYSTEM_LOCATIONS.contains(system)) { + if (!SystemResourceRegistry.isSystemLocation(system)) { throw new IllegalArgumentException("Unknown system bucket location: " + tenantLocation); } return system; diff --git a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformerTest.java b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformerTest.java index 086433047..33a859ce9 100644 --- a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformerTest.java +++ b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantLayoutTransformerTest.java @@ -89,9 +89,9 @@ public void testSystemLocations() { */ @Test public void testEverySystemLocationRoundTrips() { - for (String legacy : ResourceDescriptor.SYSTEM_LOCATIONS) { - String tenant = TenantLayoutTransformer.toTenantLocation(legacy, TENANT); - assertEquals(legacy, TenantLayoutTransformer.toLegacyLocation(tenant, TENANT)); + for (SystemResourceRegistry entry : SystemResourceRegistry.values()) { + String tenant = TenantLayoutTransformer.toTenantLocation(entry.location(), TENANT); + assertEquals(entry.location(), TenantLayoutTransformer.toLegacyLocation(tenant, TENANT)); } } @@ -100,9 +100,9 @@ public void testEverySystemLocationRoundTrips() { */ @Test public void testSystemLocationsAreTenantIndependent() { - for (String legacy : ResourceDescriptor.SYSTEM_LOCATIONS) { - assertEquals(TenantLayoutTransformer.toTenantLocation(legacy, TENANT), - TenantLayoutTransformer.toTenantLocation(legacy, "another-tenant")); + for (SystemResourceRegistry entry : SystemResourceRegistry.values()) { + assertEquals(TenantLayoutTransformer.toTenantLocation(entry.location(), TENANT), + TenantLayoutTransformer.toTenantLocation(entry.location(), "another-tenant")); } } diff --git a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java index d6a1f9351..6a836043b 100644 --- a/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java +++ b/storage/src/test/java/com/epam/aidial/core/storage/resource/TenantRootedStorageLayoutTest.java @@ -47,8 +47,8 @@ public void testSystemBucketPath() { StorageLayouts.useLayout(layout); ResourceDescriptor job = new ResourceDescriptor(ResourceTypes.BACKGROUND_JOB, "job-1", - List.of(), ResourceDescriptor.BACKGROUND_JOB_BUCKET, - ResourceDescriptor.BACKGROUND_JOB_LOCATION, false); + List.of(), SystemResourceRegistry.BACKGROUND_JOBS.bucket(), + SystemResourceRegistry.BACKGROUND_JOBS.location(), false); assertEquals(".system/background_jobs/.background_jobs/job-1", job.getAbsoluteFilePath()); assertEquals("background_jobs/background_jobs/job-1", job.getLegacyFilePath());