Skip to content

refactor: resolve tile keys in the tiles-deletion strategy (MAPCO-11269) - #39

Merged
almog8k merged 6 commits into
masterfrom
refactor/strategy-owns-key-format-MAPCO-11269
Sep 3, 2026
Merged

almog8k merged 6 commits into
masterfrom
refactor/strategy-owns-key-format-MAPCO-11269

Conversation

@almog8k

@almog8k almog8k commented Sep 2, 2026 •

Copy link
Copy Markdown
Collaborator
Question Answer
Bug fix ✖
New feature ✖
Breaking change ✖
Deprecations ✖
Documentation ✖
Tests added ✔
Chore ✔

Related issues: MAPCO-11269

Further information:

Tile-key formatting moves from the storage providers to the tiles-deletion strategy: IStorageProvider.keysForRange is replaced by resolveTileKeyGenerator (cleaner/strategies/tileKeys.ts), which picks the generator by storageProvider. No behaviour change — the emitted keys are identical.

  • FS and S3 had byte-identical implementations: the format belongs to the key family, not the provider.
  • Drops the cast the strategy needed to call keysForRange, and the TilesDeletionParams import from IStorageProvider.

@github-actions

github-actions Bot commented Sep 2, 2026 •

Copy link
Copy Markdown

🎫 Related Jira Issue: MAPCO-11269

Comment thread src/cleaner/utils/tileKeys.ts
Comment thread src/cleaner/strategies/tilesDeletionStrategy.ts Outdated
Comment thread src/cleaner/strategies/tilesDeletionStrategy.ts Outdated
Comment thread src/cleaner/utils/path.ts Outdated
resolveTileKeyGenerator is a pure params -> generator function with no DI
and no strategy state, so it does not belong under strategies/. Move it
next to the generators it composes in utils/path.ts.

Note this diverges from the ticket's stated design of adding
keysForRange() to IStorageProvider: S3 and FS emit byte-identical keys,
so the interface method would have two copy-pasted implementations, and
REDIS has no provider to implement it until MAPCO-11263. Revisit when the
Redis provider lands.

No behaviour change.
@almog8k
almog8k merged commit 806c4d8 into master Sep 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants