apache · nastra · Jun 18, 2024 · Jun 17, 2024
diff --git a/.baseline/checkstyle/checkstyle.xml b/.baseline/checkstyle/checkstyle.xml
@@ -92,6 +92,10 @@
         <property name="format" value="^\s*import\s+static\s+(?!\Qorg.assertj.core.api.Assertions.\E).*\.assertThatThrownBy;"/>
         <property name="message" value="assertThatThrownBy() should be statically imported from org.assertj.core.api.Assertions"/>
     </module>
+    <module name="RegexpMultiline">
+        <property name="format" value="^\s*import\s+\Qorg.assertj.core.api.Assertions;\E" />
+        <property name="message" value="org.assertj.core.api.Assertions should only be used with static imports" />
+    </module>
     <module name="SuppressionFilter"> <!-- baseline-gradle: README.md -->
         <property name="file" value="${config_loc}/checkstyle-suppressions.xml"/>
     </module>

diff --git a/aliyun/src/test/java/org/apache/iceberg/aliyun/TestAliyunClientFactories.java b/aliyun/src/test/java/org/apache/iceberg/aliyun/TestAliyunClientFactories.java
@@ -18,31 +18,32 @@
  */
 package org.apache.iceberg.aliyun;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 import com.aliyun.oss.OSS;
 import java.util.Map;
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
 import org.apache.iceberg.relocated.com.google.common.collect.Maps;
-import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 public class TestAliyunClientFactories {
 
   @Test
   public void testLoadDefault() {
-    Assertions.assertThat(AliyunClientFactories.defaultFactory())
+    assertThat(AliyunClientFactories.defaultFactory())
         .as("Default client should be singleton")
         .isEqualTo(AliyunClientFactories.defaultFactory());
 
     AliyunClientFactory defaultFactory = AliyunClientFactories.from(Maps.newHashMap());
-    Assertions.assertThat(defaultFactory)
+    assertThat(defaultFactory)
         .as("Should load default when factory impl not configured")
         .isInstanceOf(AliyunClientFactories.DefaultAliyunClientFactory.class);
 
-    Assertions.assertThat(defaultFactory.aliyunProperties().accessKeyId())
+    assertThat(defaultFactory.aliyunProperties().accessKeyId())
         .as("Should have no Aliyun properties set")
         .isNull();
 
-    Assertions.assertThat(defaultFactory.aliyunProperties().securityToken())
+    assertThat(defaultFactory.aliyunProperties().securityToken())
         .as("Should have no security token")
         .isNull();
 
@@ -53,15 +54,15 @@ public void testLoadDefault() {
                 "key",
                 AliyunProperties.CLIENT_SECURITY_TOKEN,
                 "token"));
-    Assertions.assertThat(defaultFactoryWithConfig)
+    assertThat(defaultFactoryWithConfig)
         .as("Should load default when factory impl not configured")
         .isInstanceOf(AliyunClientFactories.DefaultAliyunClientFactory.class);
 
-    Assertions.assertThat(defaultFactoryWithConfig.aliyunProperties().accessKeyId())
+    assertThat(defaultFactoryWithConfig.aliyunProperties().accessKeyId())
         .as("Should have access key set")
         .isEqualTo("key");
 
-    Assertions.assertThat(defaultFactoryWithConfig.aliyunProperties().securityToken())
+    assertThat(defaultFactoryWithConfig.aliyunProperties().securityToken())
         .as("Should have security token set")
         .isEqualTo("token");
   }
