[WIP] HIVE-28892: Fix spotbugs issues in hive-standalone-metastore-common with spo…

Jenkinsfile

@@ -254,7 +254,8 @@ git merge origin/target
         def spotbugsProjects = [
             ":hive-shims",
             ":hive-storage-api",
-            ":hive-service-rpc"
+            ":hive-service-rpc",
+            ":hive-standalone-metastore-common"
         ]
         sh '''#!/bin/bash
 set -e

hcatalog/core/src/main/java/org/apache/hive/hcatalog/cli/HCatDriver.java

@@ -124,7 +124,7 @@ private int setFSPermsNGrp(SessionState ss, HiveConf conf) {
       } else {
         try {
           Hive db = Hive.get();
-          Path dbPath = new Warehouse(conf).getDatabasePath(db.getDatabase(dbName));
+          Path dbPath = Warehouse.create(conf).getDatabasePath(db.getDatabase(dbName));
           FileSystem fs = dbPath.getFileSystem(conf);
           if (perms != null) {
             fs.setPermission(dbPath, perms);

hcatalog/core/src/test/java/org/apache/hive/hcatalog/cli/TestPermsGrp.java

@@ -110,7 +110,7 @@ public void setUp() throws Exception {
         MetastoreConf.getVar(hcatConf, MetastoreConf.ConfVars.CONNECT_URL_KEY));
     hcatConf.set(HiveConf.ConfVars.METASTORE_URIS.varname,
         MetastoreConf.getVar(hcatConf, MetastoreConf.ConfVars.THRIFT_URIS));
-    clientWH = new Warehouse(hcatConf);
+    clientWH = Warehouse.create(hcatConf);
     msc = new HiveMetaStoreClient(hcatConf);
   }
 

itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/MetastoreHousekeepingLeaderTestBase.java

@@ -65,7 +65,7 @@ void internalSetup(final String leaderHostName, boolean configuredLeader) throws
 
     addHouseKeepingThreadConfigs();
 
-    warehouse = new Warehouse(conf);
+    warehouse = Warehouse.create(conf);
 
     if (isServerStarted) {
       Assert.assertNotNull("Unable to connect to the MetaStore server", client);

itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreTransformer.java

@@ -1450,7 +1450,7 @@ public void testTransformerDatabase() throws Exception {
           db.getLocationUri().contains(conf.get(MetastoreConf.ConfVars.WAREHOUSE_EXTERNAL.getVarname())));
       resetHMSClient();
 
-      Warehouse wh = new Warehouse(conf);
+      Warehouse wh = Warehouse.create(conf);
       String mgdPath = wh.getDefaultDatabasePath(dbWithLocation, false).toString();
       new DatabaseBuilder()
           .setName(dbWithLocation)
@@ -1830,7 +1830,7 @@ private Table createTableWithCapabilities(Map<String, Object> props) throws Exce
           if (cat == null) {
             cat = new Catalog();
             cat.setName(catalog.toLowerCase());
-            Warehouse wh = new Warehouse(conf);
+            Warehouse wh = Warehouse.create(conf);
             cat.setLocationUri(wh.getWhRootExternal().toString() + File.separator + catalog);
             cat.setDescription("Non-hive catalog");
             client.createCatalog(cat);

itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java

@@ -58,7 +58,7 @@ public void testIsWritable() throws Exception {
     try {
       fs.mkdirs(top);
 
-      Warehouse wh = new Warehouse(conf);
+      Warehouse wh = Warehouse.create(conf);
       FsPermission writePerm = FsPermission.createImmutable((short)0777);
       FsPermission noWritePerm = FsPermission.createImmutable((short)0555);
 

itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMultipleEncryptionZones.java

@@ -108,7 +108,7 @@ public static void setUp() throws Exception {
   }
 
   private static void initReplChangeManager() throws Exception{
-    warehouse = new Warehouse(hiveConf);
+    warehouse = Warehouse.create(hiveConf);
     warehouseFs = warehouse.getWhRoot().getFileSystem(hiveConf);
     fs = new Path(cmroot).getFileSystem(hiveConf);
     fs.mkdirs(warehouse.getWhRoot());
@@ -1261,7 +1261,7 @@ public void testClearerEncrypted() throws Exception {
 
     String cmrootCmClearer = "hdfs://" + miniDFSCluster.getNameNode().getHostAndPort() + "/cmrootClearer";
     hiveConfCmClearer.set(HiveConf.ConfVars.REPL_CM_DIR.varname, cmrootCmClearer);
-    Warehouse warehouseCmClearer = new Warehouse(hiveConfCmClearer);
+    Warehouse warehouseCmClearer = Warehouse.create(hiveConfCmClearer);
     FileSystem cmfs = new Path(cmrootCmClearer).getFileSystem(hiveConfCmClearer);
     cmfs.mkdirs(warehouseCmClearer.getWhRoot());
 
@@ -1367,7 +1367,7 @@ public void testCmRootAclPermissions() throws Exception {
 
     String cmRootAclPermissions = "hdfs://" + miniDFSCluster.getNameNode().getHostAndPort() + "/cmRootAclPermissions";
     hiveConfAclPermissions.set(HiveConf.ConfVars.REPL_CM_DIR.varname, cmRootAclPermissions);
-    Warehouse warehouseCmPermissions = new Warehouse(hiveConfAclPermissions);
+    Warehouse warehouseCmPermissions = Warehouse.create(hiveConfAclPermissions);
     FileSystem cmfs = new Path(cmRootAclPermissions).getFileSystem(hiveConfAclPermissions);
     cmfs.mkdirs(warehouseCmPermissions.getWhRoot());
 
@@ -1516,7 +1516,7 @@ public void testCmrootEncrypted() throws Exception {
     //Create cm in encrypted zone
     EncryptionZoneUtils.createEncryptionZone(new Path(cmrootdirEncrypted), "test_key_db", conf);
     ReplChangeManager.resetReplChangeManagerInstance();
-    Warehouse warehouseEncrypted = new Warehouse(encryptedHiveConf);
+    Warehouse warehouseEncrypted = Warehouse.create(encryptedHiveConf);
     FileSystem warehouseFsEncrypted = warehouseEncrypted.getWhRoot().getFileSystem(encryptedHiveConf);
     FileSystem fsCmEncrypted = new Path(cmrootdirEncrypted).getFileSystem(encryptedHiveConf);
     fsCmEncrypted.mkdirs(warehouseEncrypted.getWhRoot());
@@ -1583,7 +1583,7 @@ public void testCmrootFallbackEncrypted() throws Exception {
       ReplChangeManager.resetReplChangeManagerInstance();
       boolean exceptionThrown = false;
       try {
-        new Warehouse(encryptedHiveConf);
+        Warehouse.create(encryptedHiveConf);
       } catch (MetaException e) {
         exceptionThrown = true;
         assertTrue(e.getMessage().contains("should not be encrypted"));
@@ -1620,7 +1620,7 @@ public void testCmrootFallbackRelative() throws Exception {
       ReplChangeManager.resetReplChangeManagerInstance();
       boolean exceptionThrown = false;
       try {
-        new Warehouse(encryptedHiveConf);
+        Warehouse.create(encryptedHiveConf);
       } catch (MetaException e) {
         exceptionThrown = true;
         assertTrue(e.getMessage().contains("should be absolute"));

itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java

@@ -104,7 +104,7 @@ private static void internalSetUpProvidePerm() throws Exception {
     permCmroot = "hdfs://" + permDdfs.getNameNode().getHostAndPort() + "/cmroot";
     permhiveConf.set(HiveConf.ConfVars.REPL_CM_DIR.varname, permCmroot);
     permhiveConf.setInt(CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY, 60);
-    permWarehouse = new Warehouse(permhiveConf);
+    permWarehouse = Warehouse.create(permhiveConf);
   }
 
   private static void internalSetUp() throws Exception {
@@ -116,7 +116,7 @@ private static void internalSetUp() throws Exception {
     cmroot = "hdfs://" + m_dfs.getNameNode().getHostAndPort() + "/cmroot";
     hiveConf.set(HiveConf.ConfVars.REPL_CM_DIR.varname, cmroot);
     hiveConf.setInt(CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY, 60);
-    warehouse = new Warehouse(hiveConf);
+    warehouse = Warehouse.create(hiveConf);
     fs = new Path(cmroot).getFileSystem(hiveConf);
   }
 

itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestTenantBasedStorageHierarchy.java

@@ -478,7 +478,7 @@ private Table createTableWithCapabilities(Map<String, Object> props) throws Exce
         if (cat == null) {
           cat = new Catalog();
           cat.setName(catalog.toLowerCase());
-          Warehouse wh = new Warehouse(conf);
+          Warehouse wh = Warehouse.create(conf);
           cat.setLocationUri(wh.getWhRootExternal().toString() + File.separator + catalog);
           cat.setDescription("Non-hive catalog");
           client.createCatalog(cat);

itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/cache/TestCachedStoreUpdateUsingEvents.java

@@ -93,7 +93,7 @@ public void setUp() throws Exception {
     CachedStore.stopCacheUpdateService(1);
 
     // Create the 'hive' catalog with new warehouse directory
-    HMSHandler.createDefaultCatalog(rawStore, new Warehouse(conf));
+    HMSHandler.createDefaultCatalog(rawStore, Warehouse.create(conf));
   }
 
   private Database createTestDb(String dbName, String dbOwner) throws IOException {

itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExternalTables.java

@@ -1355,7 +1355,7 @@ public void differentCatalogIncrementalReplication() throws Throwable {
     //Create the catalog
     Catalog catalog = new Catalog();
     catalog.setName("spark");
-    Warehouse wh = new Warehouse(conf);
+    Warehouse wh = Warehouse.create(conf);
     catalog.setLocationUri(wh.getWhRootExternal().toString() + File.separator + catalog);
     catalog.setDescription("Non-hive catalog");
     Hive.get(primary.hiveConf).getMSC().createCatalog(catalog);

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/archive/AlterTableArchiveUtils.java

@@ -96,7 +96,7 @@ static Path getInterMediateDir(Path dir, Configuration conf, ConfVars suffixConf
 
   static void deleteDir(Path dir, boolean shouldEnableCm, Configuration conf) throws HiveException {
     try {
-      Warehouse wh = new Warehouse(conf);
+      Warehouse wh = Warehouse.create(conf);
       wh.deleteDir(dir, true, false, shouldEnableCm);
     } catch (MetaException e) {
       throw new HiveException(e);

ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewOperation.java

@@ -87,7 +87,7 @@ public int execute() throws HiveException {
       Map<String, String> tblProps = tbl.getTTable().getParameters();
       Path tlocation = null;
       try {
-        Warehouse wh = new Warehouse(context.getConf());
+        Warehouse wh = Warehouse.create(context.getConf());
         tlocation = wh.getDefaultTablePath(context.getDb().getDatabase(tbl.getDbName()), tbl.getTableName(),
                 tblProps == null || !AcidUtils.isTablePropertyTransactional(tblProps));
       } catch (MetaException e) {

ql/src/java/org/apache/hadoop/hive/ql/exec/repl/bootstrap/load/util/Context.java

@@ -45,7 +45,7 @@ public Context(String dumpDirectory, HiveConf hiveConf, Hive hiveDb,
     this.dumpDirectory = dumpDirectory;
     this.hiveConf = hiveConf;
     this.hiveDb = hiveDb;
-    this.warehouse = new Warehouse(hiveConf);
+    this.warehouse = Warehouse.create(hiveConf);
     this.pathInfo = new PathInfo(hiveConf);
     sessionStateLineageState = lineageState;
     this.nestedContext = nestedContext;

ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java

@@ -4077,7 +4077,7 @@ public List<Partition> dropPartitions(String dbName, String tableName,
       PartitionDropOptions dropOptions) throws HiveException {
     try {
       Table table = getTable(dbName, tableName);
-      if (!dropOptions.deleteData) {
+      if (!dropOptions.isDeleteData()) {
         AcidUtils.TableSnapshot snapshot = AcidUtils.getTableSnapshot(conf, table, true);
         if (snapshot != null) {
           dropOptions.setWriteId(snapshot.getWriteId());

ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java

@@ -158,7 +158,7 @@ public class SessionHiveMetaStoreClient extends HiveMetaStoreClientWithLocalCach
 
   private Warehouse getWh() throws MetaException {
     if (wh == null) {
-      wh = new Warehouse(conf);
+      wh = Warehouse.create(conf);
     }
     return wh;
   }
@@ -1428,8 +1428,8 @@ public boolean dropPartition(String catName, String dbName, String tblName, List
     }
     Partition droppedPartition = tt.dropPartition(partVals);
     boolean result = droppedPartition != null ? true : false;
-    boolean purgeData = options != null ? options.purgeData : true;
-    boolean deleteData = options != null ? options.deleteData : true;
+    boolean purgeData = options != null ? options.isPurgeData() : true;
+    boolean deleteData = options != null ? options.isDeleteData() : true;
     if (deleteData && !tt.isExternal()) {
       result &= deletePartitionLocation(droppedPartition, purgeData);
     }
@@ -1470,8 +1470,8 @@ public List<Partition> dropPartitions(String catName, String dbName, String tblN
         Partition droppedPartition = tt.dropPartition(p.getValues());
         if (droppedPartition != null) {
           result.add(droppedPartition);
-          boolean purgeData = options != null ? options.purgeData : true;
-          boolean deleteData = options != null ? options.deleteData : true;
+          boolean purgeData = options != null ? options.isPurgeData() : true;
+          boolean deleteData = options != null ? options.isDeleteData() : true;
           if (deleteData && !tt.isExternal()) {
             deletePartitionLocation(droppedPartition, purgeData);
           }

ql/src/java/org/apache/hadoop/hive/ql/parse/ImportSemanticAnalyzer.java

@@ -347,7 +347,7 @@ public static boolean prepareImport(boolean isImportCmd,
       }
     }
 
-    Warehouse wh = new Warehouse(x.getConf());
+    Warehouse wh = Warehouse.create(x.getConf());
     Table table = tableIfExists(tblDesc, x.getHive());
     boolean tableExists = false;
 

ql/src/java/org/apache/hadoop/hive/ql/parse/MetaDataExportListener.java

@@ -61,7 +61,7 @@ private void export_meta_data(PreDropTableEvent tableEvent) throws MetaException
         tbl);
     IHMSHandler handler = tableEvent.getHandler();
     Configuration conf = handler.getConf();
-    Warehouse wh = new Warehouse(conf);
+    Warehouse wh = Warehouse.create(conf);
     Path tblPath = new Path(tbl.getSd().getLocation());
     fs = wh.getFs(tblPath);
     Date now = new Date();

ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java

@@ -2632,7 +2632,7 @@ private void getMetaData(QB qb, ReadEntity parentInput)
               String tableName = getUnescapedName((ASTNode) ast.getChild(0));
               String[] names = Utilities.getDbTableName(tableName);
               try {
-                Warehouse wh = new Warehouse(conf);
+                Warehouse wh = Warehouse.create(conf);
                 //Use destination table's db location.
                 String destTableDb = qb.getTableDesc() != null ? qb.getTableDesc().getDatabaseName() : null;
                 if (destTableDb == null) {
@@ -8263,7 +8263,7 @@ private Path getCtasOrCMVLocation(CreateTableDesc tblDesc, CreateMaterializedVie
       }
 
       String[] names = Utilities.getDbTableName(protoName);
-      Warehouse wh = new Warehouse(conf);
+      Warehouse wh = Warehouse.create(conf);
       if (tbl.getSd() == null || tbl.getSd().getLocation() == null) {
         location = wh.getDefaultTablePath(db.getDatabase(names[0]), names[1], false);
       } else {
@@ -8584,7 +8584,7 @@ private void handleLineage(Table destinationTable, LoadTableDesc ltd, Operator o
       try {
         String suffix = Utilities.getTableOrMVSuffix(ctx,
                 AcidUtils.isTableSoftDeleteEnabled(destinationTable, conf));
-        Warehouse wh = new Warehouse(conf);
+        Warehouse wh = Warehouse.create(conf);
         tlocation = wh.getDefaultTablePath(db.getDatabase(tableDesc.getDatabaseName()),
             tName + suffix, tableDesc.isExternal());
 
@@ -8602,7 +8602,7 @@ private void handleLineage(Table destinationTable, LoadTableDesc ltd, Operator o
       Path tlocation;
       String [] dbTable = Utilities.getDbTableName(createVwDesc.getViewName());
       try {
-        Warehouse wh = new Warehouse(conf);
+        Warehouse wh = Warehouse.create(conf);
         Map<String, String> tblProps = createVwDesc.getTblProps();
         tlocation = wh.getDefaultTablePath(db.getDatabase(dbTable[0]), dbTable[1],
           tblProps == null || !AcidUtils.isTablePropertyTransactional(tblProps));
@@ -14456,7 +14456,7 @@ ASTNode analyzeCreateTable(
   private String getDefaultLocation(String dbName, String tableName, boolean isExt) throws SemanticException {
     String tblLocation;
     try {
-      Warehouse wh = new Warehouse(conf);
+      Warehouse wh = Warehouse.create(conf);
       tblLocation = wh.getDefaultTablePath(db.getDatabase(dbName), tableName, isExt).toUri().getPath();
     } catch (MetaException | HiveException e) {
       throw new SemanticException(e);

ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java

@@ -560,7 +560,7 @@ private Path getDefaultCtasOrCMVLocation(final ParseContext pCtx) throws Semanti
       if (!db.databaseExists(names[0])) {
         throw new SemanticException("ERROR: The database " + names[0] + " does not exist.");
       }
-      Warehouse wh = new Warehouse(conf);
+      Warehouse wh = Warehouse.create(conf);
       return wh.getDefaultTablePath(db.getDatabase(names[0]), names[1] + suffix, isExternal);
     } catch (HiveException | MetaException e) {
       throw new SemanticException(e);

ql/src/java/org/apache/hadoop/hive/ql/queryhistory/repository/AbstractRepository.java

@@ -51,7 +51,7 @@ public void init(HiveConf conf, Schema schema) {
     this.conf = conf;
     this.schema = schema;
     try {
-      this.warehouse = new Warehouse(conf);
+      this.warehouse = Warehouse.create(conf);
     } catch (MetaException e) {
       throw new RuntimeException(e);
     }

ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java

@@ -91,7 +91,7 @@ private void initWh() throws MetaException, HiveException {
         // chicken-and-egg problem. So, we now track whether or not we're running from client-side
         // in the SBAP itself.
         hive_db = new HiveProxy(Hive.get(getConf(), StorageBasedAuthorizationProvider.class));
-        this.wh = new Warehouse(getConf());
+        this.wh = Warehouse.create(getConf());
         if (this.wh == null){
           // If wh is still null after just having initialized it, bail out - something's very wrong.
           throw new IllegalStateException("Unable to initialize Warehouse from clientside.");

ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java

@@ -277,7 +277,7 @@ private int aggregateStats(Hive db, Table tbl) {
 
     try {
       // Stats setup:
-      final Warehouse wh = new Warehouse(conf);
+      final Warehouse wh = Warehouse.create(conf);
       if (!getWork().getNoStatsAggregator() && !getWork().isNoScanAnalyzeCommand()) {
         try {
           scc = getContext();

ql/src/java/org/apache/hadoop/hive/ql/util/HiveStrictManagedMigration.java

@@ -567,7 +567,7 @@ private void readControlConfigs(FileSystem fs, Path path) {
     }, runOptions.tablePoolSize);
     wh = ThreadLocal.withInitial(() -> {
       try {
-        return new Warehouse(conf);
+        return Warehouse.create(conf);
       } catch (MetaException e) {
         throw new RuntimeException(e);
       }
@@ -578,7 +578,7 @@ private void readControlConfigs(FileSystem fs, Path path) {
 
       oldWh = ThreadLocal.withInitial(() -> {
         try {
-          return new Warehouse(oldConf);
+          return Warehouse.create(oldConf);
         } catch (MetaException e) {
           throw new RuntimeException(e);
         }

ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java

@@ -344,7 +344,7 @@ public void testMetaStoreApiTiming() throws Throwable {
    * @throws MetaException
    */
   private void validateTable(Table tbl, String tableName) throws MetaException {
-    Warehouse wh = new Warehouse(hiveConf);
+    Warehouse wh = Warehouse.create(hiveConf);
     Table ft = null;
     try {
       // hm.getTable result will not have privileges set (it does not retrieve

ql/src/test/org/apache/hadoop/hive/ql/metadata/TestSessionHiveMetastoreClientExchangePartitionsTempTable.java

@@ -61,7 +61,7 @@ public TestSessionHiveMetastoreClientExchangePartitionsTempTable(String name, Ab
   @Override
   public void setUp() throws Exception {
     initHiveConf();
-    wh = new Warehouse(conf);
+    wh = Warehouse.create(conf);
     SessionState.start(conf);
     setClient(Hive.get(conf).getMSC());
     getClient().dropDatabase(DB_NAME, true, true, true);

ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java

@@ -99,7 +99,7 @@ public void setUp() throws Exception {
     rawStore = new ObjectStore();
     rawStore.setConf(hmsHandler.getConf());
     // Create the 'hive' catalog with new warehouse directory
-    HMSHandler.createDefaultCatalog(rawStore, new Warehouse(conf));
+    HMSHandler.createDefaultCatalog(rawStore, Warehouse.create(conf));
     try {
       DropDataConnectorRequest dropDcReq = new DropDataConnectorRequest(dcName);
       dropDcReq.setIfNotExists(true);

standalone-metastore/metastore-common/spotbugs/spotbugs-exclude.xml

@@ -21,4 +21,16 @@
     <Match>
         <Class name="~org.apache.hadoop.hive.metastore.api.*" />
     </Match>
+    <Match>
+        <Class name="org.apache.hadoop.hive.common.metrics.metrics2.JsonFileMetricsReporter"/>
+        <Bug pattern="EI_EXPOSE_REP2"/>
+    </Match>
+    <Match>
+        <Class name="org.apache.hadoop.hive.metastore.security.TUGIAssumingTransport"/>
+        <Bug pattern="EI_EXPOSE_REP2"/>
+    </Match>
+    <Match>
+        <Class name="org.apache.hadoop.hive.metastore.security.TFilterTransport"/>
+        <Bug pattern="EI_EXPOSE_REP2"/>
+    </Match>
 </FindBugsFilter>