Merge 98c2c73 into 9a8e636

.gitignore

@@ -47,10 +47,6 @@ MANIFEST
 .\#*
 \#*
 
-# Hive
-derby.log
-metastore_db/
-
 # Ignore GPG files
 **/*.gpg
 **/*.key

CHANGELOG.md

@@ -1,5 +1,22 @@
-## [3.4.0] - TBD
+## [4.0.0] - TBD
+### Fixed
+- Intermediate temporary folders now cleaned up as part of test lifecycle.
+- `HiveMetaStoreCore` performs a null check before attempting to close its metastore client.
+
 ### Changed
+- Hive version updated to `2.3.8` (was `2.3.7`).
+- `hive-exec` dependency changed to use `core` classifier to allow source code retrieval for easier debugging.
+- Hive's `METASTORE_CONNECTION_POOLING_TYPE` is now set to `"NONE"`.
+- Hive's `HIVESTATSAUTOGATHER` is now set to `false`.
+- Hive's `HIVE_SERVER2_LOGGING_OPERATION_ENABLED` is now set to `false`.
+- Various Derby, scratch and similar folders now configured to use a base temporary folder.
+- JUnit4 Rules now extend `TestWatcher` instead of `ExternalResource` to allow cleanup of temporary folders on test failures.
+- `BeejuCore` `tempDir()` returns base temporary test folder instead of temporary Hive warehouse dir.
+- `BeejuCore` returns temporary Hive warehouse dir via `warehouseDir()`.
+- JUnit5 Extensions `getTempDirectory()` returns base temporary test folder instead of temporary Hive warehouse dir.
+- JUnit5 Extensions return temporary Hive warehouse dir via `getWarehouseDirectory()`.
+- JUnit4 Rules `tempDir()` returns base temporary test folder instead of temporary Hive warehouse dir.
+- JUnit4 rules return temporary Hive warehouse dir via `warehouseDir()`.
 - `hotels-oss-parent` version updated to `6.1.0` (was `5.0.0`) to enable building with Java 11.
 
 ## [3.3.0] - 2020-10-23

pom.xml

@@ -1,5 +1,6 @@
 
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
   <modelVersion>4.0.0</modelVersion>
 
   <parent>
@@ -9,7 +10,7 @@
   </parent>
 
   <artifactId>beeju</artifactId>
-  <version>3.4.0-SNAPSHOT</version>
+  <version>4.0.0-SNAPSHOT</version>
 
   <scm>
     <connection>scm:git:https://${GIT_USERNAME}:${GIT_PASSWORD}@github.com/HotelsDotCom/beeju.git</connection>
@@ -20,13 +21,19 @@
 
   <properties>
     <hadoop.version>2.7.2</hadoop.version>
-    <hive.version>2.3.7</hive.version>
+    <hive.version>2.3.8</hive.version>
     <jdk.version>1.8</jdk.version>
     <junit.jupiter.version>5.7.0</junit.jupiter.version>
     <junit.platform.version>1.3.2</junit.platform.version>
   </properties>
 
   <dependencies>
+    <!-- kryo needed for hive-exec with "core" classifier -->
+    <dependency>
+      <groupId>com.esotericsoftware</groupId>
+      <artifactId>kryo-shaded</artifactId>
+      <version>3.0.3</version>
+    </dependency>
     <dependency>
       <groupId>org.apache.hadoop</groupId>
       <artifactId>hadoop-common</artifactId>
@@ -52,6 +59,7 @@
       <groupId>org.apache.hive</groupId>
       <artifactId>hive-exec</artifactId>
       <version>${hive.version}</version>
+      <classifier>core</classifier>
       <exclusions>
         <exclusion>
           <groupId>org.apache.logging.log4j</groupId>
@@ -130,6 +138,7 @@
           <configuration>
             <!-- excluding files that don't need a header update -->
             <excludes>
+              <exclude>src/main/java/com/hotels/beeju/core/BeejuCore.java</exclude>
               <exclude>src/main/java/com/hotels/beeju/hiveserver2/RelaxedSQLStdHiveAuthorizerFactory.java</exclude>
             </excludes>
           </configuration>