@@ -70,7 +71,7 @@ public void testLoadDefault() {
   public void testLoadCustom() {
     Map<String, String> properties = Maps.newHashMap();
     properties.put(AliyunProperties.CLIENT_FACTORY, CustomFactory.class.getName());
-    Assertions.assertThat(AliyunClientFactories.from(properties))
+    assertThat(AliyunClientFactories.from(properties))
         .as("Should load custom class")
         .isInstanceOf(CustomFactory.class);
   }

diff --git a/aliyun/src/test/java/org/apache/iceberg/aliyun/oss/TestOSSFileIO.java b/aliyun/src/test/java/org/apache/iceberg/aliyun/oss/TestOSSFileIO.java
@@ -18,6 +18,8 @@
  */
 package org.apache.iceberg.aliyun.oss;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 import com.aliyun.oss.OSS;
 import com.aliyun.oss.OSSClient;
 import com.aliyun.oss.OSSClientBuilder;
@@ -39,7 +41,6 @@
 import org.apache.iceberg.relocated.com.google.common.io.ByteStreams;
 import org.apache.iceberg.util.SerializableSupplier;
 import org.apache.iceberg.util.SerializationUtil;
-import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -73,33 +74,29 @@ public void testOutputFile() throws IOException {
     writeOSSData(out, data);
 
     OSSURI uri = new OSSURI(location);
-    Assertions.assertThat(ossClient().get().doesObjectExist(uri.bucket(), uri.key()))
+    assertThat(ossClient().get().doesObjectExist(uri.bucket(), uri.key()))
         .as("OSS file should exist")
         .isTrue();
-    Assertions.assertThat(out.location()).as("Should have expected location").isEqualTo(location);
-    Assertions.assertThat(ossDataLength(uri)).as("Should have expected length").isEqualTo(dataSize);
-    Assertions.assertThat(ossDataContent(uri, dataSize))
-        .as("Should have expected content")
-        .isEqualTo(data);
+    assertThat(out.location()).as("Should have expected location").isEqualTo(location);
+    assertThat(ossDataLength(uri)).as("Should have expected length").isEqualTo(dataSize);
+    assertThat(ossDataContent(uri, dataSize)).as("Should have expected content").isEqualTo(data);
   }
 
   @Test
   public void testInputFile() throws IOException {
     String location = randomLocation();
     InputFile in = fileIO().newInputFile(location);
-    Assertions.assertThat(in.exists()).as("OSS file should not exist").isFalse();
+    assertThat(in.exists()).as("OSS file should not exist").isFalse();
 
     int dataSize = 1024 * 10;
     byte[] data = randomData(dataSize);
     OutputFile out = fileIO().newOutputFile(location);
     writeOSSData(out, data);
 
-    Assertions.assertThat(in.exists()).as("OSS file should exist").isTrue();
-    Assertions.assertThat(in.location()).as("Should have expected location").isEqualTo(location);
-    Assertions.assertThat(in.getLength()).as("Should have expected length").isEqualTo(dataSize);
-    Assertions.assertThat(inFileContent(in, dataSize))
-        .as("Should have expected content")
-        .isEqualTo(data);
+    assertThat(in.exists()).as("OSS file should exist").isTrue();
+    assertThat(in.location()).as("Should have expected location").isEqualTo(location);
+    assertThat(in.getLength()).as("Should have expected length").isEqualTo(dataSize);
+    assertThat(inFileContent(in, dataSize)).as("Should have expected content").isEqualTo(data);
   }
 
   @Test
@@ -111,22 +108,20 @@ public void testDeleteFile() throws IOException {
     writeOSSData(out, data);
 
     InputFile in = fileIO().newInputFile(location);
-    Assertions.assertThat(in.exists()).as("OSS file should exist").isTrue();
+    assertThat(in.exists()).as("OSS file should exist").isTrue();
 
     fileIO().deleteFile(in);
-    Assertions.assertThat(fileIO().newInputFile(location).exists())
-        .as("OSS file should not exist")
-        .isFalse();
+    assertThat(fileIO().newInputFile(location).exists()).as("OSS file should not exist").isFalse();
   }
 
   @Test
   public void testLoadFileIO() {
     FileIO file = CatalogUtil.loadFileIO(OSS_IMPL_CLASS, ImmutableMap.of(), conf);
-    Assertions.assertThat(file).as("Should be OSSFileIO").isInstanceOf(OSSFileIO.class);
+    assertThat(file).as("Should be OSSFileIO").isInstanceOf(OSSFileIO.class);
 
     byte[] data = SerializationUtil.serializeToBytes(file);
     FileIO expectedFileIO = SerializationUtil.deserializeFromBytes(data);
-    Assertions.assertThat(expectedFileIO)
+    assertThat(expectedFileIO)
         .as("The deserialized FileIO should be OSSFileIO")
         .isInstanceOf(OSSFileIO.class);
   }
@@ -143,22 +138,20 @@ public void serializeClient() throws URISyntaxException {
     SerializableSupplier<OSS> post = SerializationUtil.deserializeFromBytes(data);
 
     OSS client = post.get();
-    Assertions.assertThat(client)
-        .as("Should be instance of oss client")
-        .isInstanceOf(OSSClient.class);
+    assertThat(client).as("Should be instance of oss client").isInstanceOf(OSSClient.class);
 
     OSSClient oss = (OSSClient) client;
-    Assertions.assertThat(oss.getEndpoint())
+    assertThat(oss.getEndpoint())
         .as("Should have expected endpoint")
         .isEqualTo(new URI("http://" + endpoint));
 
-    Assertions.assertThat(oss.getCredentialsProvider().getCredentials().getAccessKeyId())
+    assertThat(oss.getCredentialsProvider().getCredentials().getAccessKeyId())
         .as("Should have expected access key")
         .isEqualTo(accessKeyId);
-    Assertions.assertThat(oss.getCredentialsProvider().getCredentials().getSecretAccessKey())
+    assertThat(oss.getCredentialsProvider().getCredentials().getSecretAccessKey())
         .as("Should have expected secret key")
         .isEqualTo(accessSecret);
-    Assertions.assertThat(oss.getCredentialsProvider().getCredentials().getSecurityToken())
+    assertThat(oss.getCredentialsProvider().getCredentials().getSecurityToken())
         .as("Should have no security token")
         .isNull();
   }

diff --git a/aliyun/src/test/java/org/apache/iceberg/aliyun/oss/TestOSSInputFile.java b/aliyun/src/test/java/org/apache/iceberg/aliyun/oss/TestOSSInputFile.java
@@ -18,6 +18,8 @@
  */
 package org.apache.iceberg.aliyun.oss;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.AdditionalAnswers.delegatesTo;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.reset;
@@ -36,7 +38,6 @@
 import org.apache.iceberg.io.SeekableInputStream;
 import org.apache.iceberg.metrics.MetricsContext;
 import org.apache.iceberg.relocated.com.google.common.io.ByteStreams;
-import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 public class TestOSSInputFile extends AliyunOSSTestBase {
@@ -60,7 +61,7 @@ public void testReadFile() throws Exception {
   @Test
   public void testOSSInputFile() {
     OSSURI uri = randomURI();
-    Assertions.assertThatThrownBy(
+    assertThatThrownBy(
             () ->
                 new OSSInputFile(
                     ossClient().get(), uri, aliyunProperties, -1, MetricsContext.nullMetrics()))
@@ -74,15 +75,15 @@ public void testExists() {
 
     InputFile inputFile =
         new OSSInputFile(ossMock, uri, aliyunProperties, MetricsContext.nullMetrics());
-    Assertions.assertThat(inputFile.exists()).as("OSS file should not exist").isFalse();
+    assertThat(inputFile.exists()).as("OSS file should not exist").isFalse();
     verify(ossMock, times(1)).getSimplifiedObjectMeta(uri.bucket(), uri.key());
     reset(ossMock);
 
     int dataSize = 1024;
     byte[] data = randomData(dataSize);
     writeOSSData(uri, data);
 
-    Assertions.assertThat(inputFile.exists()).as("OSS file should  exist").isTrue();
+    assertThat(inputFile.exists()).as("OSS file should  exist").isTrue();
     inputFile.exists();
     verify(ossMock, times(1)).getSimplifiedObjectMeta(uri.bucket(), uri.key());
     reset(ossMock);
@@ -108,17 +109,15 @@ public void testGetLength() {
   private void readAndVerify(OSSURI uri, byte[] data) throws IOException {
     InputFile inputFile =
         new OSSInputFile(ossClient().get(), uri, aliyunProperties, MetricsContext.nullMetrics());
-    Assertions.assertThat(inputFile.exists()).as("OSS file should exist").isTrue();
-    Assertions.assertThat(inputFile.getLength())
-        .as("Should have expected file length")
-        .isEqualTo(data.length);
+    assertThat(inputFile.exists()).as("OSS file should exist").isTrue();
+    assertThat(inputFile.getLength()).as("Should have expected file length").isEqualTo(data.length);
 
     byte[] actual = new byte[data.length];
     try (SeekableInputStream in = inputFile.newStream()) {
       ByteStreams.readFully(in, actual);
     }
 
-    Assertions.assertThat(actual).as("Should have same object content").isEqualTo(data);
+    assertThat(actual).as("Should have same object content").isEqualTo(data);
   }
 
   private void verifyLength(OSS ossClientMock, OSSURI uri, byte[] data, boolean isCache) {
@@ -132,9 +131,7 @@ private void verifyLength(OSS ossClientMock, OSSURI uri, byte[] data, boolean is
           new OSSInputFile(ossClientMock, uri, aliyunProperties, MetricsContext.nullMetrics());
     }
     inputFile.getLength();
-    Assertions.assertThat(inputFile.getLength())
-        .as("Should have expected file length")
-        .isEqualTo(data.length);
+    assertThat(inputFile.getLength()).as("Should have expected file length").isEqualTo(data.length);
   }
 
   private OSSURI randomURI() {

diff --git a/aliyun/src/test/java/org/apache/iceberg/aliyun/oss/TestOSSInputStream.java b/aliyun/src/test/java/org/apache/iceberg/aliyun/oss/TestOSSInputStream.java
@@ -18,14 +18,16 @@
  */
 package org.apache.iceberg.aliyun.oss;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Random;
 import java.util.concurrent.ThreadLocalRandom;
 import org.apache.iceberg.io.SeekableInputStream;
 import org.apache.iceberg.relocated.com.google.common.io.ByteStreams;
-import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 public class TestOSSInputStream extends AliyunOSSTestBase {
@@ -69,7 +71,7 @@ private void readAndCheck(
       SeekableInputStream in, long rangeStart, int size, byte[] original, boolean buffered)
       throws IOException {
     in.seek(rangeStart);
-    Assertions.assertThat(in.getPos()).as("Should have the correct position").isEqualTo(rangeStart);
+    assertThat(in.getPos()).as("Should have the correct position").isEqualTo(rangeStart);
 
     long rangeEnd = rangeStart + size;
     byte[] actual = new byte[size];
@@ -83,9 +85,9 @@ private void readAndCheck(
       }
     }
 
-    Assertions.assertThat(in.getPos()).as("Should have the correct position").isEqualTo(rangeEnd);
+    assertThat(in.getPos()).as("Should have the correct position").isEqualTo(rangeEnd);
 
-    Assertions.assertThat(actual)
+    assertThat(actual)
         .as("Should have expected range data")
         .isEqualTo(Arrays.copyOfRange(original, (int) rangeStart, (int) rangeEnd));
   }
@@ -95,7 +97,7 @@ public void testClose() throws Exception {
     OSSURI uri = new OSSURI(location("closed.dat"));
     SeekableInputStream closed = new OSSInputStream(ossClient().get(), uri);
     closed.close();
-    Assertions.assertThatThrownBy(() -> closed.seek(0))
+    assertThatThrownBy(() -> closed.seek(0))
         .isInstanceOf(IllegalStateException.class)
         .hasMessageContaining("Cannot seek: already closed");
   }
@@ -111,7 +113,7 @@ public void testSeek() throws Exception {
       in.seek(expected.length / 2);
       byte[] actual = new byte[expected.length / 2];
       ByteStreams.readFully(in, actual);
-      Assertions.assertThat(actual)
+      assertThat(actual)
           .as("Should have expected seeking stream")
           .isEqualTo(Arrays.copyOfRange(expected, expected.length / 2, expected.length));
     }