code review feedback

Author: cgardens

dataline-api/src/main/openapi/config.yaml

@@ -221,7 +221,7 @@ paths:
     post:
       tags:
       - source_implementation
-      summary: Get source implementations for workspace
+      summary: List source implementations for workspace
       operationId: listSourceImplementationsForWorkspace
       requestBody:
         content:

dataline-config-persistence/src/main/java/io/dataline/config/persistence/ConfigPersistenceImpl.java renamed to dataline-config-persistence/src/main/java/io/dataline/config/persistence/DefaultConfigPersistence.java

@@ -15,28 +15,16 @@
 import org.apache.commons.io.FileUtils;
 
 // we force all interaction with disk storage to be effectively single threaded.
-public class ConfigPersistenceImpl implements ConfigPersistence {
+public class DefaultConfigPersistence implements ConfigPersistence {
   private static final Object lock = new Object();
-  private static final String CONFIG_STORAGE_ROOT = "../data/config/";
-  private static final String TEST_STORAGE_ROOT = "/tmp/data/config/";
   private static final String CONFIG_SCHEMA_ROOT = "../dataline-config/src/main/resources/json/";
 
   private final ObjectMapper objectMapper;
   private final JsonSchemaValidation jsonSchemaValidation;
   private final String storageRoot;
-  private final boolean testEnv;
 
-  public static ConfigPersistenceImpl get() {
-    return new ConfigPersistenceImpl(CONFIG_STORAGE_ROOT, false);
-  }
-
-  public static ConfigPersistenceImpl getTest() {
-    return new ConfigPersistenceImpl(TEST_STORAGE_ROOT, true);
-  }
-
-  private ConfigPersistenceImpl(String storageRoot, boolean testEnv) {
+  public DefaultConfigPersistence(String storageRoot) {
     this.storageRoot = storageRoot;
-    this.testEnv = testEnv;
     jsonSchemaValidation = JsonSchemaValidation.getSingletonInstance();
     objectMapper = new ObjectMapper();
   }
@@ -95,10 +83,6 @@ public <T> void writeConfig(
   }
 
   public void deleteAll() {
-    if (!testEnv) {
-      throw new RuntimeException("deleteAll operation is only allowed in test environment");
-    }
-
     try {
       FileUtils.forceDelete(new File(storageRoot));
     } catch (IOException e) {

dataline-config-persistence/src/main/java/io/dataline/config/persistence/WorkspaceConstants.java renamed to dataline-config-persistence/src/main/java/io/dataline/config/persistence/PersistenceConstants.java

@@ -2,7 +2,9 @@
 
 import java.util.UUID;
 
-public class WorkspaceConstants {
+public class PersistenceConstants {
+  public static String DEFAULT_TEST_ROOT = "/tmp/data/config/";
+
   // for MVP we only support one workspace per deployment and we hard code its id.
   public static UUID DEFAULT_WORKSPACE_ID = UUID.fromString("5ae6b09b-fdec-41af-aaf7-7d94cfc33ef6");
 }

dataline-server/src/main/java/io/dataline/server/apis/ConfigurationApi.java

@@ -2,7 +2,7 @@
 
 import io.dataline.api.model.*;
 import io.dataline.config.persistence.ConfigPersistence;
-import io.dataline.config.persistence.ConfigPersistenceImpl;
+import io.dataline.config.persistence.DefaultConfigPersistence;
 import io.dataline.server.handlers.SourceImplementationsHandler;
 import io.dataline.server.handlers.SourceSpecificationsHandler;
 import io.dataline.server.handlers.SourcesHandler;
@@ -18,7 +18,8 @@ public class ConfigurationApi implements io.dataline.api.V1Api {
   private final SourceImplementationsHandler sourceImplementationsHandler;
 
   public ConfigurationApi() {
-    ConfigPersistence configPersistence = ConfigPersistenceImpl.get();
+    // todo: configure with env variable.
+    ConfigPersistence configPersistence = new DefaultConfigPersistence("../data/config/");
     workspacesHandler = new WorkspacesHandler(configPersistence);
     sourcesHandler = new SourcesHandler(configPersistence);
     sourceSpecificationsHandler = new SourceSpecificationsHandler(configPersistence);

dataline-server/src/main/java/io/dataline/server/errors/KnownExceptionMapper.java

@@ -13,7 +13,7 @@ public class KnownExceptionMapper implements ExceptionMapper<KnownException> {
 
   @Override
   public Response toResponse(KnownException e) {
-    LOGGER.debug("known exception", e);
+    LOGGER.debug("Known exception", e);
     return Response.status(e.getHttpCode())
         .entity(new ObjectMapper().createObjectNode().put("message", e.getMessage()))
         .type("application/json")

dataline-server/src/main/java/io/dataline/server/handlers/WorkspacesHandler.java

@@ -23,7 +23,7 @@ public WorkspaceRead getWorkspace(WorkspaceIdRequestBody workspaceIdRequestBody)
   @SuppressWarnings("unused")
   public WorkspaceRead getWorkspaceBySlug(SlugRequestBody slugRequestBody) {
     // for now we assume there is one workspace and it has a default uuid.
-    return getWorkspaceFromId(WorkspaceConstants.DEFAULT_WORKSPACE_ID);
+    return getWorkspaceFromId(PersistenceConstants.DEFAULT_WORKSPACE_ID);
   }
 
   private WorkspaceRead getWorkspaceFromId(UUID workspaceIdUuid) {

dataline-server/src/test/java/io/dataline/server/handlers/SourceImplementationsHandlerTest.java

@@ -9,8 +9,9 @@
 import io.dataline.api.model.*;
 import io.dataline.config.SourceConnectionImplementation;
 import io.dataline.config.SourceConnectionSpecification;
-import io.dataline.config.persistence.ConfigPersistenceImpl;
+import io.dataline.config.persistence.DefaultConfigPersistence;
 import io.dataline.config.persistence.PersistenceConfigType;
+import io.dataline.config.persistence.PersistenceConstants;
 import io.dataline.server.fixtures.SourceSpecificationFixtures;
 import java.io.File;
 import java.io.IOException;
@@ -20,14 +21,14 @@
 import org.junit.jupiter.api.Test;
 
 class SourceImplementationsHandlerTest {
-  private ConfigPersistenceImpl configPersistence;
+  private DefaultConfigPersistence configPersistence;
   private SourceConnectionSpecification sourceConnectionSpecification;
   private SourceConnectionImplementation sourceConnectionImplementation;
   private SourceImplementationsHandler sourceImplementationsHandler;
 
   @BeforeEach
   void setUp() {
-    configPersistence = ConfigPersistenceImpl.getTest();
+    configPersistence = new DefaultConfigPersistence(PersistenceConstants.DEFAULT_TEST_ROOT);
     sourceConnectionSpecification =
         SourceSpecificationFixtures.createSourceConnectionSpecification(configPersistence);
     sourceConnectionImplementation =

dataline-server/src/test/java/io/dataline/server/handlers/SourceSpecificationsHandlerTest.java

@@ -5,20 +5,21 @@
 import io.dataline.api.model.SourceIdRequestBody;
 import io.dataline.api.model.SourceSpecificationRead;
 import io.dataline.config.SourceConnectionSpecification;
-import io.dataline.config.persistence.ConfigPersistenceImpl;
+import io.dataline.config.persistence.DefaultConfigPersistence;
+import io.dataline.config.persistence.PersistenceConstants;
 import io.dataline.server.fixtures.SourceSpecificationFixtures;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 class SourceSpecificationsHandlerTest {
-  private ConfigPersistenceImpl configPersistence;
+  private DefaultConfigPersistence configPersistence;
   private SourceConnectionSpecification sourceConnectionSpecification;
   private SourceSpecificationsHandler sourceSpecificationHandler;
 
   @BeforeEach
   void setUp() {
-    configPersistence = ConfigPersistenceImpl.getTest();
+    configPersistence = new DefaultConfigPersistence(PersistenceConstants.DEFAULT_TEST_ROOT);
     sourceConnectionSpecification =
         SourceSpecificationFixtures.createSourceConnectionSpecification(configPersistence);
     sourceSpecificationHandler = new SourceSpecificationsHandler(configPersistence);

dataline-server/src/test/java/io/dataline/server/handlers/SourcesHandlerTest.java

@@ -6,23 +6,24 @@
 import io.dataline.api.model.SourceRead;
 import io.dataline.api.model.SourceReadList;
 import io.dataline.config.StandardSource;
-import io.dataline.config.persistence.ConfigPersistenceImpl;
+import io.dataline.config.persistence.DefaultConfigPersistence;
 import io.dataline.config.persistence.PersistenceConfigType;
+import io.dataline.config.persistence.PersistenceConstants;
 import java.util.Optional;
 import java.util.UUID;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 class SourcesHandlerTest {
-  private ConfigPersistenceImpl configPersistence;
+  private DefaultConfigPersistence configPersistence;
   private StandardSource source;
   private SourcesHandler sourceHandler;
 
   @BeforeEach
   void setUp() {
-    configPersistence = ConfigPersistenceImpl.getTest();
-    source = creatSource();
+    configPersistence = new DefaultConfigPersistence(PersistenceConstants.DEFAULT_TEST_ROOT);
+    source = creatSourceMock();
     sourceHandler = new SourcesHandler(configPersistence);
   }
 
@@ -31,7 +32,7 @@ void tearDown() {
     configPersistence.deleteAll();
   }
 
-  private StandardSource creatSource() {
+  private StandardSource creatSourceMock() {
     final UUID sourceId = UUID.randomUUID();
 
     final StandardSource standardSource = new StandardSource();
@@ -46,7 +47,7 @@ private StandardSource creatSource() {
 
   @Test
   void listSources() {
-    final StandardSource source2 = creatSource();
+    final StandardSource source2 = creatSourceMock();
     configPersistence.writeConfig(
         PersistenceConfigType.STANDARD_SOURCE, source2.getSourceId().toString(), source2);
 

dataline-server/src/test/java/io/dataline/server/handlers/WorkspacesHandlerTest.java

@@ -14,19 +14,19 @@
 import org.junit.jupiter.api.Test;
 
 class WorkspacesHandlerTest {
-  private ConfigPersistenceImpl configPersistence;
+  private DefaultConfigPersistence configPersistence;
   private StandardWorkspace workspace;
   private WorkspacesHandler workspacesHandler;
 
   @BeforeEach
   void setUp() {
-    configPersistence = ConfigPersistenceImpl.getTest();
+    configPersistence = new DefaultConfigPersistence(PersistenceConstants.DEFAULT_TEST_ROOT);
     workspace = creatWorkspace();
     workspacesHandler = new WorkspacesHandler(configPersistence);
   }
 
   private StandardWorkspace creatWorkspace() {
-    final UUID workspaceId = WorkspaceConstants.DEFAULT_WORKSPACE_ID;
+    final UUID workspaceId = PersistenceConstants.DEFAULT_WORKSPACE_ID;
 
     final StandardWorkspace standardWorkspace = new StandardWorkspace();
     standardWorkspace.setWorkspaceId(workspaceId);