Fixed PR comments

atlanhq · Jun 17, 2024 · 7b9ae98 · 7b9ae98
1 parent a365a1c
commit 7b9ae98
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 12 deletions.
diff --git a/repository/src/main/java/org/apache/atlas/repository/impexp/MigrationProgressService.java b/repository/src/main/java/org/apache/atlas/repository/impexp/MigrationProgressService.java
@@ -22,6 +22,7 @@
 import org.apache.atlas.ApplicationProperties;
 import org.apache.atlas.AtlasException;
 import org.apache.atlas.annotation.AtlasService;
+import org.apache.atlas.exception.AtlasBaseException;
 import org.apache.atlas.model.impexp.MigrationStatus;
 import org.apache.atlas.repository.graph.AtlasGraphProvider;
 import org.apache.atlas.repository.graphdb.GraphDBMigrator;
@@ -55,15 +56,15 @@ public class MigrationProgressService {
     private boolean zipFileBasedMigrationImport;
 
     @Inject
-    public MigrationProgressService(Configuration configuration, GraphDBMigrator migrator) {
+    public MigrationProgressService(Configuration configuration, GraphDBMigrator migrator) throws AtlasBaseException {
         this.migrator      = migrator;
         this.cacheValidity = (configuration != null) ? configuration.getLong(MIGRATION_QUERY_CACHE_TTL, DEFAULT_CACHE_TTL_IN_SECS) : DEFAULT_CACHE_TTL_IN_SECS;
 
         this.zipFileBasedMigrationImport = isZipFileBasedMigrationEnabled();
         initConditionallyZipFileBasedMigrator();
     }
 
-    private void initConditionallyZipFileBasedMigrator() {
+    private void initConditionallyZipFileBasedMigrator() throws AtlasBaseException {
         if (!zipFileBasedMigrationImport) {
             return;
         }

diff --git a/...itory/src/main/java/org/apache/atlas/repository/migration/DataMigrationStatusService.java b/...itory/src/main/java/org/apache/atlas/repository/migration/DataMigrationStatusService.java
@@ -18,6 +18,7 @@
 
 package org.apache.atlas.repository.migration;
 
+import org.apache.atlas.AtlasErrorCode;
 import org.apache.atlas.exception.AtlasBaseException;
 import org.apache.atlas.model.migration.MigrationImportStatus;
 import org.apache.atlas.repository.Constants;
@@ -57,16 +58,14 @@ public DataMigrationStatusService(AtlasGraph graph) {
     }
 
 
-    public void init(String fileToImport) {
+    public void init(String fileToImport) throws AtlasBaseException {
         try {
             if(!validateFilePath(fileToImport)){
-                throw new AtlasBaseException("File Path is invalid");
+                throw new AtlasBaseException(AtlasErrorCode.INVALID_PARAMETERS, "File Path is invalid");
             }
             this.status = new MigrationImportStatus(fileToImport, DigestUtils.md5Hex(new FileInputStream(fileToImport)));
         } catch (IOException e) {
             LOG.error("Not able to create Migration status", e);
-        } catch (AtlasBaseException e) {
-            LOG.error("File Path is invalid");
         }
 
         if (!this.migrationStatusVertexManagement.exists(status.getFileHash())) {
@@ -77,17 +76,15 @@ public void init(String fileToImport) {
     }
 
 
-    public MigrationImportStatus getCreate(String fileName) {
+    public MigrationImportStatus getCreate(String fileName) throws AtlasBaseException {
         MigrationImportStatus create = null;
         try {
             if(!validateFilePath(fileName)){
-                throw new AtlasBaseException("File Path is invalid");
+                throw new AtlasBaseException(AtlasErrorCode.INVALID_PARAMETERS, "File Path is invalid");
             }
             create = getCreate(new MigrationImportStatus(fileName, DigestUtils.md5Hex(new FileInputStream(fileName))));
         } catch (IOException e) {
             LOG.error("Exception occurred while creating migration import", e);
-        } catch (AtlasBaseException e) {
-            LOG.error("File Path is invalid");
         }
 
         return create;

diff --git a/.../src/main/java/org/apache/atlas/repository/store/graph/v2/bulkimport/MigrationImport.java b/.../src/main/java/org/apache/atlas/repository/store/graph/v2/bulkimport/MigrationImport.java
@@ -86,7 +86,7 @@ public EntityMutationResponse run(EntityImportStream entityStream, AtlasImportRe
         return ret;
     }
 
-    private DataMigrationStatusService createMigrationStatusService(AtlasImportResult importResult) {
+    private DataMigrationStatusService createMigrationStatusService(AtlasImportResult importResult) throws AtlasBaseException {
         DataMigrationStatusService dataMigrationStatusService = new DataMigrationStatusService();
         dataMigrationStatusService.init(importResult.getRequest().getOptions().get(AtlasImportRequest.OPTION_KEY_MIGRATION_FILE_NAME));
         return dataMigrationStatusService;

diff --git a/...ory/src/test/java/org/apache/atlas/repository/migration/MigrationProgressServiceTest.java b/...ory/src/test/java/org/apache/atlas/repository/migration/MigrationProgressServiceTest.java
@@ -17,6 +17,7 @@
  */
 package org.apache.atlas.repository.migration;
 
+import org.apache.atlas.exception.AtlasBaseException;
 import org.apache.atlas.model.impexp.MigrationStatus;
 import org.apache.atlas.repository.graphdb.*;
 import org.apache.atlas.repository.graphdb.janus.migration.ReaderStatusManager;
@@ -84,7 +85,11 @@ public void cachedStatusReturnedIfQueriedBeforeCacheExpiration() {
     }
 
     private MigrationProgressService getMigrationStatusForTest(Configuration cfg, TinkerGraph tg) {
-        return new MigrationProgressService(cfg, createMigrator(tg));
+        try {
+            return new MigrationProgressService(cfg, createMigrator(tg));
+        } catch (AtlasBaseException e) {
+            throw new RuntimeException(e);
+        }
     }
 
     @Test

diff --git a/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java b/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java
@@ -15,6 +15,7 @@ public void testValidateFilePath() {
                 {"/var/app/allowed/./file.txt", false, "Should return false for a path with relative current directory notation."},
                 {"/Users/username/repos/repo0/.\\file.txt", false, "Should return false for a path with mixed slash types potentially bypassing checks."},
                 {"tmp/file.txt", false, "Should return false for non-absolute paths."},
+                {"", false, "Should return false for empty paths"},
                 {"/var/app/allowed/..\\file.txt", false, "Should return false for paths with unusual characters aiming to navigate directories."},
                 {"/Users/username/repos/repo0/%2e%2e/notallowed/file.txt", false, "Should return false for paths with URL-encoded traversal sequences."},
                 {"/var/app/allowed/\0file.txt", false, "Should return false for paths that cause exceptions, like those containing null bytes."}