Skip to content

Commit

Permalink
Rename & enforce constants to be all uppercase (apache#10673)
Browse files Browse the repository at this point in the history
  • Loading branch information
attilakreiner authored Jul 11, 2024
1 parent 61c9b6f commit 41e1951
Show file tree
Hide file tree
Showing 40 changed files with 359 additions and 346 deletions.
4 changes: 4 additions & 0 deletions .baseline/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,10 @@
<property name="format" value="^[a-z][a-zA-Z0-9]+$"/>
<message key="name.invalidPattern" value="Member name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="ConstantName">
<property name="format" value="^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$"/>
<message key="name.invalidPattern" value="Constant name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="MethodName"> <!-- Java Style Guide: Method names -->
<property name="format" value="^[a-z][a-zA-Z0-9_]+$"/>
<message key="name.invalidPattern" value="Method name ''{0}'' must match pattern ''{1}''."/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public class TestOSSOutputStream extends AliyunOSSTestBase {
private final OSS ossMock = mock(OSS.class, delegatesTo(ossClient));

private final Path tmpDir = Files.createTempDirectory("oss-file-io-test-");
private static final Random random = ThreadLocalRandom.current();
private static final Random RANDOM = ThreadLocalRandom.current();

private final AliyunProperties props =
new AliyunProperties(
Expand Down Expand Up @@ -127,7 +127,7 @@ private byte[] data256() {

private byte[] randomData(int size) {
byte[] data = new byte[size];
random.nextBytes(data);
RANDOM.nextBytes(data);
return data;
}

Expand Down
6 changes: 3 additions & 3 deletions api/src/main/java/org/apache/iceberg/events/Listeners.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,19 @@
public class Listeners {
private Listeners() {}

private static final Map<Class<?>, Queue<Listener<?>>> listeners = Maps.newConcurrentMap();
private static final Map<Class<?>, Queue<Listener<?>>> LISTENERS = Maps.newConcurrentMap();

public static <E> void register(Listener<E> listener, Class<E> eventType) {
Queue<Listener<?>> list =
listeners.computeIfAbsent(eventType, k -> new ConcurrentLinkedQueue<>());
LISTENERS.computeIfAbsent(eventType, k -> new ConcurrentLinkedQueue<>());
list.add(listener);
}

@SuppressWarnings("unchecked")
public static <E> void notifyAll(E event) {
Preconditions.checkNotNull(event, "Cannot notify listeners for a null event.");

Queue<Listener<?>> list = listeners.get(event.getClass());
Queue<Listener<?>> list = LISTENERS.get(event.getClass());
if (list != null) {
for (Listener<?> value : list) {
Listener<E> listener = (Listener<E>) value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import org.apache.iceberg.relocated.com.google.common.collect.Streams;

public class CharSequenceSet implements Set<CharSequence>, Serializable {
private static final ThreadLocal<CharSequenceWrapper> wrappers =
private static final ThreadLocal<CharSequenceWrapper> WRAPPERS =
ThreadLocal.withInitial(() -> CharSequenceWrapper.wrap(null));

public static CharSequenceSet of(Iterable<CharSequence> charSequences) {
Expand Down Expand Up @@ -61,7 +61,7 @@ public boolean isEmpty() {
@Override
public boolean contains(Object obj) {
if (obj instanceof CharSequence) {
CharSequenceWrapper wrapper = wrappers.get();
CharSequenceWrapper wrapper = WRAPPERS.get();
boolean result = wrapperSet.contains(wrapper.set((CharSequence) obj));
wrapper.set(null); // don't hold a reference to the value
return result;
Expand Down Expand Up @@ -109,7 +109,7 @@ public boolean add(CharSequence charSequence) {
@Override
public boolean remove(Object obj) {
if (obj instanceof CharSequence) {
CharSequenceWrapper wrapper = wrappers.get();
CharSequenceWrapper wrapper = WRAPPERS.get();
boolean result = wrapperSet.remove(wrapper.set((CharSequence) obj));
wrapper.set(null); // don't hold a reference to the value
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@
import org.junit.jupiter.api.Test;

public class TestAggregateBinding {
private static final List<UnboundAggregate<Integer>> list =
private static final List<UnboundAggregate<Integer>> LIST =
ImmutableList.of(Expressions.count("x"), Expressions.max("x"), Expressions.min("x"));
private static final StructType struct =
private static final StructType STRUCT =
StructType.of(Types.NestedField.required(10, "x", Types.IntegerType.get()));

@Test
public void testAggregateBinding() {
for (UnboundAggregate<Integer> unbound : list) {
Expression expr = unbound.bind(struct, true);
for (UnboundAggregate<Integer> unbound : LIST) {
Expression expr = unbound.bind(STRUCT, true);
BoundAggregate<Integer, ?> bound = assertAndUnwrapAggregate(expr);
assertThat(bound.ref().fieldId()).as("Should reference correct field ID").isEqualTo(10);
assertThat(bound.op())
Expand All @@ -60,15 +60,15 @@ public void testCountStarBinding() {
@Test
public void testBoundAggregateFails() {
Expression unbound = Expressions.count("x");
assertThatThrownBy(() -> Binder.bind(struct, Binder.bind(struct, unbound)))
assertThatThrownBy(() -> Binder.bind(STRUCT, Binder.bind(STRUCT, unbound)))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Found already bound aggregate");
}

@Test
public void testCaseInsensitiveReference() {
Expression expr = Expressions.max("X");
Expression boundExpr = Binder.bind(struct, expr, false);
Expression boundExpr = Binder.bind(STRUCT, expr, false);
BoundAggregate<Integer, Integer> bound = assertAndUnwrapAggregate(boundExpr);
assertThat(bound.ref().fieldId()).as("Should reference correct field ID").isEqualTo(10);
assertThat(bound.op())
Expand All @@ -79,15 +79,15 @@ public void testCaseInsensitiveReference() {
@Test
public void testCaseSensitiveReference() {
Expression expr = Expressions.max("X");
assertThatThrownBy(() -> Binder.bind(struct, expr, true))
assertThatThrownBy(() -> Binder.bind(STRUCT, expr, true))
.isInstanceOf(ValidationException.class)
.hasMessageContaining("Cannot find field 'X' in struct");
}

@Test
public void testMissingField() {
UnboundAggregate<?> unbound = Expressions.count("missing");
assertThatThrownBy(() -> unbound.bind(struct, false))
assertThatThrownBy(() -> unbound.bind(STRUCT, false))
.isInstanceOf(ValidationException.class)
.hasMessageContaining("Cannot find field 'missing' in struct:");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public class TestAggregateEvaluator {
// upper bounds
ImmutableMap.of(1, toByteBuffer(IntegerType.get(), 3333)));

private static final DataFile[] dataFiles = {
private static final DataFile[] DATA_FILES = {
FILE, MISSING_SOME_NULLS_STATS_1, MISSING_SOME_NULLS_STATS_2
};

Expand Down Expand Up @@ -121,7 +121,7 @@ public void testIntAggregate() {
Expressions.min("id"));
AggregateEvaluator aggregateEvaluator = AggregateEvaluator.create(SCHEMA, list);

for (DataFile dataFile : dataFiles) {
for (DataFile dataFile : DATA_FILES) {
aggregateEvaluator.update(dataFile);
}

Expand All @@ -141,7 +141,7 @@ public void testAllNulls() {
Expressions.min("all_nulls"));
AggregateEvaluator aggregateEvaluator = AggregateEvaluator.create(SCHEMA, list);

for (DataFile dataFile : dataFiles) {
for (DataFile dataFile : DATA_FILES) {
aggregateEvaluator.update(dataFile);
}

Expand All @@ -160,7 +160,7 @@ public void testSomeNulls() {
Expressions.max("some_nulls"),
Expressions.min("some_nulls"));
AggregateEvaluator aggregateEvaluator = AggregateEvaluator.create(SCHEMA, list);
for (DataFile dataFile : dataFiles) {
for (DataFile dataFile : DATA_FILES) {
aggregateEvaluator.update(dataFile);
}

Expand All @@ -179,7 +179,7 @@ public void testNoStats() {
Expressions.max("no_stats"),
Expressions.min("no_stats"));
AggregateEvaluator aggregateEvaluator = AggregateEvaluator.create(SCHEMA, list);
for (DataFile dataFile : dataFiles) {
for (DataFile dataFile : DATA_FILES) {
aggregateEvaluator.update(dataFile);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@

final class ArrowVectorAccessors {

private static final GenericArrowVectorAccessorFactory<?, String, ?, ?> factory;
private static final GenericArrowVectorAccessorFactory<?, String, ?, ?> FACTORY;

static {
factory =
FACTORY =
new GenericArrowVectorAccessorFactory<>(
JavaDecimalFactory::new,
JavaStringFactory::new,
Expand All @@ -51,7 +51,7 @@ private ArrowVectorAccessors() {
}

static ArrowVectorAccessor<?, String, ?, ?> getVectorAccessor(VectorHolder holder) {
return factory.getVectorAccessor(holder);
return FACTORY.getVectorAccessor(holder);
}

private static final class JavaStringFactory implements StringFactory<String> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,16 @@ public class GlueTestBase {
private static final Logger LOG = LoggerFactory.getLogger(GlueTestBase.class);

// the integration test requires the following env variables
static final String testBucketName = AwsIntegTestUtil.testBucketName();
static final String TEST_BUCKET_NAME = AwsIntegTestUtil.testBucketName();

static final String catalogName = "glue";
static final String testPathPrefix = getRandomName();
static final List<String> namespaces = Lists.newArrayList();
static final String CATALOG_NAME = "glue";
static final String TEST_PATH_PREFIX = getRandomName();
static final List<String> NAMESPACES = Lists.newArrayList();

// aws clients
static final AwsClientFactory clientFactory = AwsClientFactories.defaultFactory();
static final GlueClient glue = clientFactory.glue();
static final S3Client s3 = clientFactory.s3();
static final AwsClientFactory CLIENT_FACTORY = AwsClientFactories.defaultFactory();
static final GlueClient GLUE = CLIENT_FACTORY.glue();
static final S3Client S3 = CLIENT_FACTORY.s3();

// iceberg
static GlueCatalog glueCatalog;
Expand All @@ -74,14 +74,14 @@ public class GlueTestBase {
new Schema(Types.NestedField.required(1, "c1", Types.StringType.get(), "c1"));
static PartitionSpec partitionSpec = PartitionSpec.builderFor(schema).build();
// table location properties
static final Map<String, String> tableLocationProperties =
static final Map<String, String> TABLE_LOCATION_PROPERTIES =
ImmutableMap.of(
TableProperties.WRITE_DATA_LOCATION, "s3://" + testBucketName + "/writeDataLoc",
TableProperties.WRITE_METADATA_LOCATION, "s3://" + testBucketName + "/writeMetaDataLoc",
TableProperties.WRITE_DATA_LOCATION, "s3://" + TEST_BUCKET_NAME + "/writeDataLoc",
TableProperties.WRITE_METADATA_LOCATION, "s3://" + TEST_BUCKET_NAME + "/writeMetaDataLoc",
TableProperties.WRITE_FOLDER_STORAGE_LOCATION,
"s3://" + testBucketName + "/writeFolderStorageLoc");
"s3://" + TEST_BUCKET_NAME + "/writeFolderStorageLoc");

static final String testBucketPath = "s3://" + testBucketName + "/" + testPathPrefix;
static final String TEST_BUCKET_PATH = "s3://" + TEST_BUCKET_NAME + "/" + TEST_PATH_PREFIX;

@BeforeAll
public static void beforeClass() {
Expand All @@ -90,31 +90,31 @@ public static void beforeClass() {
S3FileIOProperties s3FileIOProperties = new S3FileIOProperties();
s3FileIOProperties.setDeleteBatchSize(10);
glueCatalog.initialize(
catalogName,
testBucketPath,
CATALOG_NAME,
TEST_BUCKET_PATH,
awsProperties,
s3FileIOProperties,
glue,
GLUE,
null,
ImmutableMap.of());

glueCatalogWithSkipNameValidation = new GlueCatalog();
AwsProperties propertiesSkipNameValidation = new AwsProperties();
propertiesSkipNameValidation.setGlueCatalogSkipNameValidation(true);
glueCatalogWithSkipNameValidation.initialize(
catalogName,
testBucketPath,
CATALOG_NAME,
TEST_BUCKET_PATH,
propertiesSkipNameValidation,
new S3FileIOProperties(),
glue,
GLUE,
null,
ImmutableMap.of());
}

@AfterAll
public static void afterClass() {
AwsIntegTestUtil.cleanGlueCatalog(glue, namespaces);
AwsIntegTestUtil.cleanS3Bucket(s3, testBucketName, testPathPrefix);
AwsIntegTestUtil.cleanGlueCatalog(GLUE, NAMESPACES);
AwsIntegTestUtil.cleanS3Bucket(S3, TEST_BUCKET_NAME, TEST_PATH_PREFIX);
}

public static String getRandomName() {
Expand All @@ -123,7 +123,7 @@ public static String getRandomName() {

public static String createNamespace() {
String namespace = getRandomName();
namespaces.add(namespace);
NAMESPACES.add(namespace);
glueCatalog.createNamespace(Namespace.of(namespace));
return namespace;
}
Expand All @@ -142,7 +142,7 @@ public static String createTable(String namespace, String tableName) {
public static void updateTableDescription(
String namespace, String tableName, String description) {
GetTableResponse response =
glue.getTable(GetTableRequest.builder().databaseName(namespace).name(tableName).build());
GLUE.getTable(GetTableRequest.builder().databaseName(namespace).name(tableName).build());
Table table = response.table();
UpdateTableRequest request =
UpdateTableRequest.builder()
Expand All @@ -159,13 +159,13 @@ public static void updateTableDescription(
.storageDescriptor(table.storageDescriptor())
.build())
.build();
glue.updateTable(request);
GLUE.updateTable(request);
}

public static void updateTableColumns(
String namespace, String tableName, Function<Column, Column> columnUpdater) {
GetTableResponse response =
glue.getTable(GetTableRequest.builder().databaseName(namespace).name(tableName).build());
GLUE.getTable(GetTableRequest.builder().databaseName(namespace).name(tableName).build());
Table existingTable = response.table();
List<Column> updatedColumns =
existingTable.storageDescriptor().columns().stream()
Expand All @@ -192,6 +192,6 @@ public static void updateTableColumns(
.build())
.build())
.build();
glue.updateTable(request);
GLUE.updateTable(request);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ private void breakFallbackCatalogCommitCheck(GlueTableOperations spyOperations)

private boolean metadataFileExists(TableMetadata metadata) {
try {
s3.headObject(
S3.headObject(
HeadObjectRequest.builder()
.bucket(S3TestUtil.getBucketFromUri(metadata.metadataFileLocation()))
.key(S3TestUtil.getKeyFromUri(metadata.metadataFileLocation()))
Expand All @@ -523,7 +523,7 @@ private boolean metadataFileExists(TableMetadata metadata) {

private int metadataFileCount(TableMetadata metadata) {
return (int)
s3
S3
.listObjectsV2(
ListObjectsV2Request.builder()
.bucket(S3TestUtil.getBucketFromUri(metadata.metadataFileLocation()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,18 @@ public class TestGlueCatalogLock extends GlueTestBase {
@BeforeAll
public static void beforeClass() {
GlueTestBase.beforeClass();
String testBucketPath = "s3://" + testBucketName + "/" + testPathPrefix;
String testBucketPath = "s3://" + TEST_BUCKET_NAME + "/" + TEST_PATH_PREFIX;
lockTableName = getRandomName();
glueCatalog = new GlueCatalog();
AwsProperties awsProperties = new AwsProperties();
S3FileIOProperties s3FileIOProperties = new S3FileIOProperties();
dynamo = clientFactory.dynamo();
dynamo = CLIENT_FACTORY.dynamo();
glueCatalog.initialize(
catalogName,
CATALOG_NAME,
testBucketPath,
awsProperties,
s3FileIOProperties,
glue,
GLUE,
new DynamoDbLockManager(dynamo, lockTableName),
ImmutableMap.of());
}
Expand Down
Loading

0 comments on commit 41e1951

Please sign in to comment.