Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -246,4 +246,14 @@ integration, telemetry connectivity, and Azure Toolkit integration.
context of an API call.
Please refer to
the [QueryTimeInterval Class documentation](https://learn.microsoft.com/java/api/com.azure.monitor.query.models.querytimeinterval?view=azure-java-stable)
for additional information.
for additional information.

13. #### Calling 'stop' then 'start' APIs on a ServiceBusProcessor Client

- **Anti-pattern**: Calling `stop()` followed by `start()` on a `ServiceBusProcessorClient` instance.
- **Issue**: Calling `stop()` followed by `start()` on a `ServiceBusProcessorClient` involves significant complexity and
may
be deprecated in future versions of the SDK.
- **Severity: WARNING**
- **Recommendation**: Please close this processor instance and create a new one to restart processing. Refer to
the [GitHub issue](https://github.com/Azure/azure-sdk-for-java/issues/34464) for more details
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
package com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool;

import com.intellij.codeInspection.LocalInspectionTool;
import com.intellij.codeInspection.ProblemsHolder;
import com.intellij.psi.JavaElementVisitor;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.PsiMethodCallExpression;
import com.intellij.psi.PsiReferenceExpression;
import com.intellij.psi.PsiType;
import com.intellij.psi.PsiVariable;
import org.jetbrains.annotations.NotNull;

import java.util.HashMap;
import java.util.Map;

/**
* This class is a LocalInspectionTool that checks if a stop method is called on a ServiceBusProcessorClient object, followed by a start method call on the same object.
* If this is the case, a problem is registered with the ProblemsHolder.
*/
public class StopThenStartOnServiceBusProcessorCheck extends LocalInspectionTool {

/**
* This method builds a visitor that visits the PsiMethodCallExpression and checks if a stop method is called on a ServiceBusProcessorClient object, followed by a start method call on the same object.
* If this is the case, a problem is registered with the ProblemsHolder.
*
* @param holder The ProblemsHolder to register the problem with
* @param isOnTheFly A boolean that indicates if the inspection is being run on the fly - not used in this implementation but required by the method signature
* @return A JavaElementVisitor that visits the PsiMethodCallExpression and checks if a stop method is called on a ServiceBusProcessorClient object, followed by a start method call on the same object
*/
@NotNull
@Override
public JavaElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
return new StopThenStartOnServiceBusProcessorVisitor(holder);
}

/**
* This class is a JavaElementVisitor that visits the PsiMethodCallExpression and checks if a stop method is called on a ServiceBusProcessorClient object, followed by a start method call on the same object.
* If this is the case, a problem is registered with the ProblemsHolder.
*/
static class StopThenStartOnServiceBusProcessorVisitor extends JavaElementVisitor {

// Create a ProblemsHolder to register the problem with
private final ProblemsHolder holder;

// Create a map to store a boolean indicating if stop was called on the variable
private final Map<PsiVariable, Boolean> variableStateMap = new HashMap<>();

// Define constants for string literals
private static final RuleConfig RULE_CONFIG;
private static final boolean SKIP_WHOLE_RULE;

// Load the rule configuration
static {
final String ruleName = "StopThenStartOnServiceBusProcessorCheck";
RuleConfigLoader centralRuleConfigLoader = RuleConfigLoader.getInstance();

// Get the RuleConfig object for the rule
RULE_CONFIG = centralRuleConfigLoader.getRuleConfig(ruleName);
SKIP_WHOLE_RULE = RULE_CONFIG.skipRuleCheck() || RULE_CONFIG.getClientsToCheck().isEmpty();
}

/**
* This constructor initializes the Visitor with the ProblemsHolder
*
* @param holder The ProblemsHolder to register the problem with
*/
StopThenStartOnServiceBusProcessorVisitor(ProblemsHolder holder) {
this.holder = holder;
}

/**
* This method visits the PsiMethodCallExpression and checks if a stop method is called on a ServiceBusProcessorClient object, followed by a start method call on the same object.
* If this is the case, a problem is registered with the ProblemsHolder.
*
* @param expression The PsiMethodCallExpression to visit
*/
@Override
public void visitMethodCallExpression(@NotNull PsiMethodCallExpression expression) {
super.visitMethodCallExpression(expression);

if (SKIP_WHOLE_RULE) {
return;
}

// Check if the method being called is 'stop' or 'start'
PsiReferenceExpression methodExpression = expression.getMethodExpression();
String methodName = methodExpression.getReferenceName();

if (!(RULE_CONFIG.getMethodsToCheck().contains(methodName))) {
return;
}

// Get the qualifier of the method call - the object on which the method is called
PsiExpression qualifier = methodExpression.getQualifierExpression();

if (!(qualifier instanceof PsiReferenceExpression)) {
return;
}
PsiElement reference = ((PsiReferenceExpression) qualifier).resolve();

if (!(reference instanceof PsiVariable)) {
return;
}

// Get the variable that the method is called on
PsiVariable variable = (PsiVariable) reference;

// Check if the variable is a ServiceBusProcessorClient
if (!(isServiceBusProcessorClient(variable))) {
return;
}

// Boolean indicating if stop was called on the variable
Boolean wasStopCalled = variableStateMap.get(variable);

// If 'stop' is called, mark that 'stop' was called on the variable
if ("stop".equals(methodName)) {
variableStateMap.put(variable, true); // Mark that stop was called

// If 'close' is called, remove the variable from the map -- the resource is closed and the variable is no longer in use
} else if ("close".equals(methodName)) {
variableStateMap.remove(variable); // Remove the variable from the map

// If 'start' is called and 'stop' was called on the variable, register a problem
} else if ("start".equals(methodName) && Boolean.TRUE.equals(wasStopCalled)) {

Choose a reason for hiding this comment

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

how do we know if start was actually called before stop?

e.g. would is complain if I do

void stop() {
  client.stop();
}

void start() {
  client.start();
}

void run() {
 start();
 stop();
}

?

holder.registerProblem(expression, RULE_CONFIG.getAntiPatternMessageMap().get("antiPatternMessage"));

// Reset the state after reporting the problem
variableStateMap.remove(variable);
}
}

/**
* This method checks if the type of the variable is ServiceBusProcessorClient
*
* @param variable The variable to check
* @return A boolean indicating if the type of the variable is ServiceBusProcessorClient
*/
private static boolean isServiceBusProcessorClient(PsiVariable variable) {

PsiType type = variable.getType();
String typeText = type.getCanonicalText();
return typeText != null && typeText.contains(RULE_CONFIG.getClientsToCheck().get(0)) && typeText.startsWith(RuleConfig.AZURE_PACKAGE_NAME);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -207,5 +207,16 @@
implementationClass="com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool.KustoQueriesWithTimeIntervalInQueryStringCheck">
<class>com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool.KustoQueriesWithTimeIntervalInQueryStringCheck</class>
</localInspection>

<localInspection
language="JAVA"
shortName="StopThenStartOnServiceBusProcessorCheck"
displayName="Stop Then Start On ServiceBusProcessor Check"
groupName="Azure SDK Rules"
enabledByDefault="true"
level="WARNING"
implementationClass="com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool.StopThenStartOnServiceBusProcessorCheck">
<class>com.microsoft.azure.toolkit.intellij.azure.sdk.buildtool.StopThenStartOnServiceBusProcessorCheck</class>
</localInspection>
</extensions>
</idea-plugin>
Original file line number Diff line number Diff line change
Expand Up @@ -113,5 +113,14 @@
},
"antiPatternMessage": "KQL queries with time intervals in the query string detected.",
"solution": "Use the QueryTimeInterval parameter in the client method parameters to specify the time interval for the query"
},
"StopThenStartOnServiceBusProcessorCheck": {
"methodsToCheck": [
"stop",
"start",
"close"
],
"clientsToCheck": "ServiceBusProcessorClient",
"antiPatternMessage": "Starting Processor that was stopped before is not recommended, and this feature may be deprecated in the future. Please close this processor instance and create a new one to restart processing"
}
}
Loading