Add ServiceBus Session Receiver Client #16690

YijunXieMS · 2020-10-22T17:01:21Z

Add ServiceBusSessionReceiverAsyncClient and ServiceBusSessionReceiverClient to accept a specific session or a next available session.

Closes #15330

...cebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverAsyncClient.java

...saging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionManager.java

...g-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceiverAsyncClient.java

...cebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverAsyncClient.java

conniey · 2020-10-23T20:52:36Z

...cebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverAsyncClient.java

+
+    @Override
+    public void close() {
+        this.onClientClose.run();


Is this idempotent? If we call close multiple times, what happens.

This calls the ServiceBusClientBuilder's close() method.

...servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverClient.java

...cebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverAsyncClient.java

...ng-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusSessionManagerTest.java

...g-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceiverAsyncClient.java

...cebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverAsyncClient.java

...servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverClient.java

...cebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverAsyncClient.java

check-enforcer · 2020-10-28T03:52:31Z

This pull request is protected by Check Enforcer.

What is Check Enforcer?

Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass.

Why am I getting this message?

You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged.

What should I do now?

If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows:
/check-enforcer evaluate
Typically evaulation only takes a few seconds. If you know that your pull request is not covered by a pipeline and this is expected you can override Check Enforcer using the following command:
/check-enforcer override
Note that using the override command triggers alerts so that follow-up investigations can occur (PRs still need to be approved as normal).

What if I am onboarding a new service?

Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment:
/azp run prepare-pipelines
This will run a pipeline that analyzes the source tree and creates the pipelines necessary to build and validate your pull request. Once the pipeline has been created you can trigger the pipeline using the following comment:
/azp run java - [service] - ci

...g-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceiverAsyncClient.java

...saging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceiverClient.java

...cebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverAsyncClient.java

...g-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceiverAsyncClient.java

...cebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverAsyncClient.java

...servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverClient.java

srnagar · 2020-10-28T08:58:19Z

...servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverClient.java

+     * @return A {@link ServiceBusReceiverClient} that is tied to the available session.
+     * @throws IllegalStateException if the operation times out.
+     */
+    public ServiceBusReceiverClient acceptNextSession(Duration timeout) {


What happens if timeout is Duration.ZERO? Also, if the timeout is larger than the retry timeout what happens? I don't think any code change is required but we just need to document both these cases.

...cebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverAsyncClient.java

…Session

...ssaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusClientBuilder.java

srnagar · 2020-10-29T19:38:19Z

...ssaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusClientBuilder.java

-                maxAutoLockRenewDuration, enableAutoComplete, sessionId, isRollingSessionReceiver(),
-                maxConcurrentSessions);
-
-            final ServiceBusSessionManager sessionManager = new ServiceBusSessionManager(entityPath, entityType,
-                connectionProcessor, tracerProvider, messageSerializer, receiverOptions);
-
-            return new ServiceBusReceiverAsyncClient(connectionProcessor.getFullyQualifiedNamespace(), entityPath,
-                entityType, receiverOptions, connectionProcessor, ServiceBusConstants.OPERATION_TIMEOUT,
-                tracerProvider, messageSerializer, ServiceBusClientBuilder.this::onClientClose, sessionManager);
-        }
-
-        /**
-         * This is a rolling session receiver only if maxConcurrentSessions is > 0 AND sessionId is null or empty. If
-         * there is a sessionId, this is going to be a single, named session receiver.
-         *
-         * @return {@code true} if this is an unnamed rolling session receiver; {@code false} otherwise.
-         */
-        private boolean isRollingSessionReceiver() {
-            if (maxConcurrentSessions == null) {
-                return false;
-            }
-
-            if (maxConcurrentSessions < 1) {
-                throw logger.logExceptionAsError(
-                    new IllegalArgumentException("Maximum number of concurrent sessions must be positive."));
-            }
+                maxAutoLockRenewDuration, enableAutoComplete, null, null);

-            return CoreUtils.isNullOrEmpty(sessionId);


All of this will be required for the session processor. Keep this code and make this available only for the processor.

I added maxConcurrentSessions back at package level but keep isRollingSessionReceiver deleted from the builder because ReceiverOptions.isRollingSessionReceiver() can be inferred from it's own properties.

...saging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceiverClient.java

… package level

srnagar · 2020-10-30T06:32:40Z

...ssaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusClientBuilder.java

