feat: add tenant-rooted storage layout behind a setting #1863 - #1866
Open
DmytroZaichenkoDev wants to merge 7 commits into
Open
DmytroZaichenkoDev wants to merge 7 commits into
DmytroZaichenkoDev wants to merge 7 commits into
Conversation
Converts the bucket location prefix and the resource-type folder between the legacy bucket-rooted layout and the tenant-rooted one, in both directions. No callers yet: this is the first step of the addressing work, kept separate so the conversion can be reviewed and tested on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ResourceDescriptor composed the bucket location and the resource-type folder inline, in two places. Both now go through a StorageLayout, so the layout can be swapped in one spot later. The only implementation is the legacy one, which returns both parts verbatim, so physical paths are unchanged. ResourceDescriptorPathTest pins them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Adds the tenant-rooted StorageLayout and selects the layout at start-up from storageLayout.tenantRooted, which defaults to false, so physical paths are unchanged unless a deployment opts in. The active layout is process-wide: ResourceDescriptor is constructed everywhere and carries no configuration of its own. AiDial sets it unconditionally on every start so the layout is fully determined by settings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Enables storageLayout.tenantRooted through the real stack and exercises a conversation round trip, listing and deletion, then asserts the blob is stored under the tenant root in a reserved type folder. Unit tests cover the path conversion in isolation; this is what shows the descriptor, the cache and the blob store agreeing on the same paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 31, 2026
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
DmytroZaichenkoDev
marked this pull request as ready for review
September 3, 2026 15:04
DmytroZaichenkoDev
requested review from
Oleksii-Klimov and
astsiapanay
as code owners
September 3, 2026 15:04
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
astsiapanay
reviewed
Sep 4, 2026
| return active; | ||
| } | ||
|
|
||
| public static void useLayout(StorageLayout layout) { |
Collaborator
There was a problem hiding this comment.
Can we switch between layouts in runtime?
Contributor
Author
There was a problem hiding this comment.
No, by design: the layout is fixed at startup before any resource I/O, because a runtime flip re-addresses live data out from under the Redis cache, the dirty write-behind queue, and per-resource locks (all keyed by physical path). The setter exists so the comparison tests can boot two instances in one JVM. Per-bucket migration in P2 will come as a composite layout consulted per resolution, not a runtime switch.
Collaborator
There was a problem hiding this comment.
That's a dangerous method by nature. I'm thinking how we can make us safe. Can we come up with more robust mechanism so anyone couldn't change the layout in runtime?
DmytroZaichenkoDev
pushed a commit
that referenced
this pull request
Sep 7, 2026
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 <noreply@anthropic.com>
DmytroZaichenkoDev
pushed a commit
that referenced
this pull request
Sep 7, 2026
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 1 of 2 for #1863 (addressing only). Part 2 is #1868.
What
Adds a tenant-rooted storage layout, off by default, and routes physical path composition through it.
TenantLayoutTransform— the conversion, total and reversible in both directions.StorageLayout+LegacyStorageLayout—ResourceDescriptorcomposed the bucket location and the resource-type folder inline in two places (getAbsoluteFilePathandresolveByPath); both now go through one layout.TenantRootedStorageLayout+StorageLayouts— the new layout and the holder for the active one.AiDialselects it at start-up from a new settings section:Invariant
Off by default, so physical paths are unchanged.
ResourceDescriptorPathTestpins the current paths;TenantRootedLayoutApiTestdrives the resource API with the flag on through the real stack — round trip, listing, deletion, and the blob landing under.org/test-tenant/…in a reserved type folder — which is the only way to show the descriptor, the cache and the blob store agree on the same paths.Notes for review
ResourceDescriptoris constructed in ~63 files and carries no configuration, so there is nowhere else to put it without touching every construction site. Open to alternatives.AiDialsets the layout unconditionally on every start, including the legacy case, so it is always determined by settings rather than inherited from a previousstart()in the same JVM..applications,.toolsets) rather than both mapping to.deploymentsas the design sketch shows: merging two groups into one folder is not invertible, and a working reverse transform is required for migration.storageLayoutis a new top-level section rather than a field onstorage—Storageis jclouds provider configuration, this is a logical concern — and deliberately static settings, not hot-reloaded config.defaultTenantdefaults to"default"; that string becomes a physical path segment, so it is worth an explicit decision.Users/andKeys/prefixes are repeated inTenantLayoutTransformbecausestoragecannot depend onserver'sBucketBuilder.Not included
No migration sits behind this flag. Enabling it against populated storage re-addresses everything and the existing data is not reachable at the new paths — the README says so. Three things must land before it can be flipped anywhere real: re-encryption of path-bound secrets (see #1868 for the first half), a decision on invitation ids, which embed the physical path, and a measurement of the existing path-length distribution against the 900-byte ceiling, which tenant-rooting eats into.