Skip to content
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.

Sign up for GitHub

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

API review feedback fixes for RuleManager Client #32174

Merged
merged 2 commits into from
Nov 16, 2022
Merged

API review feedback fixes for RuleManager Client #32174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d45ff64
API review feedback fixes for RuleManager Client
ki1729 Nov 15, 2022
767fb79
Checkstyle fixes
ki1729 Nov 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1990,7 +1990,7 @@ ServiceBusReceiverAsyncClient buildAsyncClient(boolean isAutoCompleteAllowed, bo
*
* @see ServiceBusRuleManagerAsyncClient
*/
@ServiceClientBuilder(serviceClients = {ServiceBusRuleManagerAsyncClient.class})
@ServiceClientBuilder(serviceClients = {ServiceBusRuleManagerAsyncClient.class, ServiceBusRuleManagerClient.class})
public final class ServiceBusRuleManagerBuilder {
private String subscriptionName;
private String topicName;
Expand Down
23 changes: 2 additions & 21 deletions ...icebus/src/main/java/com/azure/messaging/servicebus/ServiceBusRuleManagerAsyncClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import com.azure.messaging.servicebus.administration.ServiceBusAdministrationAsyncClient;
import com.azure.messaging.servicebus.administration.models.CorrelationRuleFilter;
import com.azure.messaging.servicebus.administration.models.CreateRuleOptions;
import com.azure.messaging.servicebus.administration.models.RuleFilter;
import com.azure.messaging.servicebus.administration.models.RuleProperties;
import com.azure.messaging.servicebus.administration.models.SqlRuleAction;
import com.azure.messaging.servicebus.administration.models.SqlRuleFilter;
Expand Down Expand Up @@ -143,24 +142,6 @@ public Mono<Void> createRule(String ruleName, CreateRuleOptions options) {
return createRuleInternal(ruleName, options);
}

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we shipped this yet? If so, this is a breaking change. Otherwise, it's fine. Same with the rename.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope this hasn't been shipped yet. Ww should be good here to make these changes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only refer to the .NET SDK, it has this overload, I added it as well.

* Creates a rule to the current subscription to filter the messages reaching from topic to the subscription.
*
* @param ruleName Name of rule.
* @param filter The filter expression against which messages will be matched.
* @return A Mono that completes when the rule is created.
*
* @throws NullPointerException if {@code filter}, {@code ruleName} is null.
* @throws IllegalStateException if client is disposed.
* @throws IllegalArgumentException if ruleName is empty string, {@code filter} is not instanceof {@link SqlRuleFilter} or
* {@link CorrelationRuleFilter}.
* @throws ServiceBusException if filter matches {@code ruleName} is already created in subscription.
*/
public Mono<Void> createRule(String ruleName, RuleFilter filter) {
CreateRuleOptions options = new CreateRuleOptions(filter);
return createRuleInternal(ruleName, options);
}

/**
* Fetches all rules associated with the topic and subscription.
*
Expand All @@ -169,7 +150,7 @@ public Mono<Void> createRule(String ruleName, RuleFilter filter) {
* @throws IllegalStateException if client is disposed.
* @throws UnsupportedOperationException if client cannot support filter with descriptor in message body.
*/
public Flux<RuleProperties> getRules() {
public Flux<RuleProperties> listRules() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I do this code, refer to .NET SDK, for consistent, I use getRules here is the .NET SDK :https://github.com/Azure/azure-sdk-for-net/blob/a1d2a65005df37e018d927ce3cf300e734b8923e/sdk/servicebus/Azure.Messaging.ServiceBus/src/RuleManager/ServiceBusRuleManager.cs#L226

if (isDisposed.get()) {
return fluxError(LOGGER, new IllegalStateException(
String.format(INVALID_OPERATION_DISPOSED_RULE_MANAGER, "getRules")
Expand All @@ -178,7 +159,7 @@ public Flux<RuleProperties> getRules() {

return connectionProcessor
.flatMap(connection -> connection.getManagementNode(entityPath, entityType))
.flatMapMany(ServiceBusManagementNode::getRules);
.flatMapMany(ServiceBusManagementNode::listRules);
}

/**
Expand Down
21 changes: 2 additions & 19 deletions ...-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusRuleManagerClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import com.azure.messaging.servicebus.administration.ServiceBusAdministrationAsyncClient;
import com.azure.messaging.servicebus.administration.models.CorrelationRuleFilter;
import com.azure.messaging.servicebus.administration.models.CreateRuleOptions;
import com.azure.messaging.servicebus.administration.models.RuleFilter;
import com.azure.messaging.servicebus.administration.models.RuleProperties;
import com.azure.messaging.servicebus.administration.models.SqlRuleAction;
import com.azure.messaging.servicebus.administration.models.SqlRuleFilter;
Expand Down Expand Up @@ -83,22 +82,6 @@ public void createRule(String ruleName, CreateRuleOptions options) {
asyncClient.createRule(ruleName, options).block(operationTimeout);
}

/**
* Creates a rule to the current subscription to filter the messages reaching from topic to the subscription.
*
* @param ruleName Name of rule.
* @param filter The filter expression against which messages will be matched.
*
* @throws NullPointerException if {@code filter}, {@code ruleName} is null.
* @throws IllegalStateException if client is disposed.
* @throws IllegalArgumentException if ruleName is empty string, {@code filter} is not instanceof {@link SqlRuleFilter} or
* {@link CorrelationRuleFilter}.
* @throws ServiceBusException if filter matches {@code ruleName} is already created in subscription.
*/
public void createRule(String ruleName, RuleFilter filter) {
asyncClient.createRule(ruleName, filter).block(operationTimeout);
}

/**
* Fetches all rules associated with the topic and subscription.
*
Expand All @@ -107,8 +90,8 @@ public void createRule(String ruleName, RuleFilter filter) {
* @throws IllegalStateException if client is disposed.
* @throws UnsupportedOperationException if client cannot support filter with descriptor in message body.
*/
public IterableStream<RuleProperties> getRules() {
return new IterableStream<>(asyncClient.getRules());
public IterableStream<RuleProperties> listRules() {
return new IterableStream<>(asyncClient.listRules());
}

/**
Expand Down
2 changes: 1 addition & 1 deletion ...icebus/src/main/java/com/azure/messaging/servicebus/implementation/ManagementChannel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ public Mono<Void> deleteRule(String ruleName) {
* {@inheritDoc}
*/
@Override
public Flux<RuleProperties> getRules() {
public Flux<RuleProperties> listRules() {
return isAuthorized(OPERATION_GET_RULES).then(createChannel.flatMap(channel -> {
final Message message = createManagementMessage(OPERATION_GET_RULES, null);

Expand Down
2 changes: 1 addition & 1 deletion ...src/main/java/com/azure/messaging/servicebus/implementation/ServiceBusManagementNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ Mono<Void> updateDisposition(String lockToken, DispositionStatus dispositionStat
*
* @return A collection of {@link RuleProperties rules}.
*/
Flux<RuleProperties> getRules();
Flux<RuleProperties> listRules();

@Override
void close();
Expand Down
2 changes: 1 addition & 1 deletion ...ng-servicebus/src/samples/java/com/azure/messaging/servicebus/ManageRulesAsyncSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void run() {
);

// Fetch all rules.
ruleManager.getRules().subscribe(ruleProperties -> System.out.println(ruleProperties.getName()));
ruleManager.listRules().subscribe(ruleProperties -> System.out.println(ruleProperties.getName()));

// Delete rule.
ruleManager.deleteRule("exist-rule").subscribe(
Expand Down
2 changes: 1 addition & 1 deletion ...es/java/com/azure/messaging/servicebus/ServiceBusRuleManagerAsyncClientJavaDocSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void createRule() {
@Test
public void getRules() {
// BEGIN: com.azure.messaging.servicebus.servicebusrulemanagerasyncclient.getRules
ruleManager.getRules().subscribe(ruleProperties -> System.out.println(ruleProperties.getName()));
ruleManager.listRules().subscribe(ruleProperties -> System.out.println(ruleProperties.getName()));
// END: com.azure.messaging.servicebus.servicebusrulemanagerasyncclient.getRules
}

Expand Down
21 changes: 2 additions & 19 deletions ...us/src/test/java/com/azure/messaging/servicebus/ServiceBusRuleManagerAsyncClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import com.azure.core.util.ClientOptions;
import com.azure.core.util.logging.ClientLogger;
import com.azure.messaging.servicebus.administration.models.CreateRuleOptions;
import com.azure.messaging.servicebus.administration.models.RuleFilter;
import com.azure.messaging.servicebus.administration.models.RuleProperties;
import com.azure.messaging.servicebus.administration.models.SqlRuleFilter;
import com.azure.messaging.servicebus.implementation.MessagingEntityType;
Expand All @@ -37,8 +36,6 @@

import java.time.Duration;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.when;

public class ServiceBusRuleManagerAsyncClientTest {
Expand Down Expand Up @@ -120,20 +117,6 @@ void teardown(TestInfo testInfo) throws Exception {
mocksCloseable.close();
}

/**
* Verifies that create a rule with a {@link RuleFilter}.
*/
@Test
void createRuleWithFilter() {
// Arrange
when(managementNode.createRule(eq(RULE_NAME), any(CreateRuleOptions.class))).thenReturn(Mono.empty());

// Act & Assert
StepVerifier.create(ruleManager.createRule(RULE_NAME, ruleFilter))
.expectComplete()
.verify();
}

/**
* Verifies that create a rule with a {@link CreateRuleOptions}.
*/
Expand All @@ -150,10 +133,10 @@ void createRuleWithOptions() {
@Test
void getRules() {
// Arrange
when(managementNode.getRules()).thenReturn(Flux.fromArray(new RuleProperties[]{ruleProperties1, ruleProperties2}));
when(managementNode.listRules()).thenReturn(Flux.fromArray(new RuleProperties[]{ruleProperties1, ruleProperties2}));

// Act & Assert
StepVerifier.create(ruleManager.getRules()).expectNext(ruleProperties1, ruleProperties2).verifyComplete();
StepVerifier.create(ruleManager.listRules()).expectNext(ruleProperties1, ruleProperties2).verifyComplete();
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion ...s/src/test/java/com/azure/messaging/servicebus/implementation/ManagementChannelTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ void getRules() {
responseMessage.setBody(message.getBody());

// Assert response message content.
StepVerifier.create(managementChannel.getRules())
StepVerifier.create(managementChannel.listRules())
.assertNext(ruleProperties -> {
assertEquals("$Default", ruleProperties.getName());
assertEquals(ruleProperties.getFilter(), new TrueRuleFilter());
Expand Down