-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP | Draft]: Drop the support for AMQP v1 stack #43735
base: main
Are you sure you want to change the base?
Conversation
API change check API changes are not detected in this pull request. |
/azp run java - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
* @throws InterruptedException If the test is interrupted. | ||
*/ | ||
@Test | ||
public void testReceivingMultiSessionMessagesWithProcessor() throws InterruptedException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ServiceBusSessionReceiverAsyncClient::ServiceBusReceiverAsyncClient
is no longer responsible for internally pumping from "multiple" sessions, the v2 rework move this responsibility to SessionsMessagePump
to address the out of order message delivery and the limited concurrency v1 had. The same case is now covered by
SessionsMessagePumpIsolatedTest::shouldPumpFromMultiSessionWhenBackingProcessor
@@ -442,39 +367,7 @@ public void testProcessorWithTracingEnabled(boolean isV2) throws InterruptedExce | |||
|
|||
@Test | |||
@SuppressWarnings("unchecked") | |||
public void testProcessorWithTracingEnabledAndNullMessage() throws InterruptedException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is not required anymore since v2 no longer uses ServiceBusReceivedMessageContext
to funnel error. Which means the instrumentation in v2 processor will no longer get error-ServiceBusReceivedMessageContext
with no message. Originally, this test was added after a regression where instrumentation tried to access null message in such ServiceBusReceivedMessageContext
leading to NPE
* Integration tests for {@link ServiceBusSessionManager}. | ||
*/ | ||
@Tag("integration") | ||
class ServiceBusSessionManagerIntegrationTest extends IntegrationTestBase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ServiceBusReceiverAsyncClient
is no longer act as the internal SessionManager pumping from "multiple" sessions, v2 moved that responsibility to the new SessionsMessagePump
type to address the out of order message delivery and scaling. Valid cases in this test class are now covered by ServiceBusProcessorClientIntegrationTest
and ServiceBusReceiverAsyncClientIntegrationTest
fae62c5
to
f911d46
Compare
/azp run java - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
f911d46
to
bee48de
Compare
/azp run java - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run java - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run java - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run java - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run java - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
* Verify that when we receive multiple sessions, it'll change to the next session when one is complete. | ||
*/ | ||
@Test | ||
void multipleSessions() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The v1 ServiceBusSessionManager
is removed. The tests multipleSessions
and multipleReceiveUnnamedSession
are not valid for ServiceBusSingleSessionManager
as it streams from one session only. By removing ServiceBusSessionManager
, the session enabled ServiceBusReceiverAsyncClient
is no longer responsible for internally backing processor, but SessionsMessagePump
(added in v2).
messageSerializer, receiverOptions, CLIENT_IDENTIFIER, NOOP_TRACER); | ||
|
||
// Act & Assert | ||
StepVerifier.create(sessionManager.receive()).expectError(NullPointerException.class).verify(DEFAULT_TIMEOUT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In v2 ServiceBusSingleSessionManager
, the receive()
is guaranteed to return a Flux never null.
messageSerializer, receiverOptions, CLIENT_IDENTIFIER, NOOP_TRACER); | ||
|
||
// Act & Assert | ||
assertEquals(CLIENT_IDENTIFIER, sessionManager.getIdentifier()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved this assertion to other tests, a dedicated test is not needed.
/azp run java - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run java - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
… update the ServiceBusProcessorClient to remove the v1 processor code
…file, make the assert message for the test shouldPumpFromMultiSessionWhenBackingProcessor better
…rviceBusSessionManager) (part 3)
…V2() api, ServiceBusReceiveLinkProcessor type) (part 4)
…d and before peeking
257b00a
to
9f37782
Compare
No description provided.