Skip to content

Commit

Permalink
Introduce a BugCheck for the useage of Mongo full text searches via t…
Browse files Browse the repository at this point in the history
…he Java driver
  • Loading branch information
philleonard authored and rickie committed Jun 7, 2023
1 parent c141ebe commit 3b742fb
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 6 deletions.
5 changes: 5 additions & 0 deletions error-prone-contrib/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,11 @@
<artifactId>mockito-core</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.mongodb</groupId>
<artifactId>mongodb-driver-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.reactivestreams</groupId>
<artifactId>reactive-streams</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package tech.picnic.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.PERFORMANCE;
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;

import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;

/**
* A {@link BugChecker} that flags usages of Mongo $text filters used for full text searches.
*
* @see <a href="https://www.mongodb.com/docs/manual/text-search/">Mongo Text Search</a>
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Usage of the Mongo FTS ($text) queries is discouraged",
link = BUG_PATTERNS_BASE_URL + "MongoFullTextSearchQueryUsage",
linkType = CUSTOM,
severity = WARNING,
tags = PERFORMANCE)
public final class MongoFullTextSearchQueryUsage extends BugChecker
implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> TEXT_FILTER_INVOCATION =
staticMethod().onClass("com.mongodb.client.model.Filters").named("text");

/** Instantiates a new {@link MongoFullTextSearchQueryUsage} instance. */
public MongoFullTextSearchQueryUsage() {}

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!ThirdPartyLibrary.MONGO.isIntroductionAllowed(state)
|| !TEXT_FILTER_INVOCATION.matches(tree, state)) {
return Description.NO_MATCH;
}
return describeMatch(tree);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ public enum ThirdPartyLibrary {
* @see <a href="https://github.com/google/guava">Guava on GitHub</a>
*/
GUAVA("com.google.common.collect.ImmutableList"),

/**
* Mongo's Java driver Filters API.
*
* @see <a
* href="https://github.com/mongodb/mongo-java-driver/blob/master/driver-core/src/main/com/mongodb/client/model/Filters.java">MongoDB
* Filters API</a>
* @see <a href="https://mongodb.github.io/mongo-java-driver/">MongoDB Java drvier</a>
*/
MONGO("com.mongodb.client.model.Filters"),

/**
* New Relic's Java agent API.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package tech.picnic.errorprone.bugpatterns;

import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;

final class MongoFullTextSearchQueryUsageTest {
@Test
void identification() {
CompilationTestHelper.newInstance(MongoFullTextSearchQueryUsage.class, getClass())
.addSourceLines(
"A.java",
"import com.mongodb.client.model.Filters;",
"import com.mongodb.client.model.TextSearchOptions;",
"import org.bson.conversions.Bson;",
"",
"class A {",
"",
" void m() {",
" Bson allowed = Filters.eq(\"a\", \"b\");",
" // BUG: Diagnostic contains:",
" Bson textSearch = Filters.text(\"Some text\");",
" // BUG: Diagnostic contains:",
" Bson textSearch2 = Filters.text(\"Some text\", new TextSearchOptions().caseSensitive(true));",
" }",
"}")
.doTest();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ void isIntroductionAllowed() {
CompilationTestHelper.newInstance(TestChecker.class, getClass())
.addSourceLines(
"A.java",
"// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, NEW_RELIC_AGENT_API: true, REACTOR: true",
"// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, MONGO: true, NEW_RELIC_AGENT_API: true,",
"// REACTOR: true",
"class A {}")
.doTest();
}
Expand All @@ -34,15 +35,18 @@ void isIntroductionAllowedWitnessClassesInSymtab() {
.addSourceLines(
"A.java",
"import com.google.common.collect.ImmutableList;",
"import com.mongodb.client.model.Filters;",
"import com.newrelic.api.agent.Agent;",
"import org.assertj.core.api.Assertions;",
"import reactor.core.publisher.Flux;",
"",
"// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, NEW_RELIC_AGENT_API: true, REACTOR: true",
"// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, MONGO: true, NEW_RELIC_AGENT_API: true,",
"// REACTOR: true",
"class A {",
" void m(Class<?> clazz) {",
" m(Assertions.class);",
" m(ImmutableList.class);",
" m(Filters.class);",
" m(Agent.class);",
" m(Flux.class);",
" }",
Expand All @@ -56,7 +60,8 @@ void isIntroductionAllowedWitnessClassesPartiallyOnClassPath() {
.withClasspath(ImmutableList.class, Flux.class)
.addSourceLines(
"A.java",
"// BUG: Diagnostic contains: ASSERTJ: false, GUAVA: true, NEW_RELIC_AGENT_API: false, REACTOR: true",
"// BUG: Diagnostic contains: ASSERTJ: false, GUAVA: true, MONGO: false, NEW_RELIC_AGENT_API: false,",
"// REACTOR: true",
"class A {}")
.doTest();
}
Expand All @@ -67,7 +72,8 @@ void isIntroductionAllowedWitnessClassesNotOnClassPath() {
.withClasspath()
.addSourceLines(
"A.java",
"// BUG: Diagnostic contains: ASSERTJ: false, GUAVA: false, NEW_RELIC_AGENT_API: false, REACTOR:",
"// BUG: Diagnostic contains: ASSERTJ: false, GUAVA: false, MONGO: false, NEW_RELIC_AGENT_API: false,",
"// REACTOR:",
"// false",
"class A {}")
.doTest();
Expand All @@ -82,8 +88,8 @@ void isIntroductionAllowedIgnoreClasspathCompat(boolean ignoreClassPath) {
.addSourceLines(
"A.java",
String.format(
"// BUG: Diagnostic contains: ASSERTJ: %s, GUAVA: true, NEW_RELIC_AGENT_API: %s, REACTOR: true",
ignoreClassPath, ignoreClassPath),
"// BUG: Diagnostic contains: ASSERTJ: %s, GUAVA: true, MONGO: %s, NEW_RELIC_AGENT_API: %s, REACTOR: true",
ignoreClassPath, ignoreClassPath, ignoreClassPath),
"class A {}")
.doTest();
}
Expand Down
12 changes: 12 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,11 @@
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<groupId>org.mongodb</groupId>
<artifactId>mongodb-driver-core</artifactId>
<version>4.9.1</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
Expand Down Expand Up @@ -1019,6 +1024,13 @@
<dependencyConvergence />
<enforceBytecodeVersion>
<maxJdkVersion>${version.jdk}</maxJdkVersion>
<!-- XXX: Drop this when we forgo enforcement of
JDK 11 bytecode version compatibility. See
https://github.com/PicnicSupermarket/error-prone-support/pull/603
https://github.com/PicnicSupermarket/error-prone-support/pull/198. -->
<ignoreClasses>
<ignoreClass>org.bson.codecs.record.*</ignoreClass>
</ignoreClasses>
</enforceBytecodeVersion>
<requireEncoding>
<acceptAsciiSubset>true</acceptAsciiSubset>
Expand Down

0 comments on commit 3b742fb

Please sign in to comment.