From 73297c74cf12ab83f6e7889e8c8adf0eb8708b43 Mon Sep 17 00:00:00 2001 From: peterxcli Date: Tue, 25 Feb 2025 01:35:46 +0800 Subject: [PATCH] HDDS-1234. Fix spotbugs warnings in ozone-manager --- .../dev-support/findbugsExcludeFile.xml | 73 ------------------- .../ozone/om/TestOmMetadataManager.java | 23 +----- .../ozone/om/TestOzoneManagerHttpServer.java | 11 ++- ...zoneManagerDoubleBufferWithOMResponse.java | 15 ---- .../om/ratis/TestOzoneManagerRatisServer.java | 2 - .../TestOMDelegationTokenRequest.java | 2 - .../TestFSODirectoryPathResolver.java | 26 ++++--- .../om/snapshot/TestSnapshotDiffManager.java | 4 +- .../security/TestOzoneTokenIdentifier.java | 4 +- 9 files changed, 30 insertions(+), 130 deletions(-) diff --git a/hadoop-ozone/ozone-manager/dev-support/findbugsExcludeFile.xml b/hadoop-ozone/ozone-manager/dev-support/findbugsExcludeFile.xml index 400cb170cb3d..55abc2630178 100644 --- a/hadoop-ozone/ozone-manager/dev-support/findbugsExcludeFile.xml +++ b/hadoop-ozone/ozone-manager/dev-support/findbugsExcludeFile.xml @@ -16,77 +16,4 @@ limitations under the License. --> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java index e40acfe905e8..5c0699c2cbf2 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java @@ -205,16 +205,13 @@ public void testListAllVolumes() throws Exception { @Test public void testListBuckets() throws Exception { - String volumeName1 = "volumeA"; String prefixBucketNameWithOzoneOwner = "ozoneBucket"; String prefixBucketNameWithHadoopOwner = "hadoopBucket"; OMRequestTestUtils.addVolumeToDB(volumeName1, omMetadataManager); - TreeSet volumeABucketsPrefixWithOzoneOwner = new TreeSet<>(); - TreeSet volumeABucketsPrefixWithHadoopOwner = new TreeSet<>(); // Add exact name in prefixBucketNameWithOzoneOwner without postfix. volumeABucketsPrefixWithOzoneOwner.add(prefixBucketNameWithOzoneOwner); @@ -225,24 +222,18 @@ public void testListBuckets() throws Exception { prefixBucketNameWithOzoneOwner + i); addBucketsToCache(volumeName1, prefixBucketNameWithOzoneOwner + i); } else { - volumeABucketsPrefixWithHadoopOwner.add( - prefixBucketNameWithHadoopOwner + i); addBucketsToCache(volumeName1, prefixBucketNameWithHadoopOwner + i); } } String volumeName2 = "volumeB"; - TreeSet volumeBBucketsPrefixWithOzoneOwner = new TreeSet<>(); TreeSet volumeBBucketsPrefixWithHadoopOwner = new TreeSet<>(); OMRequestTestUtils.addVolumeToDB(volumeName2, omMetadataManager); // Add exact name in prefixBucketNameWithOzoneOwner without postfix. - volumeBBucketsPrefixWithOzoneOwner.add(prefixBucketNameWithOzoneOwner); addBucketsToCache(volumeName2, prefixBucketNameWithOzoneOwner); for (int i = 1; i < 100; i++) { if (i % 2 == 0) { // This part adds 49 buckets. - volumeBBucketsPrefixWithOzoneOwner.add( - prefixBucketNameWithOzoneOwner + i); addBucketsToCache(volumeName2, prefixBucketNameWithOzoneOwner + i); } else { volumeBBucketsPrefixWithHadoopOwner.add( @@ -251,6 +242,8 @@ public void testListBuckets() throws Exception { } } + // VOLUME A + // List all buckets which have prefix ozoneBucket List omBucketInfoList = omMetadataManager.listBuckets(volumeName1, @@ -291,7 +284,7 @@ public void testListBuckets() throws Exception { assertNotEquals(prefixBucketNameWithOzoneOwner + 10, omBucketInfo.getBucketName()); } - + // VOLUME B omBucketInfoList = omMetadataManager.listBuckets(volumeName2, null, prefixBucketNameWithHadoopOwner, 100, false); @@ -370,26 +363,19 @@ public void testListKeys() throws Exception { String prefixKeyB = "key-b"; String prefixKeyC = "key-c"; TreeSet keysASet = new TreeSet<>(); - TreeSet keysBSet = new TreeSet<>(); - TreeSet keysCSet = new TreeSet<>(); for (int i = 1; i <= 100; i++) { if (i % 2 == 0) { keysASet.add(prefixKeyA + i); addKeysToOM(volumeNameA, ozoneBucket, prefixKeyA + i, i); } else { - keysBSet.add(prefixKeyB + i); addKeysToOM(volumeNameA, hadoopBucket, prefixKeyB + i, i); } } - keysCSet.add(prefixKeyC + 1); addKeysToOM(volumeNameA, ozoneTestBucket, prefixKeyC + 0, 0); - TreeSet keysAVolumeBSet = new TreeSet<>(); TreeSet keysBVolumeBSet = new TreeSet<>(); for (int i = 1; i <= 100; i++) { if (i % 2 == 0) { - keysAVolumeBSet.add( - prefixKeyA + i); addKeysToOM(volumeNameB, ozoneBucket, prefixKeyA + i, i); } else { keysBVolumeBSet.add( @@ -524,6 +510,7 @@ public void testListKeysWithFewDeleteEntriesInCache() throws Exception { // As in total 100, 50 are marked for delete. It should list only 50 keys. assertEquals(50, omKeyInfoList.size()); + assertEquals(50, deleteKeySet.size()); TreeSet expectedKeys = new TreeSet<>(); @@ -593,7 +580,6 @@ public void testListOpenFiles(BucketLayout bucketLayout) throws Exception { } int numOpenKeys = 3; - List openKeys = new ArrayList<>(); for (int i = 0; i < numOpenKeys; i++) { final OmKeyInfo keyInfo = OMRequestTestUtils.createOmKeyInfo(volumeName, bucketName, keyPrefix + i, RatisReplicationConfig.getInstance(ONE)) @@ -613,7 +599,6 @@ public void testListOpenFiles(BucketLayout bucketLayout) throws Exception { dbOpenKeyName = omMetadataManager.getOpenKey(volumeName, bucketName, keyInfo.getKeyName(), clientID); } - openKeys.add(dbOpenKeyName); } String dbPrefix; diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHttpServer.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHttpServer.java index bde49ebd56d2..9cc925cbb920 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHttpServer.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHttpServer.java @@ -21,6 +21,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; +import java.io.IOException; import java.net.InetSocketAddress; import java.net.URL; import java.net.URLConnection; @@ -59,10 +60,9 @@ public static Collection policy() { } @BeforeAll public static void setUp(@TempDir File baseDir) throws Exception { - // Create metadata directory ozoneMetadataDirectory = new File(baseDir.getPath(), "metadata"); - ozoneMetadataDirectory.mkdirs(); + assertTrue(ozoneMetadataDirectory.mkdirs()); // Initialize the OzoneConfiguration conf = new OzoneConfiguration(); @@ -146,15 +146,14 @@ private static boolean canAccess(String scheme, InetSocketAddress addr) { return false; } try { - URL url = - new URL(scheme + "://" + NetUtils.getHostPortString(addr) + "/jmx"); + URL url = new URL(scheme + "://" + NetUtils.getHostPortString(addr) + "/jmx"); URLConnection conn = connectionFactory.openConnection(url); conn.connect(); conn.getContent(); - } catch (Exception e) { + return true; + } catch (IOException e) { return false; } - return true; } private static boolean implies(boolean a, boolean b) { diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerDoubleBufferWithOMResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerDoubleBufferWithOMResponse.java index dc619920a98d..f6587010c0ee 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerDoubleBufferWithOMResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerDoubleBufferWithOMResponse.java @@ -61,8 +61,6 @@ import org.apache.hadoop.ozone.om.response.volume.OMVolumeCreateResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.BucketInfo; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteBucketResponse; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.util.Daemon; import org.apache.ozone.test.GenericTestUtils; @@ -487,18 +485,5 @@ private OMBucketCreateResponse createBucket(String volumeName, doubleBuffer.add(omClientResponse, termIndex); return (OMBucketCreateResponse) omClientResponse; } - - /** - * Create OMBucketDeleteResponse for specified volume and bucket. - * @return OMBucketDeleteResponse - */ - private OMBucketDeleteResponse deleteBucket(String volumeName, - String bucketName) { - return new OMBucketDeleteResponse(OMResponse.newBuilder() - .setCmdType(OzoneManagerProtocolProtos.Type.DeleteBucket) - .setStatus(OzoneManagerProtocolProtos.Status.OK) - .setDeleteBucketResponse(DeleteBucketResponse.newBuilder().build()) - .build(), volumeName, bucketName); - } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java index 486ad6e5bd49..7c2bd2aa62c7 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java @@ -76,7 +76,6 @@ public class TestOzoneManagerRatisServer { private OMMetadataManager omMetadataManager; private OzoneManager ozoneManager; private OMNodeDetails omNodeDetails; - private TermIndex initialTermIndex; private SecurityConfig secConfig; private OMCertificateClient certClient; @@ -115,7 +114,6 @@ public void init(@TempDir Path metaDirPath) throws Exception { omMetadataManager = new OmMetadataManagerImpl(ozoneConfiguration, ozoneManager); when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager); - initialTermIndex = TermIndex.valueOf(0, 0); when(ozoneManager.getTransactionInfo()).thenReturn(TransactionInfo.DEFAULT_VALUE); when(ozoneManager.getConfiguration()).thenReturn(conf); final OmConfig omConfig = conf.getObject(OmConfig.class); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/security/TestOMDelegationTokenRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/security/TestOMDelegationTokenRequest.java index f275eebfa65b..767861f7c572 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/security/TestOMDelegationTokenRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/security/TestOMDelegationTokenRequest.java @@ -26,7 +26,6 @@ import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.om.OMMetadataManager; -import org.apache.hadoop.ozone.om.OMMetrics; import org.apache.hadoop.ozone.om.OmMetadataManagerImpl; import org.apache.hadoop.ozone.om.OzoneManager; import org.junit.jupiter.api.AfterEach; @@ -43,7 +42,6 @@ public class TestOMDelegationTokenRequest { private Path folder; protected OzoneManager ozoneManager; - protected OMMetrics omMetrics; protected OMMetadataManager omMetadataManager; protected ConfigurationSource conf; diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestFSODirectoryPathResolver.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestFSODirectoryPathResolver.java index 6ea7ce8ef988..6c3e5fef1552 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestFSODirectoryPathResolver.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestFSODirectoryPathResolver.java @@ -132,15 +132,23 @@ public void testGetAbsolutePathForValidObjectIDs() throws IOException { Map absolutePathMap = fsoDirectoryPathResolver .getAbsolutePathForObjectIDs(Optional.of(objIds)); - assertEquals(ImmutableMap.of( - 17L, Paths.get("/dir3/dir14/dir17"), - 9L, Paths.get("/dir2/dir9"), - 10L, Paths.get("/dir2/dir10"), - 15L, Paths.get("/dir3/dir15"), - 4L, Paths.get("/dir4"), - 3L, Paths.get("/dir3"), - 1L, Paths.get("/") - ), absolutePathMap); + Map pathMapping = ImmutableMap.builder() + .put(17L, "/dir3/dir14/dir17") + .put(9L, "/dir2/dir9") + .put(10L, "/dir2/dir10") + .put(15L, "/dir3/dir15") + .put(4L, "/dir4") + .put(3L, "/dir3") + .put(1L, "/") + .build(); + + Map expectedPaths = pathMapping.entrySet().stream() + .collect(ImmutableMap.toImmutableMap( + Map.Entry::getKey, + e -> Paths.get(e.getValue()) + )); + + assertEquals(expectedPaths, absolutePathMap); assertEquals(objIds.size(), absolutePathMap.size()); // Invalid Obj Id 19 with dirInfo dir19 which is not present in the bucket. assertThrows(IllegalArgumentException.class, diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java index 4ef0d854b0eb..674c00fc4f41 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java @@ -49,6 +49,7 @@ import static org.apache.ratis.util.JavaUtils.attempt; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -680,12 +681,13 @@ public void testObjectIdMapWithTombstoneEntries(boolean nativeLibraryLoaded, SnapshotDiffManager spy = spy(snapshotDiffManager); - doAnswer(invocation -> { + Boolean isKeyInBucket = doAnswer(invocation -> { String[] split = invocation.getArgument(0, String.class).split("/"); String keyName = split[split.length - 1]; return Integer.parseInt(keyName.substring(3)) % 2 == 0; } ).when(spy).isKeyInBucket(anyString(), anyMap(), anyString()); + assertFalse(isKeyInBucket); PersistentMap oldObjectIdKeyMap = new StubbedPersistentMap<>(); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestOzoneTokenIdentifier.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestOzoneTokenIdentifier.java index bf0b22dc5355..43cb7593437d 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestOzoneTokenIdentifier.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestOzoneTokenIdentifier.java @@ -208,7 +208,6 @@ public void testSymmetricTokenPerf() { public void testSymmetricTokenPerfHelper(String hmacAlgorithm, int keyLen) { final int testTokenCount = 1000; List tokenIds = new ArrayList<>(); - List tokenPasswordSym = new ArrayList<>(); for (int i = 0; i < testTokenCount; i++) { tokenIds.add(generateTestToken()); } @@ -234,8 +233,7 @@ public void testSymmetricTokenPerfHelper(String hmacAlgorithm, int keyLen) { long startTime = Time.monotonicNowNanos(); for (int i = 0; i < testTokenCount; i++) { - tokenPasswordSym.add( - signTokenSymmetric(tokenIds.get(i), mac, secretKey)); + signTokenSymmetric(tokenIds.get(i), mac, secretKey); } long duration = Time.monotonicNowNanos() - startTime; LOG.info("Average token sign time with {}({} symmetric key) is {} ns",