@@ -139,15 +148,15 @@
 
     <plugins>
       <!-- below mess is needed in order to run both Junit4 and Junit5 tests, and is based on approach documented in
-      
-         https://medium.com/@nieldw/running-junit5-and-junit4-tests-in-the-same-project-with-maven-122c8aa4f3bc
-          
-         JUnit claim that Surefire plugin 2.22.2 can be used to do this:
-         
-         https://junit.org/junit5/docs/current/user-guide/#running-tests-build-maven
-         
-         But the approach documented there doesn't work. We should revisit this when 
-         newer versions of Surefire and JUnit are released.
+
+        https://medium.com/@nieldw/running-junit5-and-junit4-tests-in-the-same-project-with-maven-122c8aa4f3bc
+
+        JUnit claim that Surefire plugin 2.22.2 can be used to do this:
+
+        https://junit.org/junit5/docs/current/user-guide/#running-tests-build-maven
+
+        But the approach documented there doesn't work. We should revisit this when
+        newer versions of Surefire and JUnit are released.
       -->
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>

src/main/java/com/hotels/beeju/BeejuJUnitRule.java

@@ -1,5 +1,5 @@
 /**
- * Copyright (C) 2015-2020 Expedia, Inc.
+ * Copyright (C) 2015-2021 Expedia, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,21 +16,20 @@
 package com.hotels.beeju;
 
 import java.io.File;
-import java.io.IOException;
-import java.io.UncheckedIOException;
 import java.util.Map;
 
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.metastore.HiveMetaStoreClient;
 import org.apache.thrift.TException;
-import org.junit.rules.ExternalResource;
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
 
 import com.hotels.beeju.core.BeejuCore;
 
 /**
  * Base class for BeeJU JUnit Rules that require a Hive Metastore database configuration pre-set.
  */
