Skip to content

Add map_version.folder_name column, derive filename#332

Merged
Brutus5000 merged 2 commits into
developfrom
feature/map-version-folder-name
Jun 24, 2026
Merged

Add map_version.folder_name column, derive filename#332
Brutus5000 merged 2 commits into
developfrom
feature/map-version-folder-name

Conversation

@Brutus5000

@Brutus5000 Brutus5000 commented Jun 22, 2026

Copy link
Copy Markdown
Member

Why

Every /data/map request that filters on versions.folderName (e.g. the client's "find latest version by folder name" lookup) takes a flat ~5s. Root cause: in the API folderName is a transient/computed attribute derived in-memory from filename in a @PostLoad enricher, so Elide cannot translate versions.folderName == X into SQL. It falls back to loading and enriching the entire map_version table on every request and filtering in memory.

filename (maps/<folder>.zip) just stores the folder name with a fixed prefix/suffix. This migration makes the folder name a real, indexable column.

What

  • Add folder_name (NOT NULL, UNIQUE) as the new source of truth and backfill it from filename (prefix/suffix-agnostic).
  • Drop the old real filename column and re-add it as a VIRTUAL generated column CONCAT('maps/', folder_name, '.zip') — no storage, recomputed on read.

filename is kept (generated) purely for backwards compatibility: faf-server (game_service, ladder_service, coop lookups), faf-rust-replayserver (get_game_stat_row) and any API clients reading the filename attribute keep seeing the identical value with no changes. Its UNIQUE index is dropped as redundant — it is a 1:1 function of the already-UNIQUE folder_name.

Coordinated change

The API side (entity mapping, upload service, enricher) lives in faf-java-api on branch feature/map-version-folder-name, which pins the migrations image to v144. This PR must be released as v144 for that branch's integration tests to pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Optimized database schema structure to improve data consistency and reliability.

Make the map folder name a real, indexable column. Previously only
`filename` (e.g. `maps/scmp_001.v0001.zip`) was stored and the API derived
the folder name from it in-memory, so `versions.folderName == X` filters
could not be pushed to SQL and forced a full table scan on every map lookup.

`folder_name` becomes the source of truth; `filename` is kept as a VIRTUAL
generated column so faf-server, the replay server and existing API clients
keep seeing the identical `maps/<folder>.zip` value.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@Brutus5000, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 56 minutes and 11 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4edf0548-4165-49fb-bc44-88f2a36bf02f

📥 Commits

Reviewing files that changed from the base of the PR and between 49db144 and b53426c.

📒 Files selected for processing (1)
  • test-data.sql
📝 Walkthrough

Walkthrough

A new SQL migration refactors the map_version table to add a persisted folder_name column backfilled from filename, enforces it as NOT NULL with a unique constraint, then drops the stored filename column and replaces it with a VIRTUAL generated column derived as maps/<folder_name>.zip.

Changes

map_version Schema Refactor

Layer / File(s) Summary
Add folder_name column, backfill, and constrain
migrations/V144__map_version_folder_name.sql
Adds the folder_name column after filename, backfills it by extracting the substring between the last / and .zip from existing filename values, then sets it NOT NULL and adds a unique constraint.
Replace stored filename with VIRTUAL generated column
migrations/V144__map_version_folder_name.sql
Drops the old stored filename column and reintroduces filename as a VIRTUAL generated column computed as maps/<folder_name>.zip, preserving the output format without storage or indexing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A folder by name is more swift to find,
No slicing through paths, no zip left behind.
The filename stays, but as virtual art,
While folder_name plays the canonical part.
Hop hop, the schema is tidy and bright! 🗺️

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding a folder_name column and refactoring filename as a derived column.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/map-version-folder-name

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

filename is now a generated column, so test-data.sql must populate the new
NOT NULL folder_name instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@migrations/V144__map_version_folder_name.sql`:
- Line 13: The folder_name column is set to varchar(200) but the generated
filename concatenates it with fixed characters (maps/ prefix and .zip suffix
totaling 9 characters), which can exceed the filename column's varchar(200)
limit if folder_name exceeds 191 characters. Change the folder_name column
definition in the ALTER TABLE map_version statement from varchar(200) to
varchar(191) to prevent potential overflow. Also apply the same reduction to the
folder_name column definitions at lines 22-23 and line 30 in this migration
file.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 15f3d33e-5559-44b5-ab11-d004948e31d1

📥 Commits

Reviewing files that changed from the base of the PR and between de989b4 and 49db144.

📒 Files selected for processing (1)
  • migrations/V144__map_version_folder_name.sql

-- and existing API clients continue to see the identical `maps/<folder>.zip` value.

-- 1. add the new source-of-truth column
ALTER TABLE map_version ADD COLUMN folder_name varchar(200) NULL AFTER filename;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align column lengths to prevent derived filename overflow.

folder_name is allowed up to 200 chars, but generated filename is varchar(200) and adds 9 fixed chars (maps/ + .zip). Any folder_name longer than 191 can fail writes (or truncate, depending on SQL mode).

Suggested fix
-ALTER TABLE map_version ADD COLUMN folder_name varchar(200) NULL AFTER filename;
+ALTER TABLE map_version ADD COLUMN folder_name varchar(191) NULL AFTER filename;
...
 ALTER TABLE map_version
-  MODIFY folder_name varchar(200) NOT NULL,
+  MODIFY folder_name varchar(191) NOT NULL,
   ADD UNIQUE KEY folder_name (folder_name);

Also applies to: 22-23, 30-30

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@migrations/V144__map_version_folder_name.sql` at line 13, The folder_name
column is set to varchar(200) but the generated filename concatenates it with
fixed characters (maps/ prefix and .zip suffix totaling 9 characters), which can
exceed the filename column's varchar(200) limit if folder_name exceeds 191
characters. Change the folder_name column definition in the ALTER TABLE
map_version statement from varchar(200) to varchar(191) to prevent potential
overflow. Also apply the same reduction to the folder_name column definitions at
lines 22-23 and line 30 in this migration file.

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.

1 participant