Remove backward compatibility code for meta repository migration #1210
Conversation
Motivation: Following the successful migration of the `meta` repository's contents to the `dogma` repository in issue line#1128, a backward-compatibility layer was temporarily kept in place. This logic allowed the system to use the old `meta` repository if the migration was not yet complete. Now that the migration has been finalized across all environments, this compatibility code is no longer necessary. This commit removes the obsolete code to simplify the codebase and rely exclusively on the `dogma` repository. Modifications: - Removed the conditional logic that checked the migration status to decide between using the `meta` or `dogma` repository. - However, it's still prohibited to create or use the `meta` repository. Result: - The codebase is now simpler and easier to maintain.
WalkthroughThis pull request removes the meta-to-dogma repository migration infrastructure, including the migration service, plugin, and related commands. It updates tests to use REPO_DOGMA instead of REPO_META and simplifies the project initialization logic by eliminating meta repository handling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesServiceTest.java (1)
158-167: Clarify or update the backward compatibility comment.The comment states "This is needed for the backward compatibility test," but given that this PR removes the meta→dogma migration backward compatibility layer, this phrasing may be misleading. If this comment refers to supporting both project-level and repo-level credentials (which the parameterized tests verify), consider updating it to clarify what backward compatibility is being maintained here.
client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogmaTest.java (1)
45-70: Rename test methods to reflect the dogma repository.Both test method names still reference "MetaRepository," but after the migration, they now test
Project.REPO_DOGMA. This creates confusion for future developers who might misunderstand what repository these tests are validating.Apply this diff to rename the test methods:
- void pushFileToMetaRepositoryShouldFail() throws UnknownHostException { + void pushFileToDogmaRepositoryShouldFail() throws UnknownHostException { final CentralDogma client = new ArmeriaCentralDogmaBuilder() .host(dogma.serverAddress().getHostString(), dogma.serverAddress().getPort()) .build(); assertThatThrownBy(() -> client.forRepo("foo", Project.REPO_DOGMA) .commit("summary", Change.ofJsonUpsert("/bar.json", "{ \"a\": \"b\" }")) .push() .join()) .isInstanceOf(CompletionException.class) .hasCauseInstanceOf(InvalidPushException.class); } - void pushMirrorsJsonFileToMetaRepository() throws UnknownHostException { + void pushMirrorsJsonFileToDogmaRepository() throws UnknownHostException { final CentralDogma client = new ArmeriaCentralDogmaBuilder() .host(dogma.serverAddress().getHostString(), dogma.serverAddress().getPort()) .build(); final PushResult result = client.forRepo("foo", Project.REPO_DOGMA) .commit("summary", Change.ofJsonUpsert("/repos/foo/mirrors/foo.json", "{}")) .push() .join(); assertThat(result.revision().major()).isPositive(); }server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProject.java (4)
108-118: Open-existing path now consistently derives state from dogma metadataFor non-internal projects, deriving
creationTimeMillisandauthorfrommetadata().creation()inREPO_DOGMAand wiringmetaRepotoDefaultMetaRepository(repos.get(REPO_DOGMA))makes the dogma repo the single source of truth and removes the old meta-repo dependency. The INTERNAL_PROJECT_DOGMA branch remains isolated and avoids attaching a metadata listener or metaRepo, which is appropriate.If you expect any projects without
metadata.jsonafter migration, consider a more user-friendly failure (e.g., a specific exception) instead of a genericCentralDogmaExceptionfrominitialMetadata().
176-180: Reserved repo creation is minimal and race-safe
createReservedReposnow createsREPO_DOGMAon demand withAuthor.SYSTEMand honors theencryptDogmaRepoflag, swallowingRepositoryExistsExceptionfor races. This is a straightforward replacement for any prior meta/dogma combo and looks correct.You might want to log when the dogma repo is auto-created to aid debugging startup/bootstrapping issues.
236-246: initialMetadata assumes per-project dogma metadata is present
initialMetadata()now asserts the project is notINTERNAL_PROJECT_DOGMAand readsMETADATA_JSONfrom the project’s ownREPO_DOGMA, updatinglastMetadataRevisionandprojectMetadata. This matches the new design where all metadata resides in dogma rather than a separate meta repo, and it ensures the listener has an initial revision to compare against.If there’s any chance
METADATA_JSONis missing (e.g., partially migrated instances), consider handling that with a clearer exception or on-the-fly initialization rather than relying onget()to fail.
253-278: Metadata listener invariants are clear and dogma-onlyThe listener now explicitly asserts the project is not the internal dogma project and watches
METADATA_JSONinREPO_DOGMA, updatinglastMetadataRevisionandprojectMetadataonly when a newer revision arrives. Combined with the constructors settinglastMetadataRevisionbefore registering the listener, theassert lastMetadataRevision != nullis safe and keeps state consistent.Given Java assertions are often disabled in production, consider converting the
assert lastMetadataRevision != nullto a runtime check with a clear error message to avoid a potentialNullPointerExceptionif invariants are broken by future changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogmaTest.java(2 hunks)it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/ForceRefUpdateTest.java(3 hunks)it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/GitMirrorAuthTest.java(1 hunks)it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/GitMirrorIntegrationTest.java(2 hunks)it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/LocalToRemoteGitMirrorTest.java(2 hunks)it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/ZoneAwareMirrorTest.java(1 hunks)it/server/src/test/java/com/linecorp/centraldogma/it/RepositoryManagementTest.java(1 hunks)server/src/main/java/com/linecorp/centraldogma/server/command/Command.java(0 hunks)server/src/main/java/com/linecorp/centraldogma/server/command/StandaloneCommandExecutor.java(0 hunks)server/src/main/java/com/linecorp/centraldogma/server/internal/storage/MigratingMetaToDogmaRepositoryService.java(0 hunks)server/src/main/java/com/linecorp/centraldogma/server/internal/storage/MigratingMetaToDogmaRepositoryServicePlugin.java(0 hunks)server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProject.java(6 hunks)server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProjectManager.java(1 hunks)server/src/main/java/com/linecorp/centraldogma/server/storage/project/Project.java(1 hunks)server/src/main/resources/META-INF/services/com.linecorp.centraldogma.server.plugin.Plugin(0 hunks)server/src/test/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1Test.java(2 hunks)server/src/test/java/com/linecorp/centraldogma/server/internal/api/MigrateToEncryptedRepositoryTest.java(0 hunks)server/src/test/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1Test.java(3 hunks)server/src/test/java/com/linecorp/centraldogma/server/internal/storage/MigratingMetaToDogmaRepositoryServiceTest.java(0 hunks)server/src/test/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProjectTest.java(1 hunks)xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesServiceTest.java(1 hunks)
💤 Files with no reviewable changes (7)
- server/src/test/java/com/linecorp/centraldogma/server/internal/api/MigrateToEncryptedRepositoryTest.java
- server/src/test/java/com/linecorp/centraldogma/server/internal/storage/MigratingMetaToDogmaRepositoryServiceTest.java
- server/src/main/java/com/linecorp/centraldogma/server/internal/storage/MigratingMetaToDogmaRepositoryService.java
- server/src/main/java/com/linecorp/centraldogma/server/command/Command.java
- server/src/main/resources/META-INF/services/com.linecorp.centraldogma.server.plugin.Plugin
- server/src/main/java/com/linecorp/centraldogma/server/command/StandaloneCommandExecutor.java
- server/src/main/java/com/linecorp/centraldogma/server/internal/storage/MigratingMetaToDogmaRepositoryServicePlugin.java
🧰 Additional context used
🧬 Code graph analysis (2)
server/src/main/java/com/linecorp/centraldogma/server/storage/project/Project.java (1)
server/src/main/java/com/linecorp/centraldogma/server/command/ResetMetaRepositoryCommand.java (1)
Deprecated(31-64)
server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProject.java (1)
common/src/main/java/com/linecorp/centraldogma/common/Author.java (1)
Author(32-135)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build-ubuntu-latest-jdk-17-min-java-11
- GitHub Check: build-windows-latest-jdk-21
- GitHub Check: build-ubuntu-latest-jdk-11
- GitHub Check: build-ubuntu-latest-jdk-21-snapshot
- GitHub Check: build-ubuntu-latest-jdk-17-coverage
- GitHub Check: build-macos-latest-jdk-21
- GitHub Check: lint
- GitHub Check: docker-build
- GitHub Check: flaky-tests
🔇 Additional comments (15)
it/server/src/test/java/com/linecorp/centraldogma/it/RepositoryManagementTest.java (1)
114-114: LGTM! Correctly reflects the completed meta → dogma migration.The removal of
Project.REPO_METAfrom the expected repository set correctly reflects the completion of the meta repository migration. The assertion now properly validates that only theREPO_DOGMAsystem repository exists alongside the test repositories, which aligns with the PR objectives.xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesServiceTest.java (1)
199-206: LGTM! Error message correctly references dogma repository.The test assertion has been appropriately updated to expect credential lookups in
@xds/dogmainstead of@xds/meta, consistent with the completed migration.server/src/main/java/com/linecorp/centraldogma/server/storage/project/Project.java (1)
50-51: LGTM - Proper deprecation strategy.The approach of deprecating REPO_META while keeping the constant is correct, as it prevents users from creating new repositories with the reserved name "meta".
it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/GitMirrorAuthTest.java (1)
111-111: LGTM - Consistent repository migration.The change from
REPO_METAtoREPO_DOGMAcorrectly aligns with the meta→dogma migration.it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/ZoneAwareMirrorTest.java (1)
174-174: LGTM - Consistent repository migration.The change from
REPO_METAtoREPO_DOGMAcorrectly aligns with the meta→dogma migration.it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/LocalToRemoteGitMirrorTest.java (1)
370-385: LGTM - Consistent repository migration.Both
forRepo()calls inpushMirrorSettings()correctly changed fromREPO_METAtoREPO_DOGMA, maintaining consistency across credential and mirror configuration commits.it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/ForceRefUpdateTest.java (1)
109-220: LGTM - Consistent repository migration.All three
forRepo()calls correctly changed fromREPO_METAtoREPO_DOGMAacross cleanup, credential setup, and mirror configuration methods.server/src/test/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProjectTest.java (1)
40-49: LGTM - Appropriate migration verification.The test correctly verifies that only the dogma repository is created and that
metaRepo()now returns the dogma repository, confirming the meta→dogma consolidation.server/src/test/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1Test.java (1)
138-158: LGTM - Consistent test migration.The test correctly updated from verifying that the
metarepository cannot be pushed to, to verifying that thedogmarepository (now the internal repository) cannot be pushed to. Both the method name and endpoint are consistently updated.it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/GitMirrorIntegrationTest.java (1)
488-519: LGTM - Consistent repository migration.Both
forRepo()calls inpushMirrorSettings()correctly changed fromREPO_METAtoREPO_DOGMA, maintaining consistency across credential and mirror configuration commits.server/src/test/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1Test.java (3)
241-245: Test now correctly asserts dogma repository is non-removableSwitching the deletion test from
REPO_METAtoProject.REPO_DOGMAmatches the new rule that dogma is the reserved internal repo and must not be deleted; the 403 assertion remains appropriate and keeps the contract explicit.
298-310: Internal repository status test correctly targets dogmaUpdating
updateInternalRepositoryStatusto toggleRepositoryStatusonProject.REPO_DOGMAaligns with metadata now living in dogma. The flow—set READ_ONLY, assert credential creation fails with 503, then restore ACTIVE and assert credentials can be created—is still valid and exercises the right internal repo.
417-419: Adjusted remaining-repositories assertion is consistent with removed meta repoThe updated comment and
assertThat(jsonNode).hasSize(2)correctly reflect that onlydogmaandtrustinremain after removing the two user repos. This matches the new world wheremetais gone and avoids overspecifying exact repository names in the assertion.server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProjectManager.java (1)
83-87: Constructor call matches new DefaultProject signatureThe updated
createChildnow passes arguments in the correct order for the simplifiedDefaultProjectconstructor (rootDir, repositoryWorker, purgeWorker, creationTimeMillis, author, cache, encryptionStorageManager, encrypt). This removes the dependency on an internal dogma project initializer cleanly without changing behavior.server/src/main/java/com/linecorp/centraldogma/server/internal/storage/project/DefaultProject.java (1)
133-157: New-project constructor correctly bootstraps dogma and metadataThe create-new-project path now:
- Ensures
REPO_DOGMAexists viacreateReservedRepos.- For non-internal projects, initializes metadata in
REPO_DOGMA, attaches a listener, and wraps that repo inDefaultMetaRepository.- Then assigns the ctor’s
creationTimeMillisandauthordirectly.This keeps internal-project handling separate while guaranteeing that non-internal projects have a usable metaRepo tied to dogma from the outset.
| * The meta repository was used to store metadata, but it's no longer used anymore. | ||
| * We still keep this constant to prevent creating a repository with this name. | ||
| * | ||
| * @deprecated meta repository is not used anymore. |
There was a problem hiding this comment.
Fix grammatical error in Javadoc.
The phrase "no longer used anymore" is redundant.
Apply this diff to fix the grammar:
- * The meta repository was used to store metadata, but it's no longer used anymore.
+ * The meta repository was used to store metadata, but it is no longer used.
* We still keep this constant to prevent creating a repository with this name.
*
* @deprecated meta repository is not used anymore.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * The meta repository was used to store metadata, but it's no longer used anymore. | |
| * We still keep this constant to prevent creating a repository with this name. | |
| * | |
| * @deprecated meta repository is not used anymore. | |
| * The meta repository was used to store metadata, but it is no longer used. | |
| * We still keep this constant to prevent creating a repository with this name. | |
| * | |
| * @deprecated meta repository is not used anymore. |
🤖 Prompt for AI Agents
In
server/src/main/java/com/linecorp/centraldogma/server/storage/project/Project.java
around lines 45 to 48, the Javadoc contains the redundant phrase "no longer used
anymore"; edit the comment to remove redundancy by changing the sentence to
something like "The meta repository was used to store metadata, but it is no
longer used." and update the @deprecated line to read "@deprecated The meta
repository is no longer used." so the wording is concise and grammatically
correct.
3 participants
Motivation:
Following the successful migration of the
metarepository's contents to thedogmarepository in issue #1128, a backward-compatibility layer was temporarily kept in place. This logic allowed the system to use the oldmetarepository if the migration was not yet complete. Now that the migration has been finalized across all environments, this compatibility code is no longer necessary. This commit removes the obsolete code to simplify the codebase and rely exclusively on thedogmarepository.Modifications:
metaordogmarepository.metarepository.Result:
Summary by CodeRabbit
Release Notes
Deprecations
Changes