Only reset embedded Central DB schema on a genuinely uninitialized schema and fix merge conflict issue - #211
Merged
Conversation
PR OpenRune#204 (Demonic gorillas) dropped the [[npc]] header before the pre-existing saltfish NPC block, so its id/inherit/contentGroup/ moveRestrict/wanderRange keys got parsed as part of the previous entry's [npc.params] table instead. buildCache failed outright as a result (ParamSerializer expects every params key to be an integer param id).
…zed schema CentralEmbeddedLifecycle.startIfConfigured caught any Throwable from Central's startup and unconditionally dropped the entire public schema of the embedded Postgres DB, treating every failure as schema corruption. A plain port-bind conflict (or any other unrelated startup failure) took the same DROP SCHEMA public CASCADE path as a real missing schema. Narrowed the reset to only fire when the exception's cause chain contains a SQLException whose SQLSTATE indicates Central's tables genuinely don't exist yet: 42P01 (undefined_table), 42703 (undefined_column), 3F000 (invalid_schema_name). Everything else now propagates and fails startup without touching the database, matching the reported port-conflict case. Moved the classification logic into a standalone CentralStartupFailure object so it's testable without standing up the lifecycle's real dependencies; CentralStartupFailureTest covers the bind-conflict case, an unrelated exception, an unrelated SQL error, and all three genuine uninitialized-schema SQLSTATEs (wrapped and unwrapped).
jelqscape
changed the title
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
2 participants
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.
Summary
CentralEmbeddedLifecycle.startIfConfiguredcaught anyThrowablefrom Central's embedded startup and unconditionally ranDROP SCHEMA public CASCADEagainst the embedded Postgres DB, treating every startup failure as schema corruption. A plain port-bind conflict (or any other unrelated startup failure) took the same "wipe the whole database" path as an actually corrupted/uninitialized schema - not scoped to the case the comment (// Dev recovery) claims it's for.Fix
Narrowed the reset to only fire when the failure's cause chain contains a
SQLExceptionwhose SQLSTATE indicates Central's expected tables genuinely don't exist yet:42P01- undefined_table42703- undefined_column3F000- invalid_schema_nameEverything else (bind conflicts, generic runtime errors, unrelated SQL errors like connection-refused) now just propagates and fails startup normally, without touching the database.
The classification logic (
CentralStartupFailure.indicatesUninitializedSchema) is pulled out into its own object so it's testable without standing up the lifecycle's real dependencies.Also included
main'snpcs.tomlcurrently failsbuildCacheoutright - a recent PR dropped the[[npc]]separator before an existing NPC block, so TOML parsing merged the two entries and crashed on decode (params key 'id' must be an integer param id). Fixed alongside this since it was blocking any fresh build/test of the actual Central fix.Testing
CentralStartupFailureTest: verifies a bind conflict, an unrelated exception, and an unrelated SQL error (connection-refused) do not trigger a reset; verifies all three genuine uninitialized-schema SQLSTATEs do, both wrapped and unwrapped.🤖 Generated with Claude Code