Gh 4482 implement external table transport for oracle - #4558
Merged
jeremydmiller merged 4 commits intoSep 23, 2026
Merged
jeremydmiller merged 4 commits into
jeremydmiller merged 4 commits into
Conversation
added 3 commits
September 23, 2026 18:03
Create a set of compliance tests for External Db Table transport in order to unify the testing and make it easier to add more providers. - Slight refactoring of the set upfor ExternalDbTransportStore in the RDBMS solution to group the related functions into a single interface/partial class and enable the generic compliance tests - Refactoring the existing Postgres and SqlServer external transport tests to use the new compliance suite - Implemented the compliance test suite for MySql and Sqlite - - Fixed an issue for MySql implementation where if Wolverine creates the external table queue, the body was created as JSON instead of LONGBLOB like other json columns. - - Fixed an issue for the Sqlite implementation where the schema name was applied incorrectly rather than prefixed.
Implemented the External Table transport configuration for Oracle. Created test suite using ExternalTableTransportCompliance
jeremydmiller
added a commit
that referenced
this pull request
Sep 23, 2026
`unknown_tenant_problem_details_4516.without_the_opt_in_the_unknown_tenant_still_escapes` went red on CI the moment #4551 merged, and it takes the whole `test` job with it -- so every open PR is red regardless of its contents (#4553, #4557, #4558 at the time of writing, two of them from contributors). The test was self contradictory. It asked Alba to assert a 404 inside a scenario it expected to THROW: await Should.ThrowAsync<UnknownTenantIdException>(async () => await host.Scenario(x => { x.Get.Url("/gh4516/tenanted?tenantId=ghost"); x.StatusCodeShouldBe(404); // <- asserted on a request expected to blow up })); Locally the UnknownTenantIdException propagated before Alba evaluated its assertions, so the expected exception won and the test passed. On CI the host turned the exception into a 500 first, so Alba's own 404 assertion fired and raised ScenarioAssertionException instead of UnknownTenantIdException -- a different type, so Should.ThrowAsync failed. The control's actual purpose is narrower than what it asserted: it exists to show that MapUnknownTenantToNotFound() is what produces the 404, not something else in the pipeline. It is not a claim about HOW an unmapped failure surfaces, and that is exactly the part that varies by environment. It now uses IgnoreStatusCode() and asserts only that the response is not a mapped 404, tolerating the other surfacing -- the exception escaping the scenario -- and nothing else. Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
jeremydmiller
added a commit
that referenced
this pull request
Sep 23, 2026
Non-behavioral tidy-up of the external database table transport landed in #4558. - ExternalMessageTable.BuildListenerAsync gated on IMessageDatabase while the listener it builds requires IExternalDbTransportStore. A store implementing only the former got past the guard and failed again inside the listener with a second, less specific message. Gate on the interface actually required. - Drop an unused local in the Oracle poller that re-materialized ExternalMessageTable.Columns() a second time. - Drop an unused `using Spectre.Console;` left behind when the external table members were extracted out of OracleMessageStore. - MySql publishes into a LONGBLOB column, so bind the byte[] payload directly instead of round-tripping it through Encoding.UTF8.GetString. Not a bug fix: a non-ASCII probe round-trips correctly either way on utf8mb4. This just removes a redundant encode/decode and the implicit dependence on the connection charset. - Note why the SQLite compliance fixture ignores the schemaName argument. Verified: pinned `wolverine.slnx -c Release -f net9.0` clean (0 warnings), and the external_message_tables suite green on all five providers — PostgreSQL, Sql Server, MySql, SQLite and Oracle (30 tests). Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
jeremydmiller
added a commit
that referenced
this pull request
Sep 23, 2026
…e DLQ replay coverage (#4577) Two findings from the #4558 review. ### The Oracle external table poller swallowed every failure OracleMessageStore.PollForMessagesFromExternalTablesAsync caught Exception and wrote ex.Message to Console.Error. ExternalMessageTableListener already wraps that call and logs the failure through its ILogger with the table name, then keeps polling -- so catching here pre-empted the only structured report there was. An Oracle poll failure produced a bare line on stderr with no stack trace and nothing whatsoever in the configured log sink. It also printed on every shutdown cancellation. MessageDatabase<T>'s implementation does not catch, and this was the only Console.Error.WriteLine under src/Persistence. The catch is gone; the try/finally that releases the advisory lock stays, since that part IS Oracle-specific (OracleAdvisoryLock holds its own connection rather than riding the poll connection). ReleaseLockAsync swallows and logs its own errors and is time-bounded, so the finally cannot mask the exception on its way out. Guarded by a new Oracle test that points a listener at a table that does not exist, with AllowWolverineControl off so Wolverine cannot create it, and asserts the failure reaches the logger. Reinstating the catch turns it red. ### DLQ replay coverage came back, on all five providers pull_in_message_that_goes_to_dead_letter_queue_and_replay_it was deleted from the Postgres tests in #4558 rather than migrated, leaving nothing covering an external-table-sourced message reaching the dead letter queue and being replayed out of it. That path is not generic DLQ behaviour: ExternalMessageTableListener returns no-op CompleteAsync/DeferAsync and forces ShouldPersistBeforeProcessing to false. It now lives in ExternalTableTransportCompliance, so it covers PostgreSQL, Sql Server, MySql, SQLite and Oracle instead of Postgres alone. Two changes from the original: the wait for the message to reach the DLQ is bounded rather than an unbounded `while (!ids.Any())` spin, so "it never got dead lettered" fails instead of hanging; and the handler's completion source uses TrySetResult so a redelivery cannot tear the run down from a background thread. Verified: pinned `wolverine.slnx -c Release -f net9.0` clean (0 warnings), the external_message_tables suite green on all five providers (7 tests each), and the full OracleTests.Transport namespace green (33 tests). Both new tests were negative-controlled -- each fails when its fix is reverted. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
jeremydmiller
added a commit
that referenced
this pull request
Sep 23, 2026
…uard reachable, fix the docs (#4578) The last four findings from the #4558 review. ### The compliance suite never checked that the source rows were deleted TestMessageSend asserted only the received envelope's Destination. A deleteManyAsync that quietly matched no rows would still commit, still call ReceivedAsync, and still go green -- while the row sat in the table and was redelivered on every subsequent poll, forever. That is the exact failure mode worth guarding on a transport whose whole job is to drain a table, and it was invisible. The poller deletes in the same transaction that stores to the inbox and only calls ReceivedAsync after that commits, so the assertion needs no polling: once the tracked session is done, the table must be empty. Counting rows needs one provider-specific detail, so qualifiedTableNameFor is virtual. SQLite overrides it because SQLite has no schemas -- Wolverine folds the schema name into a table-name prefix there (TablePrefixing, GH-3943). ### The guard for a non-relational message store was unreachable SendMessageThroughExternalTable null checked the result of JasperFx's As<T>(), which is documented as a hard cast and throws InvalidCastException. The branch could never run, so a RavenDb/Cosmos/multi-tenanted store got a bare InvalidCastException instead of the explanation that was written for it. Now an `is not` pattern, throwing InvalidOperationException to match the equivalent guards in ExternalMessageTable.BuildListenerAsync and ExternalMessageTableListener. ### The docs advertised an API that does not exist The sample told readers to call UseOraclePersistenceAndTransport. There is no such method -- Oracle has no combined helper, and opts into its database queue transport fluently off PersistMessagesWithOracle instead. ### external-tables.md was hand edited rather than generated The snippet footer still pointed at #L233-L295 for a region that had moved to #L44-L112, and hand-added blank lines sat around the code fence. Regenerated with mdsnippets. Only this one file is included: the tool rewrites 89 docs files repo-wide, and committing the other 88 here would collide with every concurrent branch for no benefit. Verified: pinned `wolverine.slnx -c Release -f net9.0` clean (0 warnings), and external_message_tables green on all five providers (6 tests each). Both new assertions were negative-controlled. Making the PostgreSQL delete match zero rows fails all three end-to-end tests with "should have been drained" while the three migration tests still pass -- which is precisely the hole. Restoring the As<T>() form makes the guard test fail with InvalidCastException, confirming that is what callers actually got. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 24, 2026
Merged
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.
Add Compliance Tests for Externally Controlled Database Tables
Create a set of compliance tests for External Db Table transport in order to unify the testing and make it easier to add more providers.
Implement Externally Table Transport for Oracle
Implemented the External Table transport configuration for Oracle.
Created test suite using the above compliance tests
Update documentation
Updated the documentation in external-tables.md