+        ServiceBusReceiverClient buildClientForProcessor() {
+            return new ServiceBusReceiverClient(buildAsyncClientForProcessor(false), retryOptions.getTryTimeout());
        }


Processor will never build a sync client. So, this method can be removed.

srnagar · 2020-10-30T06:33:31Z

...ssaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusClientBuilder.java

        }

-        private ServiceBusReceiverAsyncClient buildAsyncClient(boolean isAutoCompleteAllowed) {
+        private ServiceBusReceiverAsyncClient buildAsyncClientForProcessor(boolean isAutoCompleteAllowed) {


Since the async client built for processor will always have autoComplete enabled, the boolean param is redundant and can be removed.

srnagar · 2020-10-30T08:52:15Z

...ssaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusClientBuilder.java

-                return false;
+        public ServiceBusSessionReceiverClient buildClient() {
+            return new ServiceBusSessionReceiverClient(buildAsyncClient(false),
+                retryOptions.getTryTimeout());


This timeout is for a single attempt, right? If the user has set retry policy to 3 attempts, will this not wait for the 2nd attempt after the first attempt fails due to timeout?

Add ServiceBus Session Receiver Client

aee1663

YijunXieMS added Service Bus Client This issue points to a problem in the data-plane of the library. labels Oct 22, 2020

YijunXieMS added this to the [2020] November milestone Oct 22, 2020

YijunXieMS self-assigned this Oct 22, 2020

YijunXieMS added 2 commits October 22, 2020 11:04

Merge branch 'master' into sb_session_receiver

094f9ed

More small fixes after merging from master

b4aeb73

YijunXieMS requested review from conniey, hemanttanwar and srnagar October 22, 2020 18:07

YijunXieMS marked this pull request as ready for review October 22, 2020 18:07

YijunXieMS requested a review from yvgopal as a code owner October 22, 2020 18:07

YijunXieMS added 2 commits October 22, 2020 16:56

More small fixes after merging from master

b7d7ab1

create an unnamed session manager for getReceiverClient

d9af703

hemanttanwar reviewed Oct 23, 2020

View reviewed changes

conniey reviewed Oct 23, 2020

View reviewed changes

srnagar reviewed Oct 23, 2020

View reviewed changes

YijunXieMS added 2 commits October 27, 2020 16:35

Update code review comment

f59e84f

Add test case for ServiceBusSessionReceiverAsyncClient and -Client

8b063c3

YijunXieMS added 2 commits October 27, 2020 22:08

Merge branch 'master' into sb_session_receiver

a9a7a0e

Fix merge problems and tests

2c2c7ae

srnagar reviewed Oct 28, 2020

View reviewed changes

YijunXieMS added 7 commits October 28, 2020 13:58

Use ServiceBusClientBuilder's retryPolicy timeout

464d533

Remove/hide receiver methods that have param sessionId

27580ca

Small fixes

30266e1

Remove INVALID_LOCK_TOKEN_STRING

4eb8093

Fix unnamesession test

60d0b52

cannot acquire lock wording adjustment

600f60f

Add maxLockRenewal to fix test

22308dd

YijunXieMS added 3 commits October 29, 2020 00:23

Add @ServiceClient annotation

925d7b8

Adjust readme sample code lines

b08e95d

Supress @ServiceClient check for methods acceptNextSession and accept…

c86c5fc

…Session

YijunXieMS requested review from JonathanGiles and mssfang as code owners October 29, 2020 18:36

srnagar reviewed Oct 29, 2020

View reviewed changes

YijunXieMS added 6 commits October 29, 2020 15:14

Add back maxConcurrentSessions to ServiceBusSessionReceiverBuilder at…

ef3d9d5

… package level

Remove </p> from javadoc

c13f280

Restore some session related code but hide with package level

e717790

Update sync ServiceBusReceiverClientTest

110fcb7

Try fix javadoc ci problem

d514793

Try fix javadoc ci problem

36c8c95

srnagar approved these changes Oct 30, 2020

View reviewed changes

YijunXieMS merged commit 3b5d04c into Azure:master Oct 30, 2020

Add ServiceBus Session Receiver Client #16690

Add ServiceBus Session Receiver Client #16690

Uh oh!

Conversation

YijunXieMS commented Oct 22, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

check-enforcer bot commented Oct 28, 2020

What is Check Enforcer?

Why am I getting this message?

What should I do now?

What if I am onboarding a new service?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants