Skip to content

Commit

Permalink
Build warning fixes (#75)
Browse files Browse the repository at this point in the history
* Simplify Stream API
* Simplify collection creation
* Lambdas & Method refs
* Pattern variables
* Final fields
* Simplify if/else
* if/else -> switch
* isEmpty()
* Unnecessary null checks
* requireNonNullElseGet
* Diamond operator
* Boxing / unboxing
* Unnecessary StringBuilder
* Unnecessary modifier
* Unnecessary semicolon
* Visibility issues
* Static access
* Usage of raw class
* Sequenced collection
  • Loading branch information
adutra authored Aug 6, 2024
1 parent 74cfe50 commit 353acea
Show file tree
Hide file tree
Showing 37 changed files with 189 additions and 221 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ public class PolarisEclipseLinkMetaStoreSessionImpl implements PolarisMetaStoreS
private static final Logger LOG =
LoggerFactory.getLogger(PolarisEclipseLinkMetaStoreSessionImpl.class);

private EntityManagerFactory emf;
private ThreadLocal<EntityManager> localSession = new ThreadLocal<>();
private final EntityManagerFactory emf;
private final ThreadLocal<EntityManager> localSession = new ThreadLocal<>();
private final PolarisEclipseLinkStore store;
private final PolarisStorageIntegrationProvider storageIntegrationProvider;
private static volatile Map<String, String> properties;
Expand Down Expand Up @@ -119,8 +119,8 @@ public PolarisEclipseLinkMetaStoreSessionImpl(

/** Load the persistence unit properties from a given configuration file */
private Map<String, String> loadProperties(String confFile, String persistenceUnitName) {
if (this.properties != null) {
return this.properties;
if (properties != null) {
return properties;
}

try {
Expand All @@ -141,7 +141,7 @@ private Map<String, String> loadProperties(String confFile, String persistenceUn
nodeMap.getNamedItem("value").getNodeValue());
}

this.properties = properties;
PolarisEclipseLinkMetaStoreSessionImpl.properties = properties;
return properties;
} catch (Exception e) {
LOG.warn(
Expand Down Expand Up @@ -262,10 +262,10 @@ public void writeToEntities(

/** {@inheritDoc} */
@Override
public void persistStorageIntegrationIfNeeded(
public <T extends PolarisStorageConfigurationInfo> void persistStorageIntegrationIfNeeded(
@NotNull PolarisCallContext callContext,
@NotNull PolarisBaseEntity entity,
@Nullable PolarisStorageIntegration storageIntegration) {
@Nullable PolarisStorageIntegration<T> storageIntegration) {
// not implemented for eclipselink store
}

Expand Down Expand Up @@ -373,7 +373,7 @@ public void deleteAll(@NotNull PolarisCallContext callCtx) {
public @NotNull List<PolarisBaseEntity> lookupEntities(
@NotNull PolarisCallContext callCtx, List<PolarisEntityId> entityIds) {
return this.store.lookupEntities(localSession.get(), entityIds).stream()
.map(model -> ModelEntity.toEntity(model))
.map(ModelEntity::toEntity)
.toList();
}

Expand Down Expand Up @@ -477,7 +477,7 @@ public List<PolarisEntityActiveRecord> lookupEntityActiveBatch(
return this.store
.lookupFullEntitiesActive(localSession.get(), catalogId, parentId, entityType)
.stream()
.map(model -> ModelEntity.toEntity(model))
.map(ModelEntity::toEntity)
.filter(entityFilter)
.limit(limit)
.map(transformer)
Expand Down Expand Up @@ -534,7 +534,7 @@ public int lookupEntityGrantRecordsVersion(
return this.store
.lookupAllGrantRecordsOnSecurable(localSession.get(), securableCatalogId, securableId)
.stream()
.map(model -> ModelGrantRecord.toGrantRecord(model))
.map(ModelGrantRecord::toGrantRecord)
.toList();
}

Expand All @@ -546,7 +546,7 @@ public int lookupEntityGrantRecordsVersion(
return this.store
.lookupGrantRecordsOnGrantee(localSession.get(), granteeCatalogId, granteeId)
.stream()
.map(model -> ModelGrantRecord.toGrantRecord(model))
.map(ModelGrantRecord::toGrantRecord)
.toList();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public class PolarisEclipseLinkStore {
private static final Logger LOG = LoggerFactory.getLogger(PolarisEclipseLinkStore.class);

// diagnostic services
private PolarisDiagnostics diagnosticServices;
private final PolarisDiagnostics diagnosticServices;

/**
* Constructor, allocate everything at once
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,11 @@ public void setActivatedPrincipalRoles(List<PrincipalRoleEntity> activatedPrinci

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("principalEntity=" + getPrincipalEntity());
sb.append(";activatedPrincipalRoleNames=" + getActivatedPrincipalRoleNames());
sb.append(";activatedPrincipalRoles=" + getActivatedPrincipalRoles());
return sb.toString();
return "principalEntity="
+ getPrincipalEntity()
+ ";activatedPrincipalRoleNames="
+ getActivatedPrincipalRoleNames()
+ ";activatedPrincipalRoles="
+ getActivatedPrincipalRoles();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package io.polaris.core.catalog;

import com.google.common.collect.ImmutableList;
import io.polaris.core.entity.PolarisEntity;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -36,10 +37,11 @@ public class PolarisCatalogHelpers {
private PolarisCatalogHelpers() {}

public static List<String> tableIdentifierToList(TableIdentifier identifier) {
List<String> fullList = new ArrayList<>();
ImmutableList.Builder<String> fullList =
ImmutableList.builderWithExpectedSize(identifier.namespace().length() + 1);
fullList.addAll(Arrays.asList(identifier.namespace().levels()));
fullList.add(identifier.name());
return fullList;
return fullList.build();
}

public static TableIdentifier listToTableIdentifier(List<String> ids) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public interface CallContext extends AutoCloseable {

// For requests that make use of a Catalog instance, this holds the instance that was
// created, scoped to the current call context.
public static final String REQUEST_PATH_CATALOG_INSTANCE_KEY = "REQUEST_PATH_CATALOG_INSTANCE";
String REQUEST_PATH_CATALOG_INSTANCE_KEY = "REQUEST_PATH_CATALOG_INSTANCE";

// Authenticator filters should populate this field alongside resolving a SecurityContext.
// Value type: AuthenticatedPolarisPrincipal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,20 @@ public NameAndId nameAndId() {

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("name=" + getName());
sb.append(";id=" + getId());
sb.append(";parentId=" + getParentId());
sb.append(";entityVersion=" + getEntityVersion());
sb.append(";type=" + getType());
sb.append(";subType=" + getSubType());
sb.append(";internalProperties=" + getInternalPropertiesAsMap());
return sb.toString();
return "name="
+ getName()
+ ";id="
+ getId()
+ ";parentId="
+ getParentId()
+ ";entityVersion="
+ getEntityVersion()
+ ";type="
+ getType()
+ ";subType="
+ getSubType()
+ ";internalProperties="
+ getInternalPropertiesAsMap();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@
public abstract class LocalPolarisMetaStoreManagerFactory<StoreType>
implements MetaStoreManagerFactory {

Map<String, PolarisMetaStoreManager> metaStoreManagerMap = new HashMap<>();
Map<String, StorageCredentialCache> storageCredentialCacheMap = new HashMap<>();
Map<String, StoreType> backingStoreMap = new HashMap<>();
Map<String, Supplier<PolarisMetaStoreSession>> sessionSupplierMap = new HashMap<>();
protected PolarisDiagnostics diagServices = new PolarisDefaultDiagServiceImpl();
final Map<String, PolarisMetaStoreManager> metaStoreManagerMap = new HashMap<>();
final Map<String, StorageCredentialCache> storageCredentialCacheMap = new HashMap<>();
final Map<String, StoreType> backingStoreMap = new HashMap<>();
final Map<String, Supplier<PolarisMetaStoreSession>> sessionSupplierMap = new HashMap<>();
protected final PolarisDiagnostics diagServices = new PolarisDefaultDiagServiceImpl();

protected PolarisStorageIntegrationProvider storageIntegration;

private Logger logger =
private final Logger logger =
org.slf4j.LoggerFactory.getLogger(LocalPolarisMetaStoreManagerFactory.class);

protected abstract StoreType createBackingStore(@NotNull PolarisDiagnostics diagnostics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,20 +150,25 @@ public int getAttemptCount() {
while (jParser.nextToken() != JsonToken.END_OBJECT) {
if (jParser.getCurrentToken() == JsonToken.FIELD_NAME) {
String fieldName = jParser.currentName();
if (fieldName.equals(PolarisTaskConstants.LAST_ATTEMPT_EXECUTOR_ID)) {
jParser.nextToken();
executorId = jParser.getText();
} else if (fieldName.equals(PolarisTaskConstants.LAST_ATTEMPT_START_TIME)) {
jParser.nextToken();
lastAttemptStartTime = Long.parseLong(jParser.getText());
} else if (fieldName.equals(PolarisTaskConstants.ATTEMPT_COUNT)) {
jParser.nextToken();
attemptCount = Integer.parseInt(jParser.getText());
} else {
JsonToken next = jParser.nextToken();
if (next == JsonToken.START_OBJECT || next == JsonToken.START_ARRAY) {
jParser.skipChildren();
}
switch (fieldName) {
case PolarisTaskConstants.LAST_ATTEMPT_EXECUTOR_ID:
jParser.nextToken();
executorId = jParser.getText();
break;
case PolarisTaskConstants.LAST_ATTEMPT_START_TIME:
jParser.nextToken();
lastAttemptStartTime = Long.parseLong(jParser.getText());
break;
case PolarisTaskConstants.ATTEMPT_COUNT:
jParser.nextToken();
attemptCount = Integer.parseInt(jParser.getText());
break;
default:
JsonToken next = jParser.nextToken();
if (next == JsonToken.START_OBJECT || next == JsonToken.START_ARRAY) {
jParser.skipChildren();
}
break;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public List<PolarisEntity> getRawFullPath() {
if (resolvedPath == null) {
return null;
}
return resolvedPath.stream().map(resolved -> resolved.getEntity()).toList();
return resolvedPath.stream().map(ResolvedPolarisEntity::getEntity).toList();
}

public List<ResolvedPolarisEntity> getResolvedParentPath() {
Expand All @@ -68,14 +68,11 @@ public List<PolarisEntity> getRawParentPath() {
if (resolvedPath == null) {
return null;
}
return getResolvedParentPath().stream().map(resolved -> resolved.getEntity()).toList();
return getResolvedParentPath().stream().map(ResolvedPolarisEntity::getEntity).toList();
}

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("resolvedPath:");
sb.append(resolvedPath);
return sb.toString();
return "resolvedPath:" + resolvedPath;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ public void writeToEntities(

/** {@inheritDoc} */
@Override
public void persistStorageIntegrationIfNeeded(
public <T extends PolarisStorageConfigurationInfo> void persistStorageIntegrationIfNeeded(
@NotNull PolarisCallContext callContext,
@NotNull PolarisBaseEntity entity,
@Nullable PolarisStorageIntegration storageIntegration) {
@Nullable PolarisStorageIntegration<T> storageIntegration) {
// not implemented for in-memory store
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,11 @@ public List<PolarisGrantRecord> getGrantRecordsAsSecurable() {

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("entity:");
sb.append(entity);
sb.append(";grantRecordsAsGrantee:");
sb.append(grantRecordsAsGrantee);
sb.append(";grantRecordsAsSecurable:");
sb.append(grantRecordsAsSecurable);
return sb.toString();
return "entity:"
+ entity
+ ";grantRecordsAsGrantee:"
+ grantRecordsAsGrantee
+ ";grantRecordsAsSecurable:"
+ grantRecordsAsSecurable;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,20 @@
public class EntityCacheEntry {

// epoch time (ns) when the cache entry was added to the cache
private long createdOnNanoTimestamp;
private final long createdOnNanoTimestamp;

// epoch time (ns) when the cache entry was added to the cache
private long lastAccessedNanoTimestamp;

// the entity which have been cached.
private PolarisBaseEntity entity;
private final PolarisBaseEntity entity;

// grants associated to this entity, for a principal, a principal role, or a catalog role these
// are role usage
// grants on that entity. For a catalog securable (i.e. a catalog, namespace, or table_like
// securable), these are
// the grants on this securable.
private List<PolarisGrantRecord> grantRecords;
private final List<PolarisGrantRecord> grantRecords;

/**
* Constructor used when an entry is initially created after loading the entity and its grants
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,7 @@ public PolarisResolvedPathWrapper getPassthroughResolvedPath(Object key) {
List<ResolvedPolarisEntity> resolvedEntities = new ArrayList<>();
resolvedEntities.add(
new ResolvedPolarisEntity(passthroughResolver.getResolvedReferenceCatalog()));
resolvedPath.stream()
.forEach(cacheEntry -> resolvedEntities.add(new ResolvedPolarisEntity(cacheEntry)));
resolvedPath.forEach(cacheEntry -> resolvedEntities.add(new ResolvedPolarisEntity(cacheEntry)));
LOG.debug("Returning resolvedEntities from getPassthroughResolvedPath: {}", resolvedEntities);
return new PolarisResolvedPathWrapper(resolvedEntities);
}
Expand Down Expand Up @@ -320,7 +319,7 @@ public PolarisEntitySubType getLeafSubType(Object key) {
pathLookup);
int index = pathLookup.get(key);
List<EntityCacheEntry> resolved = primaryResolver.getResolvedPaths().get(index);
if (resolved.size() == 0) {
if (resolved.isEmpty()) {
return PolarisEntitySubType.NULL_SUBTYPE;
}
return resolved.get(resolved.size() - 1).getEntity().getSubType();
Expand Down Expand Up @@ -360,8 +359,7 @@ public PolarisResolvedPathWrapper getResolvedPath(Object key, boolean prependRoo
resolvedEntities.add(getResolvedRootContainerEntity());
}
resolvedEntities.add(new ResolvedPolarisEntity(primaryResolver.getResolvedReferenceCatalog()));
resolvedPath.stream()
.forEach(cacheEntry -> resolvedEntities.add(new ResolvedPolarisEntity(cacheEntry)));
resolvedPath.forEach(cacheEntry -> resolvedEntities.add(new ResolvedPolarisEntity(cacheEntry)));
return new PolarisResolvedPathWrapper(resolvedEntities);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,11 @@ public boolean isOptional() {

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("entityNames:");
sb.append(entityNames.toString());
sb.append(";lastEntityType:");
sb.append(lastEntityType.toString());
sb.append(";isOptional:");
sb.append(isOptional);
return sb.toString();
return "entityNames:"
+ entityNames.toString()
+ ";lastEntityType:"
+ lastEntityType.toString()
+ ";isOptional:"
+ isOptional;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public enum StatusEnum {

// error, an entity could not be resolved
ENTITY_COULD_NOT_BE_RESOLVED,
};
}

private final StatusEnum status;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class AwsStorageConfigurationInfo extends PolarisStorageConfigurationInfo
@JsonIgnore private static final int MAX_ALLOWED_LOCATIONS = 5;

// Technically, it should be ^arn:(aws|aws-cn|aws-us-gov):iam::\d{12}:role/.+$,
@JsonIgnore public static String ROLE_ARN_PATTERN = "^arn:aws:iam::\\d{12}:role/.+$";
@JsonIgnore public static final String ROLE_ARN_PATTERN = "^arn:aws:iam::\\d{12}:role/.+$";

// AWS role to be assumed
private final @NotNull String roleARN;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,9 @@ public S3Client s3() {
.applyMutation(s3FileIOProperties::applyEndpointConfigurations)
.applyMutation(s3FileIOProperties::applyServiceConfigurations)
.applyMutation(
s3ClientBuilder -> {
s3FileIOProperties.applyCredentialConfigurations(
awsClientProperties, s3ClientBuilder);
})
s3ClientBuilder ->
s3FileIOProperties.applyCredentialConfigurations(
awsClientProperties, s3ClientBuilder))
.applyMutation(s3FileIOProperties::applySignerConfiguration)
.applyMutation(s3FileIOProperties::applyS3AccessGrantsConfigurations)
.applyMutation(s3ClientBuilder -> s3ClientBuilder.crossRegionAccessEnabled(true))
Expand Down
Loading

0 comments on commit 353acea

Please sign in to comment.