Update SpotBugs and fix new issues (#420)

* Bump com.github.spotbugs:spotbugs-maven-plugin from 4.7.3.6 to 4.8.1.0

Bumps [com.github.spotbugs:spotbugs-maven-plugin](https://github.com/spotbugs/spotbugs-maven-plugin) from 4.7.3.6 to 4.8.1.0.
- [Release notes](https://github.com/spotbugs/spotbugs-maven-plugin/releases)
- [Commits](spotbugs/spotbugs-maven-plugin@spotbugs-maven-plugin-4.7.3.6...spotbugs-maven-plugin-4.8.1.0)

---
updated-dependencies:
- dependency-name: com.github.spotbugs:spotbugs-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Move PatchOperationPath parsing into util method.

Construction of object now happens in util method as to not be vunlerable to finalizer attacks.
This is probably a nit, but SpotBugs/FindSecBugs flagged this issue in the latest update.

There are a few other minor related issues flagged by the spotbugs upgrade as well
- missing static keywords
- fields that could be private

* Explicitly configure lombok annotation processor

This will prevent build cache misses

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

pom.xml

@@ -438,6 +438,13 @@
             <!-- parent pom currently defines v1.7 -->
             <source>${maven.compiler.source}</source>
             <target>${maven.compiler.target}</target>
+            <annotationProcessorPaths>
+              <path>
+                <groupId>org.projectlombok</groupId>
+                <artifactId>lombok</artifactId>
+                <version>${version.lombok}</version>
+              </path>
+            </annotationProcessorPaths>
           </configuration>
         </plugin>
         <plugin>
@@ -627,7 +634,7 @@
         <plugin>
           <groupId>com.github.spotbugs</groupId>
           <artifactId>spotbugs-maven-plugin</artifactId>
-          <version>4.7.3.6</version>
+          <version>4.8.1.0</version>
           <configuration>
             <plugins>
               <plugin>

scim-client/src/test/java/org/apache/directory/scim/client/rest/ScimUserClientTest.java

@@ -207,7 +207,7 @@ public void patch(MockWebServer server, ScimUserClient client) throws Exception
     PatchRequest patchRequest = new PatchRequest().add(
       new PatchOperation()
         .setOperation(PatchOperation.Type.REMOVE)
-        .setPath(new PatchOperationPath("name.honorificPrefix"))
+        .setPath(PatchOperationPath.fromString("name.honorificPrefix"))
     );
 
     ScimUser expectedUser = new ScimUser();

scim-core/src/main/java/org/apache/directory/scim/core/repository/PatchHandlerImpl.java

@@ -134,7 +134,7 @@ private <T extends ScimResource> void apply(T source, Map<String, Object> source
 
   private PatchOperationPath tryGetOperationPath(String key) {
     try {
-      return new PatchOperationPath(key);
+      return PatchOperationPath.fromString(key);
     } catch (FilterParseException e) {
       log.warn("Parsing path failed with exception.", e);
       throw new UnsupportedFilterException("Cannot parse path expression: " + e.getMessage());

scim-core/src/main/java/org/apache/directory/scim/core/repository/RepositoryRegistry.java

@@ -46,17 +46,16 @@ public RepositoryRegistry(SchemaRegistry schemaRegistry) {
     this.schemaRegistry = schemaRegistry;
   }
 
-  public RepositoryRegistry(SchemaRegistry schemaRegistry, List<Repository<? extends ScimResource>> scimRepositories) {
-    this.schemaRegistry = schemaRegistry;
+  public void registerRepositories(List<Repository<? extends ScimResource>> scimRepositories) {
     scimRepositories.stream()
-      .map(repository -> (Repository<ScimResource>) repository)
+      .map(repository -> (Repository<? extends ScimResource>) repository)
       .forEach(repository -> {
-      try {
-        registerRepository(repository.getResourceClass(), repository);
-      } catch (InvalidRepositoryException e) {
-        throw new ScimResourceInvalidException("Failed to register repository " + repository.getClass() + " for ScimResource type " + repository.getResourceClass(), e);
-      }
-    });
+        try {
+          registerRepository(repository.getResourceClass(), repository);
+        } catch (InvalidRepositoryException e) {
+          throw new ScimResourceInvalidException("Failed to register repository " + repository.getClass() + " for ScimResource type " + repository.getResourceClass(), e);
+        }
+      });
   }
 
   public synchronized <T extends ScimResource> void registerRepository(Class<T> clazz, Repository<T> repository) throws InvalidRepositoryException {

scim-core/src/main/java/org/apache/directory/scim/core/repository/UpdateRequest.java

@@ -313,9 +313,7 @@ private PatchOperation handleExtensions(JsonNode valueNode, Type patchOpType, Pa
     operation.setOperation(patchOpType);
 
     AttributeReference attributeReference = new AttributeReference(parseData.pathUri, null);
-    PatchOperationPath patchOperationPath = new PatchOperationPath();
-    ValuePathExpression valuePathExpression = new ValuePathExpression(attributeReference);
-    patchOperationPath.setValuePathExpression(valuePathExpression);
+    PatchOperationPath patchOperationPath = new PatchOperationPath(new ValuePathExpression(attributeReference));
 
     operation.setPath(patchOperationPath);
     operation.setValue(determineValue(patchOpType, valueNode, parseData));
@@ -447,9 +445,7 @@ private PatchOperation buildPatchOperation(PatchOperation.Type patchOpType, Pars
       subAttribute = subAttributes.get(0);
     }
     AttributeReference attributeReference = new AttributeReference(parseData.pathUri, attribute, subAttribute);
-    PatchOperationPath patchOperationPath = new PatchOperationPath();
-    ValuePathExpression valuePathExpression = new ValuePathExpression(attributeReference, valueFilterExpression);
-    patchOperationPath.setValuePathExpression(valuePathExpression);
+    PatchOperationPath patchOperationPath = new PatchOperationPath(new ValuePathExpression(attributeReference, valueFilterExpression));
 
     operation.setPath(patchOperationPath);
     operation.setValue(value);

scim-core/src/main/java/org/apache/directory/scim/core/spi/ScimpleComponents.java

@@ -44,7 +44,9 @@ public SchemaRegistry schemaRegistry() {
   @Produces
   @ApplicationScoped
   public RepositoryRegistry repositoryRegistry(SchemaRegistry schemaRegistry, Instance<Repository<? extends ScimResource>> repositoryInstances) {
-    return new RepositoryRegistry(schemaRegistry, repositoryInstances.stream().collect(Collectors.toList()));
+    RepositoryRegistry registry = new RepositoryRegistry(schemaRegistry);
+    registry.registerRepositories(repositoryInstances.stream().collect(Collectors.toList()));
+    return registry;
   }
 
   /*

scim-core/src/test/java/org/apache/directory/scim/core/repository/PatchHandlerTest.java

@@ -322,7 +322,7 @@ public void applyRemove() {
   public void applyWithFilterExpression() throws FilterParseException {
     PatchOperation op = new PatchOperation();
     op.setOperation(REPLACE);
-    op.setPath(new PatchOperationPath("emails[type EQ \"home\"].value"));
+    op.setPath(PatchOperationPath.fromString("emails[type EQ \"home\"].value"));
     op.setValue("[email protected]");
     ScimUser updatedUser = patchHandler.apply(user(), List.of(op));
     List<Email> emails = updatedUser.getEmails();
@@ -341,7 +341,7 @@ public void applyWithFilterExpression() throws FilterParseException {
   public void replaceItem() throws FilterParseException {
     PatchOperation op = new PatchOperation();
     op.setOperation(REPLACE);
-    op.setPath(new PatchOperationPath("emails[type EQ \"home\"]"));
+    op.setPath(PatchOperationPath.fromString("emails[type EQ \"home\"]"));
     op.setValue(Map.of(
       "type", "other",
       "value", "[email protected]"
@@ -395,7 +395,7 @@ public void replaceMultipleAttributes() {
   public void replaceCollection() throws FilterParseException {
     PatchOperation op = new PatchOperation();
     op.setOperation(REPLACE);
-    op.setPath(new PatchOperationPath("emails"));
+    op.setPath(PatchOperationPath.fromString("emails"));
     op.setValue(List.of(
       Map.of(
         "value", "[email protected]",
@@ -422,7 +422,7 @@ public void replaceCollection() throws FilterParseException {
   public void deleteItemWithFilter() throws FilterParseException {
     PatchOperation op = new PatchOperation();
     op.setOperation(REMOVE);
-    op.setPath(new PatchOperationPath("emails[type EQ \"home\"]"));
+    op.setPath(PatchOperationPath.fromString("emails[type EQ \"home\"]"));
     ScimUser updatedUser = patchHandler.apply(user(), List.of(op));
     List<Email> emails = updatedUser.getEmails();
     assertThat(emails).isEqualTo(List.of(
@@ -437,7 +437,7 @@ public void deleteItemWithFilter() throws FilterParseException {
   public void deleteAttributeWithPath() throws FilterParseException {
     PatchOperation op = new PatchOperation();
     op.setOperation(REMOVE);
-    op.setPath(new PatchOperationPath("nickName"));
+    op.setPath(PatchOperationPath.fromString("nickName"));
     ScimUser updatedUser = patchHandler.apply(user(), List.of(op));
     assertThat(updatedUser.getNickName()).isNull();
   }
@@ -446,7 +446,7 @@ public void deleteAttributeWithPath() throws FilterParseException {
   public void deleteCollectionWithPath() throws FilterParseException {
     PatchOperation op = new PatchOperation();
     op.setOperation(REMOVE);
-    op.setPath(new PatchOperationPath("emails"));
+    op.setPath(PatchOperationPath.fromString("emails"));
     ScimUser updatedUser = patchHandler.apply(user(), List.of(op));
     assertThat(updatedUser.getEmails()).isNull();
   }
@@ -455,7 +455,7 @@ public void deleteCollectionWithPath() throws FilterParseException {
   public void deleteItemWithComplexFilter() throws FilterParseException {
     PatchOperation op = new PatchOperation();
     op.setOperation(REMOVE);
-    op.setPath(new PatchOperationPath("emails[type EQ \"home\"] and value ew \"example.com\""));
+    op.setPath(PatchOperationPath.fromString("emails[type EQ \"home\"] and value ew \"example.com\""));
     ScimUser updatedUser = patchHandler.apply(user(), List.of(op));
     assertThat(updatedUser.getEmails()).isEqualTo(List.of(
       new Email()
@@ -469,7 +469,7 @@ public void deleteItemWithComplexFilter() throws FilterParseException {
   public void addAttribute() throws FilterParseException {
     PatchOperation op = new PatchOperation();
     op.setOperation(ADD);
-    op.setPath(new PatchOperationPath("profileUrl"));
+    op.setPath(PatchOperationPath.fromString("profileUrl"));
     op.setValue("https://profile.example.com");
 
     ScimUser expectedUser = user()
@@ -483,7 +483,7 @@ public void addAttribute() throws FilterParseException {
   public void addItem() throws FilterParseException {
     PatchOperation op = new PatchOperation();
     op.setOperation(ADD);
-    op.setPath(new PatchOperationPath("emails"));
+    op.setPath(PatchOperationPath.fromString("emails"));
     op.setValue(Map.of(
       "type", "other",
       "value", "[email protected]"));
@@ -537,11 +537,11 @@ public void addMultipleProperties() throws FilterParseException {
   public void multiplePatchOperations() throws FilterParseException {
     PatchOperation opRm = new PatchOperation();
     opRm.setOperation(REMOVE);
-    opRm.setPath(new PatchOperationPath("emails[type EQ \"home\"]"));
+    opRm.setPath(PatchOperationPath.fromString("emails[type EQ \"home\"]"));
 
     PatchOperation opAdd = new PatchOperation();
     opAdd.setOperation(ADD);
-    opAdd.setPath(new PatchOperationPath("emails"));
+    opAdd.setPath(PatchOperationPath.fromString("emails"));
     opAdd.setValue(Map.of(
       "value", "[email protected]",
       "type", "other")
@@ -564,11 +564,11 @@ public void multiplePatchOperations() throws FilterParseException {
   public void replaceCollectionWithMultipleOps() throws FilterParseException {
     PatchOperation opRm = new PatchOperation();
     opRm.setOperation(REMOVE);
-    opRm.setPath(new PatchOperationPath("emails"));
+    opRm.setPath(PatchOperationPath.fromString("emails"));
 
     PatchOperation opAdd = new PatchOperation();
     opAdd.setOperation(ADD);
-    opAdd.setPath(new PatchOperationPath("emails"));
+    opAdd.setPath(PatchOperationPath.fromString("emails"));
     opAdd.setValue(List.of(
       Map.of(
         "value", "[email protected]",
@@ -597,7 +597,7 @@ private PatchOperation patchOperation(Type operationType, String path, Object va
     PatchOperation op = new PatchOperation();
     op.setOperation(operationType);
     if (path != null) {
-      op.setPath(new PatchOperationPath(path));
+      op.setPath(PatchOperationPath.fromString(path));
     }
     if (value != null) {
       op.setValue(value);

scim-core/src/test/java/org/apache/directory/scim/core/repository/RepositoryRegistryTest.java

@@ -44,7 +44,7 @@ public void initializeWithException() throws InvalidRepositoryException {
 
     doThrow(new InvalidRepositoryException("test exception")).when(repository).getExtensionList();
 
-    assertThrows(ScimResourceInvalidException.class, () -> new RepositoryRegistry(schemaRegistry, List.of(repository)));
+    assertThrows(ScimResourceInvalidException.class, () -> new RepositoryRegistry(schemaRegistry).registerRepositories(List.of(repository)));
   }
 
   @Test

scim-core/src/test/java/org/apache/directory/scim/core/repository/UpdateRequestTest.java

@@ -917,7 +917,7 @@ private List<PatchOperation> createUser1PatchOps() throws FilterParseException {
     List<PatchOperation> patchOperations = new ArrayList<>();
     PatchOperation removePhoneNumberOp = new PatchOperation();
     removePhoneNumberOp.setOperation(Type.REMOVE);
-    removePhoneNumberOp.setPath(new PatchOperationPath("phoneNumbers[type eq \"home\"]"));
+    removePhoneNumberOp.setPath(PatchOperationPath.fromString("phoneNumbers[type eq \"home\"]"));
     patchOperations.add(removePhoneNumberOp);
     return patchOperations;
   }

scim-spec/scim-spec-protocol/src/main/java/org/apache/directory/scim/protocol/adapter/FilterWrapper.java

@@ -33,9 +33,9 @@
 
 @Slf4j
 @Data
-public class FilterWrapper {
+final public class FilterWrapper {
 
-  public Filter filter;
+  final public Filter filter;
 
   public FilterWrapper(String string) {
 

scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/adapter/PatchOperationPathAdapter.java

@@ -29,7 +29,7 @@ public PatchOperationPath unmarshal(String v) throws Exception {
     if (v == null) {
       return null;
     }
-    return new PatchOperationPath(v);
+    return PatchOperationPath.fromString(v);
   }
 
   @Override

scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/patch/PatchOperationPath.java

@@ -19,8 +19,10 @@
 
 package org.apache.directory.scim.spec.patch;
 
-import org.antlr.v4.runtime.ANTLRInputStream;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
 import org.antlr.v4.runtime.BaseErrorListener;
+import org.antlr.v4.runtime.CharStreams;
 import org.antlr.v4.runtime.CommonTokenStream;
 import org.antlr.v4.runtime.RecognitionException;
 import org.antlr.v4.runtime.Recognizer;
@@ -31,29 +33,25 @@
 import org.apache.directory.scim.spec.filter.FilterParser;
 import org.apache.directory.scim.spec.filter.FilterParseException;
 import org.apache.directory.scim.spec.filter.ValuePathExpression;
-import lombok.Data;
 import lombok.extern.slf4j.Slf4j;
 
 import java.io.Serializable;
 
-@Data
 @Slf4j
+@EqualsAndHashCode
 public class PatchOperationPath implements Serializable {
 
   private static final long serialVersionUID = 449365558879593512L;
 
-  private ValuePathExpression valuePathExpression;
+  @Getter
+  private final ValuePathExpression valuePathExpression;
 
-  public PatchOperationPath() {
-
+  public PatchOperationPath(ValuePathExpression valuePathExpression) {
+    this. valuePathExpression = valuePathExpression;
   }
 
-  public PatchOperationPath(String patchPath) throws FilterParseException {
-    parsePatchPath(patchPath);
-  }
-
-  protected void parsePatchPath(String patchPath) throws FilterParseException {
-    FilterLexer l = new FilterLexer(new ANTLRInputStream(patchPath));
+  static ValuePathExpression parsePatchPath(String patchPath) throws FilterParseException {
+    FilterLexer l = new FilterLexer(CharStreams.fromString(patchPath));
     FilterParser p = new FilterParser(new CommonTokenStream(l));
     p.setBuildParseTree(true);
 
@@ -69,7 +67,7 @@ public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol, int
       PatchPathListener patchPathListener = new PatchPathListener();
       ParseTreeWalker.DEFAULT.walk(patchPathListener, tree);
 
-      this.valuePathExpression = patchPathListener.getValuePathExpression();
+      return patchPathListener.getValuePathExpression();
     } catch (IllegalStateException e) {
       throw new FilterParseException(e);
     }
@@ -80,4 +78,8 @@ public String toString() {
     return valuePathExpression.toFilter();
   }
 
+  public static PatchOperationPath fromString(String patchPath) throws FilterParseException {
+    return new PatchOperationPath(parsePatchPath(patchPath));
+  }
+
 }

scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/resources/PhoneNumber.java

@@ -321,17 +321,17 @@ public abstract static class PhoneNumberBuilder {
 
     static final Logger LOGGER = LoggerFactory.getLogger(PhoneNumberBuilder.class);
 
-    final String HYPHEN = "-";
-    final String INTERNATIONAL_PREFIX = "+";
-    final String PREFIX = "tel:%s";
-    final String EXTENSTION_PREFIX = ";ext=%s";
-    final String ISUB_PREFIX = ";isub=%s";
-    final String CONTEXT_PREFIX = ";phone-context=%s";
-    final String PARAMS_STRING = ";%s=%s";
-    final String LOCAL_SUBSCRIBER_NUMBER_REGEX = "^[\\d\\.\\-\\(\\)]+$";
-    final String DOMAIN_NAME_REGEX = "^[a-zA-Z0-9\\.\\-]+$";
-    final String GLOBAL_NUMBER_REGEX = "^(\\+)?[\\d\\.\\-\\(\\)]+$";
-    final String COUNTRY_CODE_REGEX = "^(\\+)?[1-9][0-9]{0,2}$";
+    static final String HYPHEN = "-";
+    static final String INTERNATIONAL_PREFIX = "+";
+    static final String PREFIX = "tel:%s";
+    static final String EXTENSTION_PREFIX = ";ext=%s";
+    static final String ISUB_PREFIX = ";isub=%s";
+    static final String CONTEXT_PREFIX = ";phone-context=%s";
+    static final String PARAMS_STRING = ";%s=%s";
+    static final String LOCAL_SUBSCRIBER_NUMBER_REGEX = "^[\\d\\.\\-\\(\\)]+$";
+    static final String DOMAIN_NAME_REGEX = "^[a-zA-Z0-9\\.\\-]+$";
+    static final String GLOBAL_NUMBER_REGEX = "^(\\+)?[\\d\\.\\-\\(\\)]+$";
+    static final String COUNTRY_CODE_REGEX = "^(\\+)?[1-9][0-9]{0,2}$";
 
     String number;
     String display;

scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/patch/PatchOperationPathTest.java

@@ -39,7 +39,7 @@ public static String[] pathValues() {
   @ParameterizedTest
   @MethodSource("pathValues")
   public void testPathParsing(String value) throws Exception {
-    PatchOperationPath path = new PatchOperationPath(value);
+    PatchOperationPath path = PatchOperationPath.fromString(value);
     log.debug("ValuePathExpression: {}", path.getValuePathExpression());
 
     String result = path.toString();

src/pmd/ruleset.xml

@@ -22,6 +22,10 @@ under the License.
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
 
+  <description>
+    PMD Ruleset for used by Apache SCIMple.
+  </description>
+
   <rule ref="category/java/bestpractices.xml/AvoidUsingHardCodedIP" />
   <rule ref="category/java/bestpractices.xml/CheckResultSet" />
   <rule ref="category/java/bestpractices.xml/PrimitiveWrapperInstantiation" />

src/spotbugs/excludes.xml

@@ -61,4 +61,9 @@
   <!-- doesn't seem to play well with lombok -->
   <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"/>
 
+  <Match>
+    <Class name="org.apache.directory.scim.client.rest.BaseScimClient"/>
+    <Bug pattern="CT_CONSTRUCTOR_THROW"/>
+  </Match>
+
 </FindBugsFilter>

support/spring-boot/src/main/java/org/apache/directory/scim/spring/ScimpleSpringConfiguration.java

@@ -83,7 +83,9 @@ Instance<SelfIdResolver> selfIdResolverInstance(SelfIdResolver selfIdResolver) {
   @Bean
   @ConditionalOnMissingBean
   RepositoryRegistry repositoryRegistry(SchemaRegistry schemaRegistry, List<Repository<? extends ScimResource>> scimResources) {
-    return new RepositoryRegistry(schemaRegistry, scimResources);
+    RepositoryRegistry registry = new RepositoryRegistry(schemaRegistry);
+    registry.registerRepositories(scimResources);
+    return registry;
   }
 
   @Bean