-abstract class BeejuJUnitRule extends ExternalResource {
+abstract class BeejuJUnitRule extends TestWatcher {
 
   protected BeejuCore core;
 
@@ -46,17 +45,17 @@ public BeejuJUnitRule(
   }
 
   @Override
-  protected void before() throws Throwable {
-    createDatabase(databaseName());
+  public void starting(Description description) {
+    try {
+      createDatabase(databaseName());
+    } catch (TException e) {
+      throw new RuntimeException("Error starting rule", e);
+    }
   }
 
   @Override
-  protected void after() {
-    try {
-      core.cleanUp();
-    } catch (IOException e) {
-      throw new UncheckedIOException(e);
-    }
+  public void finished(Description description) {
+    core.cleanUp();
   }
 
   /**
@@ -100,6 +99,13 @@ public HiveMetaStoreClient newClient() {
   File tempDir() {
     return core.tempDir().toFile();
   }
+
+  /**
+   * @return Root of warehouse directory
+   */
+  File warehouseDir() {
+    return core.warehouseDir().toFile();
+  }
 
   /**
    * Create a new database with the specified name.

src/main/java/com/hotels/beeju/HiveMetaStoreJUnitRule.java

@@ -1,5 +1,5 @@
 /**
- * Copyright (C) 2015-2020 Expedia, Inc.
+ * Copyright (C) 2015-2021 Expedia, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,8 +16,12 @@
 package com.hotels.beeju;
 
 import java.util.Map;
+import java.util.concurrent.ExecutionException;
 
 import org.apache.hadoop.hive.metastore.HiveMetaStoreClient;
+import org.junit.runner.Description;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.hotels.beeju.core.HiveMetaStoreCore;
 
@@ -27,6 +31,8 @@
  * A fresh database instance will be created for each test method.
  */
 public class HiveMetaStoreJUnitRule extends BeejuJUnitRule {
+
+  private static final Logger log = LoggerFactory.getLogger(HiveMetaStoreJUnitRule.class);
 
   private final HiveMetaStoreCore hiveMetaStoreCore = new HiveMetaStoreCore(core);
 
@@ -50,17 +56,19 @@ public HiveMetaStoreJUnitRule(String databaseName) {
    * Create a Hive Metastore with a pre-created database using the provided name and configuration.
    *
    * @param databaseName Database name.
-   * @param preConfiguration Hive configuration properties that will be set prior to BeeJU potentially overriding these with its defaults.
+   * @param preConfiguration Hive configuration properties that will be set prior to BeeJU potentially overriding these
+   *          with its defaults.
    */
   public HiveMetaStoreJUnitRule(String databaseName, Map<String, String> preConfiguration) {
     super(databaseName, preConfiguration);
   }
 
   /**
    * Create a Hive Metastore with a pre-created database using the provided name and configuration.
-   *    
+   * 
    * @param databaseName Database name.
-   * @param preConfiguration Hive configuration properties that will be set prior to BeeJU potentially overriding these with its defaults.
+   * @param preConfiguration Hive configuration properties that will be set prior to BeeJU potentially overriding these
+   *          with its defaults.
    * @param postConfiguration Hive configuration properties that will be set to override BeeJU's defaults.
    */
   public HiveMetaStoreJUnitRule(
@@ -71,15 +79,23 @@ public HiveMetaStoreJUnitRule(
   }
 
   @Override
-  protected void before() throws Throwable {
-    super.before();
-    hiveMetaStoreCore.initialise();
+  public void starting(Description description) {
+    super.starting(description);
+    try {
+      hiveMetaStoreCore.initialise();
+    } catch (InterruptedException | ExecutionException e) {
+      throw new RuntimeException("Error initialising metastore core", e);
+    }
   }
 
   @Override
-  protected void after() {
-    hiveMetaStoreCore.shutdown();
-    super.after();
+  public void finished(Description description) {
+    try {
+      hiveMetaStoreCore.shutdown();
+    } catch (Throwable t) {
+      log.warn("Error shutting down metastore core", t);
+    }
+    super.finished(description);
   }
 
   /**

src/main/java/com/hotels/beeju/HiveServer2JUnitRule.java

@@ -1,5 +1,5 @@
 /**
- * Copyright (C) 2015-2020 Expedia, Inc.
+ * Copyright (C) 2015-2021 Expedia, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,9 +15,12 @@
  */
 package com.hotels.beeju;
 
+import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.util.Map;
 
 import org.apache.hive.jdbc.HiveDriver;
+import org.junit.runner.Description;
 
 import com.hotels.beeju.core.HiveServer2Core;
 
@@ -63,16 +66,27 @@ public HiveServer2JUnitRule(String databaseName, Map<String, String> configurati
   }
 
   @Override
-  protected void before() throws Throwable {
-    hiveServer2Core.startServerSocket();
-    super.before();
-    hiveServer2Core.initialise();
+  public void starting(Description description) {
+    try {
+      hiveServer2Core.startServerSocket();
+    } catch (IOException e) {
+      throw new UncheckedIOException("Error starting HiveServer2 server socket", e);
+    }
+    super.starting(description);
+    try {
+      hiveServer2Core.initialise();
+    } catch (InterruptedException e) {
+      throw new RuntimeException("Error initialising HiveServer2 core", e);
+    }
   }
 
   @Override
-  protected void after() {
-    hiveServer2Core.shutdown();
-    super.after();
+  public void finished(Description description) {
+    try {
+      hiveServer2Core.shutdown();
+    } finally {
+      super.finished(description);
+    }
   }
 
   /**
@@ -91,5 +105,4 @@ public String connectionURL() {
     return hiveServer2Core.getJdbcConnectionUrl();
   }
 
-
 }

src/main/java/com/hotels/beeju/ThriftHiveMetaStoreApp.java

@@ -1,5 +1,5 @@
 /**
- * Copyright (C) 2015-2020 Expedia, Inc.
+ * Copyright (C) 2015-2021 Expedia, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,7 +22,7 @@ public class ThriftHiveMetaStoreApp {
   public static void main(String[] args) throws Throwable {
     ThriftHiveMetaStoreJUnitRule rule = new ThriftHiveMetaStoreJUnitRule();
     rule.setThriftPort(22334);
-    rule.before();
+    rule.starting(null);
     System.out.println("BeeJU Thrift Hive Metastore listening on: " + rule.getThriftConnectionUri());
     CountDownLatch latch = new CountDownLatch(1);
     latch.await();