Refactoring datasource changes to a new module. (opensearch-project#1504

) * Reafactor into new datasources module Signed-off-by: vamsi-amazon <reddyvam@amazon.com> * Refactored all the datasource releated code to new datasources module Signed-off-by: vamsi-amazon <reddyvam@amazon.com> * More unit tests Signed-off-by: vamsi-amazon <reddyvam@amazon.com> --------- Signed-off-by: vamsi-amazon <reddyvam@amazon.com>
Bit-Quill · Apr 10, 2023 · 7584f79 · 7584f79
1 parent e805151
commit 7584f79
Show file tree

Hide file tree

Showing 56 changed files with 1,625 additions and 494 deletions.
diff --git a/common/build.gradle b/common/build.gradle
@@ -36,7 +36,6 @@ dependencies {
     api group: 'com.google.guava', name: 'guava', version: '31.0.1-jre'
     api group: 'org.apache.logging.log4j', name: 'log4j-core', version:'2.17.1'
     api group: 'org.apache.commons', name: 'commons-lang3', version: '3.12.0'
-    api 'com.amazonaws:aws-encryption-sdk-java:2.4.0'
 
     testImplementation group: 'junit', name: 'junit', version: '4.13.2'
     testImplementation group: 'org.assertj', name: 'assertj-core', version: '3.9.1'

diff --git a/config/checkstyle/google_checks.xml b/config/checkstyle/google_checks.xml
@@ -39,8 +39,9 @@
     <property name="max" value="100"/>
     <property name="ignorePattern" value="^package.*|^import.*|a href|href|http://|https://|ftp://"/>
   </module>
-
+  <module name="SuppressWarningsFilter" />
   <module name="TreeWalker">
+    <module name="SuppressWarningsHolder" />
     <module name="OuterTypeFilename"/>
     <module name="IllegalTokenText">
       <property name="tokens" value="STRING_LITERAL, CHAR_LITERAL"/>

diff --git a/core/src/main/java/org/opensearch/sql/datasource/model/DataSourceMetadata.java b/core/src/main/java/org/opensearch/sql/datasource/model/DataSourceMetadata.java
@@ -12,16 +12,13 @@
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.collect.ImmutableMap;
-import com.google.gson.Gson;
-import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import lombok.AllArgsConstructor;
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
-import lombok.RequiredArgsConstructor;
 import lombok.Setter;
 import org.opensearch.sql.datasource.DataSourceService;
 

diff --git a/core/src/main/java/org/opensearch/sql/storage/DataSourceFactory.java b/core/src/main/java/org/opensearch/sql/storage/DataSourceFactory.java
@@ -7,7 +7,6 @@
 
 package org.opensearch.sql.storage;
 
-import java.util.Map;
 import org.opensearch.sql.datasource.DataSourceService;
 import org.opensearch.sql.datasource.model.DataSource;
 import org.opensearch.sql.datasource.model.DataSourceMetadata;

diff --git a/datasources/build.gradle b/datasources/build.gradle
@@ -0,0 +1,82 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+plugins {
+    id 'java-library'
+    id "io.freefair.lombok"
+    id 'jacoco'
+}
+
+repositories {
+    mavenCentral()
+}
+
+dependencies {
+    implementation project(':core')
+    implementation project(':protocol')
+    implementation group: 'org.opensearch', name: 'opensearch', version: "${opensearch_version}"
+    implementation group: 'org.opensearch', name: 'opensearch-x-content', version: "${opensearch_version}"
+    implementation group: 'org.opensearch', name: 'common-utils', version: "${opensearch_build}"
+    implementation group: 'commons-io', name: 'commons-io', version: '2.8.0'
+    implementation 'com.amazonaws:aws-encryption-sdk-java:2.4.0'
+
+    testImplementation group: 'junit', name: 'junit', version: '4.13.2'
+    testImplementation('org.junit.jupiter:junit-jupiter:5.6.2')
+    testImplementation group: 'net.bytebuddy', name: 'byte-buddy-agent', version: '1.12.13'
+    testImplementation group: 'org.hamcrest', name: 'hamcrest-library', version: '2.1'
+    testImplementation group: 'org.mockito', name: 'mockito-core', version: '5.2.0'
+    testImplementation group: 'org.mockito', name: 'mockito-junit-jupiter', version: '5.2.0'
+    testImplementation 'org.junit.jupiter:junit-jupiter:5.6.2'
+}
+
+test {
+    useJUnitPlatform()
+    testLogging {
+        events "passed", "skipped", "failed"
+        exceptionFormat "full"
+    }
+}
+
+jacocoTestReport {
+    reports {
+        html.enabled true
+        xml.enabled true
+    }
+    afterEvaluate {
+        classDirectories.setFrom(files(classDirectories.files.collect {
+            fileTree(dir: it)
+        }))
+    }
+}
+test.finalizedBy(project.tasks.jacocoTestReport)
+
+jacocoTestCoverageVerification {
+    violationRules {
+        rule {
+            element = 'CLASS'
+            excludes = [
+                'org.opensearch.sql.datasources.settings.DataSourceSettings',
+                'org.opensearch.sql.datasources.exceptions.*',
+                'org.opensearch.sql.datasources.model.*',
+                'org.opensearch.sql.datasources.rest.*'
+            ]
+            limit {
+                counter = 'LINE'
+                minimum = 1.0
+            }
+            limit {
+                counter = 'BRANCH'
+                minimum = 0.9
+            }
+        }
+    }
+    afterEvaluate {
+        classDirectories.setFrom(files(classDirectories.files.collect {
+            fileTree(dir: it)
+        }))
+    }
+}
+check.dependsOn jacocoTestCoverageVerification
+jacocoTestCoverageVerification.dependsOn jacocoTestReport
diff --git a/datasources/lombok.config b/datasources/lombok.config
@@ -0,0 +1,3 @@
+# This file is generated by the 'io.freefair.lombok' Gradle plugin
+config.stopBubbling = true
+lombok.addLombokGeneratedAnnotation = true
diff --git a/...source/model/auth/AuthenticationType.java → .../datasources/auth/AuthenticationType.java b/...source/model/auth/AuthenticationType.java → .../datasources/auth/AuthenticationType.java
@@ -5,7 +5,7 @@
  *
  */
 
-package org.opensearch.sql.datasource.model.auth;
+package org.opensearch.sql.datasources.auth;
 
 import java.util.Collections;
 import java.util.HashMap;

diff --git a/...ce/DataSourceUserAuthorizationHelper.java → ...th/DataSourceUserAuthorizationHelper.java b/...ce/DataSourceUserAuthorizationHelper.java → ...th/DataSourceUserAuthorizationHelper.java
@@ -1,6 +1,10 @@
-package org.opensearch.sql.datasource;
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.sql.datasources.auth;
 
-import java.util.List;
 import org.opensearch.sql.datasource.model.DataSourceMetadata;
 
 /**

diff --git a/...ataSourceUserAuthorizationHelperImpl.java → ...ataSourceUserAuthorizationHelperImpl.java b/...ataSourceUserAuthorizationHelperImpl.java → ...ataSourceUserAuthorizationHelperImpl.java
@@ -3,16 +3,15 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package org.opensearch.sql.plugin.datasource;
+package org.opensearch.sql.datasources.auth;
 
-import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT;
 import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME;
 
 import java.util.List;
 import lombok.AllArgsConstructor;
 import org.opensearch.client.Client;
+import org.opensearch.commons.ConfigConstants;
 import org.opensearch.commons.authuser.User;
-import org.opensearch.sql.datasource.DataSourceUserAuthorizationHelper;
 import org.opensearch.sql.datasource.model.DataSourceMetadata;
 
 @AllArgsConstructor
@@ -21,13 +20,15 @@ public class DataSourceUserAuthorizationHelperImpl implements DataSourceUserAuth
 
   private Boolean isAuthorizationRequired() {
     String userString = client.threadPool()
-        .getThreadContext().getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT);
+        .getThreadContext().getTransient(
+            ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT);
     return userString != null;
   }
 
   private List<String> getUserRoles() {
     String userString = client.threadPool()
-        .getThreadContext().getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT);
+        .getThreadContext().getTransient(
+            ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT);
     User user = User.parse(userString);
     return user.getRoles();
   }

diff --git a/...earch/sql/common/encryptor/Encryptor.java → .../sql/datasources/encryptor/Encryptor.java b/...earch/sql/common/encryptor/Encryptor.java → .../sql/datasources/encryptor/Encryptor.java
@@ -5,7 +5,7 @@
  *
  */
 
-package org.opensearch.sql.common.encryptor;
+package org.opensearch.sql.datasources.encryptor;
 
 public interface Encryptor {
 

diff --git a/...h/sql/common/encryptor/EncryptorImpl.java → .../datasources/encryptor/EncryptorImpl.java b/...h/sql/common/encryptor/EncryptorImpl.java → .../datasources/encryptor/EncryptorImpl.java
@@ -5,7 +5,7 @@
  *
  */
 
-package org.opensearch.sql.common.encryptor;
+package org.opensearch.sql.datasources.encryptor;
 
 import com.amazonaws.encryptionsdk.AwsCrypto;
 import com.amazonaws.encryptionsdk.CommitmentPolicy;
@@ -29,8 +29,8 @@ public String encrypt(String plainText) {
         .build();
 
     JceMasterKey jceMasterKey
-        = JceMasterKey.getInstance(new SecretKeySpec(masterKey.getBytes(), "AES"), "Custom", "",
-        "AES/GCM/NoPadding");
+        = JceMasterKey.getInstance(new SecretKeySpec(masterKey.getBytes(), "AES"), "Custom",
+        "opensearch.config.master.key", "AES/GCM/NoPadding");
 
     final CryptoResult<byte[], JceMasterKey> encryptResult = crypto.encryptData(jceMasterKey,
         plainText.getBytes(StandardCharsets.UTF_8));
@@ -44,8 +44,8 @@ public String decrypt(String encryptedText) {
         .build();
 
     JceMasterKey jceMasterKey
-        = JceMasterKey.getInstance(new SecretKeySpec(masterKey.getBytes(), "AES"), "Custom", "",
-        "AES/GCM/NoPadding");
+        = JceMasterKey.getInstance(new SecretKeySpec(masterKey.getBytes(), "AES"), "Custom",
+        "opensearch.config.master.key", "AES/GCM/NoPadding");
 
     final CryptoResult<byte[], JceMasterKey> decryptedResult
         = crypto.decryptData(jceMasterKey, Base64.getDecoder().decode(encryptedText));

diff --git a/...ceptions/DataSourceNotFoundException.java → ...ceptions/DataSourceNotFoundException.java b/...ceptions/DataSourceNotFoundException.java → ...ceptions/DataSourceNotFoundException.java
@@ -5,7 +5,7 @@
  *
  */
 
-package org.opensearch.sql.datasource.exceptions;
+package org.opensearch.sql.datasources.exceptions;
 
 /**
  * DataSourceNotFoundException.

diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/exceptions/ErrorMessage.java b/datasources/src/main/java/org/opensearch/sql/datasources/exceptions/ErrorMessage.java
@@ -0,0 +1,78 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+
+package org.opensearch.sql.datasources.exceptions;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonObject;
+import lombok.Getter;
+import org.opensearch.rest.RestStatus;
+
+/**
+ * Error Message.
+ */
+public class ErrorMessage {
+
+  protected Throwable exception;
+
+  private final int status;
+
+  @Getter
+  private final String type;
+
+  @Getter
+  private final String reason;
+
+  @Getter
+  private final String details;
+
+  /**
+   * Error Message Constructor.
+   */
+  public ErrorMessage(Throwable exception, int status) {
+    this.exception = exception;
+    this.status = status;
+
+    this.type = fetchType();
+    this.reason = fetchReason();
+    this.details = fetchDetails();
+  }
+
+  private String fetchType() {
+    return exception.getClass().getSimpleName();
+  }
+
+  protected String fetchReason() {
+    return status == RestStatus.BAD_REQUEST.getStatus()
+        ? "Invalid Request"
+        : "There was internal problem at backend";
+  }
+
+  protected String fetchDetails() {
+    // Some exception prints internal information (full class name) which is security concern
+    return emptyStringIfNull(exception.getLocalizedMessage());
+  }
+
+  private String emptyStringIfNull(String str) {
+    return str != null ? str : "";
+  }
+
+  @Override
+  public String toString() {
+    JsonObject jsonObject = new JsonObject();
+    jsonObject.addProperty("status", status);
+    jsonObject.add("error", getErrorAsJson());
+    return new Gson().toJson(jsonObject);
+  }
+
+  private JsonObject getErrorAsJson() {
+    JsonObject errorJson = new JsonObject();
+    errorJson.addProperty("type", type);
+    errorJson.addProperty("reason", reason);
+    errorJson.addProperty("details", details);
+    return errorJson;
+  }
+}
diff --git a/.../model/CreateDataSourceActionRequest.java → ...nsport/CreateDataSourceActionRequest.java b/.../model/CreateDataSourceActionRequest.java → ...nsport/CreateDataSourceActionRequest.java
@@ -5,7 +5,7 @@
  *
  */
 
-package org.opensearch.sql.plugin.model;
+package org.opensearch.sql.datasources.model.transport;
 
 
 import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME;

diff --git a/...model/CreateDataSourceActionResponse.java → ...sport/CreateDataSourceActionResponse.java b/...model/CreateDataSourceActionResponse.java → ...sport/CreateDataSourceActionResponse.java
@@ -5,7 +5,7 @@
  *
  */
 
-package org.opensearch.sql.plugin.model;
+package org.opensearch.sql.datasources.model.transport;
 
 import java.io.IOException;
 import lombok.Getter;

diff --git a/.../model/DeleteDataSourceActionRequest.java → ...nsport/DeleteDataSourceActionRequest.java b/.../model/DeleteDataSourceActionRequest.java → ...nsport/DeleteDataSourceActionRequest.java
@@ -5,7 +5,7 @@
  *
  */
 
-package org.opensearch.sql.plugin.model;
+package org.opensearch.sql.datasources.model.transport;
 
 import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME;
 

diff --git a/...model/DeleteDataSourceActionResponse.java → ...sport/DeleteDataSourceActionResponse.java b/...model/DeleteDataSourceActionResponse.java → ...sport/DeleteDataSourceActionResponse.java
@@ -5,7 +5,7 @@
  *
  */
 
-package org.opensearch.sql.plugin.model;
+package org.opensearch.sql.datasources.model.transport;
 
 import java.io.IOException;
 import lombok.Getter;

diff --git a/...gin/model/GetDataSourceActionRequest.java → ...transport/GetDataSourceActionRequest.java b/...gin/model/GetDataSourceActionRequest.java → ...transport/GetDataSourceActionRequest.java
@@ -5,7 +5,7 @@
  *
  */
 
-package org.opensearch.sql.plugin.model;
+package org.opensearch.sql.datasources.model.transport;
 
 import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME;
 

diff --git a/...in/model/GetDataSourceActionResponse.java → ...ransport/GetDataSourceActionResponse.java b/...in/model/GetDataSourceActionResponse.java → ...ransport/GetDataSourceActionResponse.java
@@ -5,7 +5,7 @@
  *
  */
 
-package org.opensearch.sql.plugin.model;
+package org.opensearch.sql.datasources.model.transport;
 
 import java.io.IOException;
 import lombok.Getter;

diff --git a/.../model/UpdateDataSourceActionRequest.java → ...nsport/UpdateDataSourceActionRequest.java b/.../model/UpdateDataSourceActionRequest.java → ...nsport/UpdateDataSourceActionRequest.java
@@ -5,19 +5,17 @@
  *
  */
 
-package org.opensearch.sql.plugin.model;
+package org.opensearch.sql.datasources.model.transport;
 
 
 import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME;
 
 import java.io.IOException;
 import lombok.Getter;
-import org.json.JSONObject;
 import org.opensearch.action.ActionRequest;
 import org.opensearch.action.ActionRequestValidationException;
 import org.opensearch.common.io.stream.StreamInput;
 import org.opensearch.sql.datasource.model.DataSourceMetadata;
-import org.opensearch.sql.protocol.response.format.JsonResponseFormatter;
 
 public class UpdateDataSourceActionRequest
     extends ActionRequest {