chore(errorprone): Enabling EqualsGetClass, PatternMatchingInstanceof, and UnusedMethod in ErrorProne

api/iceberg-service/src/main/java/org/apache/polaris/service/types/NotificationRequest.java

@@ -54,7 +54,7 @@ public boolean equals(Object o) {
     if (this == o) {
       return true;
     }
-    if (o == null || getClass() != o.getClass()) {
+    if (!(o instanceof NotificationRequest)) {
       return false;
     }
     NotificationRequest notificationRequest = (NotificationRequest) o;

api/iceberg-service/src/main/java/org/apache/polaris/service/types/TableUpdateNotification.java

@@ -106,7 +106,7 @@ public boolean equals(Object o) {
     if (this == o) {
       return true;
     }
-    if (o == null || getClass() != o.getClass()) {
+    if (!(o instanceof TableUpdateNotification)) {
       return false;
     }
     TableUpdateNotification tableUpdateNotification = (TableUpdateNotification) o;

api/polaris-catalog-service/src/main/java/org/apache/polaris/service/types/PolicyIdentifier.java

@@ -104,7 +104,7 @@ public boolean equals(Object o) {
     if (this == o) {
       return true;
     }
-    if (o == null || getClass() != o.getClass()) {
+    if (!(o instanceof PolicyIdentifier)) {
       return false;
     }
     PolicyIdentifier policyIdentifier = (PolicyIdentifier) o;

codestyle/errorprone-rules.properties

@@ -87,7 +87,7 @@ DistinctVarargsChecker=ERROR
 DoubleCheckedLocking=ERROR
 # Double-checked locking on non-volatile fields is unsafe
 
-# TODO enable: EqualsGetClass=ERROR
+EqualsGetClass=ERROR
 # Prefer instanceof to getClass when implementing Object#equals.
 
 EqualsIncompatibleType=ERROR
@@ -184,7 +184,7 @@ OrphanedFormatString=ERROR
 Overrides=ERROR
 # Varargs doesn't agree for overridden method
 
-# TODO PatternMatchingInstanceof=ERROR
+PatternMatchingInstanceof=ERROR
 # This code can be simplified to use a pattern-matching instanceof.
 
 StreamToIterable=ERROR
@@ -202,7 +202,7 @@ URLEqualsHashCode=ERROR
 UnnecessaryLambda=ERROR
 # Returning a lambda from a helper method or saving it in a constant is unnecessary; prefer to implement the functional interface method directly and use a method reference instead.
 
-# TODO enable: UnusedMethod=ERROR
+UnusedMethod=ERROR
 # Unused.
 
 UnusedNestedClass=ERROR

plugins/spark/v3.5/integration/src/intTest/java/org/apache/polaris/spark/quarkus/it/SparkIntegrationBase.java

@@ -181,8 +181,8 @@ private Object[] toJava(Row row) {
               }
 
               Object value = row.get(pos);
-              if (value instanceof Row) {
-                return toJava((Row) value);
+              if (value instanceof Row valueRow) {
+                return toJava(valueRow);
               } else if (value instanceof scala.collection.Seq) {
                 return row.getList(pos);
               } else if (value instanceof scala.collection.Map) {

plugins/spark/v3.5/spark/src/main/java/org/apache/polaris/spark/rest/CreateGenericTableRequest.java

@@ -145,7 +145,7 @@ public boolean equals(Object o) {
     if (this == o) {
       return true;
     }
-    if (o == null || getClass() != o.getClass()) {
+    if (!(o instanceof CreateGenericTableRequest)) {
       return false;
     }
     CreateGenericTableRequest createGenericTableRequest = (CreateGenericTableRequest) o;

plugins/spark/v3.5/spark/src/main/java/org/apache/polaris/spark/rest/GenericTable.java

@@ -144,7 +144,7 @@ public boolean equals(Object o) {
     if (this == o) {
       return true;
     }
-    if (o == null || getClass() != o.getClass()) {
+    if (!(o instanceof GenericTable)) {
       return false;
     }
     GenericTable genericTable = (GenericTable) o;

plugins/spark/v3.5/spark/src/main/java/org/apache/polaris/spark/rest/ListGenericTablesResponse.java

@@ -98,7 +98,7 @@ public boolean equals(Object o) {
     if (this == o) {
       return true;
     }
-    if (o == null || getClass() != o.getClass()) {
+    if (!(o instanceof ListGenericTablesResponse)) {
       return false;
     }
     ListGenericTablesResponse listGenericTablesResponse = (ListGenericTablesResponse) o;

plugins/spark/v3.5/spark/src/main/java/org/apache/polaris/spark/rest/LoadGenericTableResponse.java

@@ -74,7 +74,7 @@ public boolean equals(Object o) {
     if (this == o) {
       return true;
     }
-    if (o == null || getClass() != o.getClass()) {
+    if (!(o instanceof LoadGenericTableResponse)) {
       return false;
     }
     LoadGenericTableResponse loadGenericTableResponse = (LoadGenericTableResponse) o;

polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntityId.java

@@ -49,7 +49,7 @@ public long getId() {
   @Override
   public boolean equals(Object o) {
     if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
+    if (!(o instanceof PolarisEntityId)) return false;
     PolarisEntityId that = (PolarisEntityId) o;
     return catalogId == that.catalogId && id == that.id;
   }

polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisGrantRecord.java

@@ -139,7 +139,7 @@ public String toString() {
   @Override
   public boolean equals(Object o) {
     if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
+    if (!(o instanceof PolarisGrantRecord)) return false;
     PolarisGrantRecord that = (PolarisGrantRecord) o;
     return securableCatalogId == that.securableCatalogId
         && securableId == that.securableId

polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCacheByNameKey.java

@@ -101,7 +101,7 @@ public String getName() {
   @Override
   public boolean equals(Object o) {
     if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
+    if (!(o instanceof EntityCacheByNameKey)) return false;
     EntityCacheByNameKey that = (EntityCacheByNameKey) o;
     return parentId == that.parentId
         && typeCode == that.typeCode

polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/ResolverEntityName.java

@@ -54,7 +54,7 @@ public boolean isOptional() {
   @Override
   public boolean equals(Object o) {
     if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
+    if (!(o instanceof ResolverEntityName)) return false;
     ResolverEntityName that = (ResolverEntityName) o;
     return getEntityType() == that.getEntityType()
         && Objects.equals(getEntityName(), that.getEntityName());

polaris-core/src/main/java/org/apache/polaris/core/policy/PolarisPolicyMappingRecord.java

@@ -198,7 +198,7 @@ public String toString() {
   @Override
   public boolean equals(Object o) {
     if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
+    if (!(o instanceof PolarisPolicyMappingRecord)) return false;
     PolarisPolicyMappingRecord that = (PolarisPolicyMappingRecord) o;
     return targetCatalogId == that.targetCatalogId
         && targetId == that.targetId

polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BaseResolverTest.java

@@ -403,55 +403,6 @@ protected void testResolveCatalogRole(boolean useCache) {
         this.cache, "test", "R5", ResolverStatus.StatusEnum.ENTITY_COULD_NOT_BE_RESOLVED);
   }
 
-  /**
-   * Create a simple resolver without a reference catalog, any principal roles sub-scope and using
-   * P1 as the caller principal
-   *
-   * @return new resolver to test with
-   */
-  @Nonnull
-  private Resolver allocateResolver() {
-    return this.allocateResolver(null, null);
-  }
-
-  /**
-   * Create a simple resolver without any principal roles sub-scope and using P1 as the caller
-   * principal
-   *
-   * @param referenceCatalogName the reference e catalog name, can be null
-   * @return new resolver to test with
-   */
-  @Nonnull
-  private Resolver allocateResolver(@Nullable String referenceCatalogName) {
-    return this.allocateResolver(null, referenceCatalogName);
-  }
-
-  /**
-   * Create a simple resolver without any principal roles sub-scope and using P1 as the caller
-   * principal
-   *
-   * @param cache if not null, cache to use, else one will be created
-   * @return new resolver to test with
-   */
-  @Nonnull
-  private Resolver allocateResolver(@Nullable InMemoryEntityCache cache) {
-    return this.allocateResolver(cache, null);
-  }
-
-  /**
-   * Create a simple resolver without any principal roles sub-scope and using P1 as the caller
-   * principal
-   *
-   * @param cache if not null, cache to use, else one will be created
-   * @param referenceCatalogName the reference e catalog name, can be null
-   * @return new resolver to test with
-   */
-  @Nonnull
-  private Resolver allocateResolver(
-      @Nullable InMemoryEntityCache cache, @Nullable String referenceCatalogName) {
-    return this.allocateResolver(cache, null, referenceCatalogName);
-  }
-
   /**
    * Create a simple resolver without any principal roles sub-scope and using P1 as the caller
    * principal
@@ -518,72 +469,6 @@ public String getAuthenticationScheme() {
         referenceCatalogName);
   }
 
-  /**
-   * Resolve a principal and optionally a principal role
-   *
-   * @param cache if not null, cache to use
-   * @param principalName name of the principal name being created
-   * @param exists true if this principal already exists
-   * @param principalRoleName name of the principal role, should exist
-   */
-  private void resolvePrincipalAndPrincipalRole(
-      InMemoryEntityCache cache, String principalName, boolean exists, String principalRoleName) {
-    Resolver resolver = allocateResolver(cache);
-
-    // for a principal creation, we simply want to test if the principal we are creating exists
-    // or not
-    resolver.addOptionalEntityByName(PolarisEntityType.PRINCIPAL, principalName);
-
-    // add principal role if one passed-in
-    if (principalRoleName != null) {
-      resolver.addOptionalEntityByName(PolarisEntityType.PRINCIPAL_ROLE, principalRoleName);
-    }
-
-    // done, run resolve
-    ResolverStatus status = resolver.resolveAll();
-
-    // we expect success
-    Assertions.assertThat(status.getStatus()).isEqualTo(ResolverStatus.StatusEnum.SUCCESS);
-
-    // the principal does not exist, check that this is the case
-    if (exists) {
-      // the principal exist, check that this is the case
-      this.ensureResolved(
-          resolver.getResolvedEntity(PolarisEntityType.PRINCIPAL, principalName),
-          PolarisEntityType.PRINCIPAL,
-          principalName);
-    } else {
-      // not found
-      Assertions.assertThat(resolver.getResolvedEntity(PolarisEntityType.PRINCIPAL, principalName))
-          .isNull();
-    }
-
-    // validate that we were able to resolve the principal and the two principal roles
-    this.ensureResolved(resolver.getResolvedCallerPrincipal(), PolarisEntityType.PRINCIPAL, "P1");
-
-    // validate that the two principal roles have been activated
-    List<ResolvedPolarisEntity> principalRolesResolved = resolver.getResolvedCallerPrincipalRoles();
-
-    // expect two principal roles
-    Assertions.assertThat(principalRolesResolved).hasSize(2);
-    principalRolesResolved.sort(Comparator.comparing(p -> p.getEntity().getName()));
-
-    // ensure they are PR1 and PR2
-    this.ensureResolved(principalRolesResolved.get(0), PolarisEntityType.PRINCIPAL_ROLE, "PR1");
-    this.ensureResolved(
-        principalRolesResolved.get(principalRolesResolved.size() - 1),
-        PolarisEntityType.PRINCIPAL_ROLE,
-        "PR2");
-
-    // if a principal role was passed-in, ensure it exists
-    if (principalRoleName != null) {
-      this.ensureResolved(
-          resolver.getResolvedEntity(PolarisEntityType.PRINCIPAL_ROLE, principalRoleName),
-          PolarisEntityType.PRINCIPAL_ROLE,
-          principalRoleName);
-    }
-  }
-
   /**
    * Main resolve driver
    *

runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java

@@ -305,8 +305,6 @@ private void authorizeGrantOnRootContainerToPrincipalRoleOperationOrThrow(
           status.getFailedToResolvedEntityName(), principalRoleName);
     }
 
-    // TODO: Merge this method into authorizeGrantOnTopLevelEntityToPrincipalRoleOperationOrThrow
-    // once we remove any special handling logic for the rootContainer.
     PolarisResolvedPathWrapper rootContainerWrapper =
         resolutionManifest.getResolvedRootContainerEntityAsPath();
     PolarisResolvedPathWrapper principalRoleWrapper =
@@ -321,42 +319,6 @@ private void authorizeGrantOnRootContainerToPrincipalRoleOperationOrThrow(
         principalRoleWrapper);
   }
 
-  private void authorizeGrantOnTopLevelEntityToPrincipalRoleOperationOrThrow(
-      PolarisAuthorizableOperation op,
-      String topLevelEntityName,
-      PolarisEntityType topLevelEntityType,
-      String principalRoleName) {
-    resolutionManifest =
-        resolutionManifestFactory.createResolutionManifest(callContext, securityContext, null);
-    resolutionManifest.addTopLevelName(
-        topLevelEntityName, topLevelEntityType, false /* isOptional */);
-    resolutionManifest.addTopLevelName(
-        principalRoleName, PolarisEntityType.PRINCIPAL_ROLE, false /* isOptional */);
-    ResolverStatus status = resolutionManifest.resolveAll();
-
-    if (status.getStatus() == ResolverStatus.StatusEnum.ENTITY_COULD_NOT_BE_RESOLVED) {
-      throw new NotFoundException(
-          "Entity %s not found when trying to assign %s of type %s to %s",
-          status.getFailedToResolvedEntityName(),
-          topLevelEntityName,
-          topLevelEntityType,
-          principalRoleName);
-    }
-
-    PolarisResolvedPathWrapper topLevelEntityWrapper =
-        resolutionManifest.getResolvedTopLevelEntity(topLevelEntityName, topLevelEntityType);
-    PolarisResolvedPathWrapper principalRoleWrapper =
-        resolutionManifest.getResolvedTopLevelEntity(
-            principalRoleName, PolarisEntityType.PRINCIPAL_ROLE);
-
-    authorizer.authorizeOrThrow(
-        polarisPrincipal,
-        resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(),
-        op,
-        topLevelEntityWrapper,
-        principalRoleWrapper);
-  }
-
   private void authorizeGrantOnPrincipalRoleToPrincipalOperationOrThrow(
       PolarisAuthorizableOperation op, String principalRoleName, String principalName) {
     resolutionManifest =

runtime/service/src/main/java/org/apache/polaris/service/auth/external/mapping/DefaultPrincipalMapper.java

@@ -51,7 +51,7 @@ public OptionalLong mapPrincipalId(SecurityIdentity identity) {
     return principalMapper
         .idClaimPath()
         .map(claimPath -> claimsLocator.locateClaim(claimPath, jwt))
-        .map(id -> id instanceof Number ? ((Number) id).longValue() : Long.parseLong(id.toString()))
+        .map(id -> id instanceof Number number ? number.longValue() : Long.parseLong(id.toString()))
         .map(OptionalLong::of)
         .orElse(OptionalLong.empty());
   }