fix style issues and add proper docs

cpp/build-support/lint_cpp_cli.py

@@ -77,6 +77,7 @@ EXCLUSIONS = _paths('''\
     arrow/visitor_inline.h
     gandiva/cache.h
     gandiva/jni
+    jni/
     test
     internal''')
 

cpp/src/jni/orc/concurrent_map.h

@@ -19,34 +19,38 @@
 #define JNI_ID_TO_MODULE_MAP_H
 
 #include <memory>
+#include <mutex>
 #include <unordered_map>
 #include <utility>
 
 #include "arrow/util/macros.h"
 
 namespace arrow {
+namespace jni {
 
-template <typename HOLDER>
+/**
+ * An utility class that map module id to module pointers.
+ * @tparam Holder class of the object to hold.
+ */
+template <typename Holder>
 class concurrentMap {
  public:
   concurrentMap() : module_id_(kInitModuleId) {}
 
-  jlong Insert(HOLDER holder) {
-    mtx_.lock();
+  jlong Insert(Holder holder) {
+    std::lock_guard<std::mutex> lock(mtx_);
     jlong result = module_id_++;
-    map_.insert(std::pair<jlong, HOLDER>(result, holder));
-    mtx_.unlock();
+    map_.insert(std::pair<jlong, Holder>(result, holder));
     return result;
   }
 
   void Erase(jlong module_id) {
-    mtx_.lock();
+    std::lock_guard<std::mutex> lock(mtx_);
     map_.erase(module_id);
-    mtx_.unlock();
   }
 
-  HOLDER Lookup(jlong module_id) {
-    HOLDER result = NULLPTR;
+  Holder Lookup(jlong module_id) {
+    Holder result = NULLPTR;
     try {
       result = map_.at(module_id);
     } catch (const std::out_of_range& e) {
@@ -54,30 +58,30 @@ class concurrentMap {
     if (result != NULLPTR) {
       return result;
     }
-    mtx_.lock();
+    std::lock_guard<std::mutex> lock(mtx_);
     try {
       result = map_.at(module_id);
     } catch (const std::out_of_range& e) {
     }
-    mtx_.unlock();
     return result;
   }
 
   void Clear() {
-    mtx_.lock();
+    std::lock_guard<std::mutex> lock(mtx_);
     map_.clear();
-    mtx_.unlock();
   }
 
  private:
-  static const int kInitModuleId = 4;
+  // starting value of the module_id.
+  static constexpr int kInitModuleId = 4;
 
   int64_t module_id_;
   std::mutex mtx_;
   // map from module ids returned to Java and module pointers
-  std::unordered_map<jlong, HOLDER> map_;
+  std::unordered_map<jlong, Holder> map_;
 };
 
+}  // namespace jni
 }  // namespace arrow
 
 #endif  // JNI_ID_TO_MODULE_MAP_H

cpp/src/jni/orc/jni_wrapper.cpp

@@ -48,9 +48,11 @@ static jmethodID record_batch_constructor;
 
 static jint JNI_VERSION = JNI_VERSION_1_6;
 
-static arrow::concurrentMap<std::shared_ptr<arrow::Buffer>> buffer_holder_;
-static arrow::concurrentMap<std::shared_ptr<RecordBatchReader>> orc_stripe_reader_holder_;
-static arrow::concurrentMap<std::shared_ptr<ORCFileReader>> orc_reader_holder_;
+using arrow::jni::concurrentMap;
+
+static concurrentMap<std::shared_ptr<arrow::Buffer>> buffer_holder_;
+static concurrentMap<std::shared_ptr<RecordBatchReader>> orc_stripe_reader_holder_;
+static concurrentMap<std::shared_ptr<ORCFileReader>> orc_reader_holder_;
 
 jclass CreateGlobalClassReference(JNIEnv* env, const char* class_name) {
   jclass local_class = env->FindClass(class_name);
@@ -70,6 +72,19 @@ jmethodID GetMethodID(JNIEnv* env, jclass this_class, const char* name, const ch
   return ret;
 }
 
+std::string JStringToCString(JNIEnv* env, jstring string) {
+  int32_t jlen, clen;
+  clen = env->GetStringUTFLength(string);
+  jlen = env->GetStringLength(string);
+  std::vector<char> buffer(clen);
+  env->GetStringUTFRegion(string, 0, jlen, buffer.data());
+  return std::string(buffer.data(), clen);
+}
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 jint JNI_OnLoad(JavaVM* vm, void* reserved) {
   JNIEnv* env;
   if (vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION) != JNI_OK) {
@@ -113,27 +128,6 @@ void JNI_OnUnload(JavaVM* vm, void* reserved) {
   orc_reader_holder_.Clear();
 }
 
-std::shared_ptr<ORCFileReader> GetNativeReader(jlong id) {
-  return orc_reader_holder_.Lookup(id);
-}
-
-std::shared_ptr<RecordBatchReader> GetStripeReader(jlong id) {
-  return orc_stripe_reader_holder_.Lookup(id);
-}
-
-std::string JStringToCString(JNIEnv* env, jstring string) {
-  int32_t jlen, clen;
-  clen = env->GetStringUTFLength(string);
-  jlen = env->GetStringLength(string);
-  std::vector<char> buffer(clen);
-  env->GetStringUTFRegion(string, 0, jlen, buffer.data());
-  return std::string(buffer.data(), clen);
-}
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 JNIEXPORT jlong JNICALL Java_org_apache_arrow_adapter_orc_OrcReaderJniWrapper_open(
     JNIEnv* env, jobject this_obj, jstring file_path) {
   std::shared_ptr<arrow::io::ReadableFile> in_file;
@@ -188,7 +182,7 @@ Java_org_apache_arrow_adapter_orc_OrcReaderJniWrapper_nextStripeReader(JNIEnv* e
                                                                        jobject this_obj,
                                                                        jlong id,
                                                                        jlong batch_size) {
-  auto reader = GetNativeReader(id);
+  auto reader = orc_reader_holder_.Lookup(id);
   std::shared_ptr<RecordBatchReader> stripe_reader;
   auto status = reader->NextStripeReader(batch_size, &stripe_reader);
 
@@ -207,7 +201,7 @@ JNIEXPORT jbyteArray JNICALL
 Java_org_apache_arrow_adapter_orc_OrcStripeReaderJniWrapper_getSchema(JNIEnv* env,
                                                                       jclass this_cls,
                                                                       jlong id) {
-  auto stripe_reader = GetStripeReader(id);
+  auto stripe_reader = orc_stripe_reader_holder_.Lookup(id);
   auto schema = stripe_reader->schema();
 
   std::shared_ptr<arrow::Buffer> out;
@@ -227,7 +221,7 @@ JNIEXPORT jobject JNICALL
 Java_org_apache_arrow_adapter_orc_OrcStripeReaderJniWrapper_next(JNIEnv* env,
                                                                  jclass this_cls,
                                                                  jlong id) {
-  auto stripe_reader = GetStripeReader(id);
+  auto stripe_reader = orc_stripe_reader_holder_.Lookup(id);
   std::shared_ptr<arrow::RecordBatch> record_batch;
   auto status = stripe_reader->ReadNext(&record_batch);
   if (!status.ok() || !record_batch) {

java/adapter/orc/src/main/java/org/apache/arrow/adapter/orc/OrcJniUtils.java

@@ -18,10 +18,10 @@
 package org.apache.arrow.adapter.orc;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.Files;
-import java.nio.file.InvalidPathException;
 import java.nio.file.StandardCopyOption;
 import java.util.UUID;
 
@@ -32,6 +32,8 @@ class OrcJniUtils {
   private static final String LIBRARY_NAME = "arrow_orc_jni";
   private static boolean isLoaded = false;
 
+  private OrcJniUtils() {}
+
   /**
    * Load arrow orc jni library from jar.
    * @throws IOException throws IOException in case target library can not be found.
@@ -55,7 +57,7 @@ class OrcJniUtils {
     try (final InputStream is = OrcReaderJniWrapper.class.getClassLoader()
             .getResourceAsStream(libraryToLoad)) {
       if (is == null) {
-        throw new InvalidPathException(libraryToLoad,  "file was not found inside JAR.");
+        throw new FileNotFoundException(libraryToLoad);
       } else {
         Files.copy(is, temp.toPath(), StandardCopyOption.REPLACE_EXISTING);
       }
@@ -66,15 +68,6 @@ class OrcJniUtils {
   private static File setupFile(String tmpDir, String libraryToLoad)
           throws IOException {
     final String randomizeFileName = libraryToLoad + UUID.randomUUID();
-    final File temp = new File(tmpDir, randomizeFileName);
-    if (temp.exists() && !temp.delete()) {
-      throw new InvalidPathException(
-              temp.getAbsolutePath(), "File already exists and cannot be removed.");
-    }
-    if (!temp.createNewFile()) {
-      throw new InvalidPathException(temp.getAbsolutePath(), "File could not be created.");
-    }
-    temp.deleteOnExit();
-    return temp;
+    return File.createTempFile(tmpDir, randomizeFileName);
   }
 }

java/adapter/orc/src/main/java/org/apache/arrow/adapter/orc/OrcMemoryJniWrapper.java

@@ -44,14 +44,26 @@ class OrcMemoryJniWrapper implements AutoCloseable {
     this.capacity = capacity;
   }
 
+  /**
+   * Return the size of underlying chunk of memory that has valid data.
+   * @return
+   */
   long getSize() {
     return size;
   }
 
+  /**
+   * Return the size of underlying chunk of memory managed by this OrcMemoryJniWrapper.
+   * @return
+   */
   long getCapacity() {
     return capacity;
   }
 
+  /**
+   * Return the memory address of underlying chunk of memory.
+   * @return
+   */
   long getMemoryAddress() {
     return memoryAddress;
   }