storage: Add tests for variant-file-delete --force. #TASK-6737

opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/manager/operations/VariantDeleteOperationManager.java

@@ -27,13 +27,17 @@
 import org.opencb.opencga.storage.core.metadata.models.TaskMetadata;
 import org.opencb.opencga.storage.core.variant.VariantStorageEngine;
 import org.opencb.opencga.storage.core.variant.VariantStorageOptions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
 
 public class VariantDeleteOperationManager extends OperationManager {
 
+    private final Logger logger = LoggerFactory.getLogger(VariantDeleteOperationManager.class);
+
     public VariantDeleteOperationManager(VariantStorageManager variantStorageManager, VariantStorageEngine engine) {
         super(variantStorageManager, engine);
     }
@@ -65,6 +69,12 @@ public void removeFile(String study, List<String> inputFiles, URI outdir, String
                     // Might be partially loaded in VariantStorage. Check FileMetadata
                     FileMetadata fileMetadata = variantStorageEngine.getMetadataManager()
                             .getFileMetadata(studyMetadata.getId(), file.getName());
+                    if (fileMetadata != null && !fileMetadata.getPath().equals(file.getUri().getPath())) {
+                        // FileMetadata path does not match the catalog path. This file is not registered in the storage.
+                        throw new CatalogException("Unable to remove variants from file '" + file.getPath() + "'. "
+                                + "File is not registered in the storage. "
+                                + "Instead, found file with same name but different path '" + fileMetadata.getPath() + "'");
+                    }
                     boolean canBeRemoved;
                     if (force) {
                         // When forcing remove, just require the file to be registered in the storage
@@ -74,8 +84,9 @@ public void removeFile(String study, List<String> inputFiles, URI outdir, String
                         canBeRemoved = fileMetadata != null && fileMetadata.getIndexStatus() != TaskMetadata.Status.NONE;
                     }
                     if (!canBeRemoved) {
-                        throw new CatalogException("Unable to remove variants from file " + file.getName() + ". "
-                                + "IndexStatus = " + catalogIndexStatus);
+                        throw new CatalogException("Unable to remove variants from file '" + file.getPath() + "'. "
+                                + "IndexStatus = " + catalogIndexStatus + "."
+                                + (fileMetadata == null ? " File not found in storage." : ""));
                     }
                 }
                 fileNames.add(file.getName());

opencga-analysis/src/test/java/org/opencb/opencga/analysis/variant/manager/operations/AbstractVariantOperationManagerTest.java

@@ -197,7 +197,11 @@ protected File getSmallFile() throws IOException, CatalogException {
     }
 
     protected File create(String resourceName) throws IOException, CatalogException {
-        return create(studyId, getResourceUri(resourceName));
+        return create(resourceName, "data/vcfs/");
+    }
+
+    protected File create(String resourceName, String path) throws IOException, CatalogException {
+        return create(studyId, getResourceUri(resourceName), path);
     }
 
     protected File create(String studyId, URI uri) throws IOException, CatalogException {

opencga-analysis/src/test/java/org/opencb/opencga/analysis/variant/manager/operations/RemoveVariantsTest.java

@@ -32,7 +32,9 @@
 import org.opencb.opencga.core.models.sample.Sample;
 import org.opencb.opencga.core.models.study.Study;
 import org.opencb.opencga.core.testclassification.duration.MediumTests;
+import org.opencb.opencga.storage.core.variant.VariantStorageOptions;
 
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
@@ -44,6 +46,7 @@
 import static org.hamcrest.CoreMatchers.anyOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.*;
+import static org.opencb.opencga.storage.core.variant.VariantStorageBaseTest.getResourceUri;
 
 /**
  * Created on 10/07/17.
@@ -83,6 +86,42 @@ public void testLoadAndRemoveOneWithOtherLoaded() throws Exception {
         testLoadAndRemoveOne();
     }
 
+    @Test
+    public void testLoadAndRemoveForce() throws Exception {
+        File file77 = create("platinum/1K.end.platinum-genomes-vcf-NA12877_S1.genome.vcf.gz");
+        File file78 = create("platinum/1K.end.platinum-genomes-vcf-NA12878_S1.genome.vcf.gz");
+        File file79 = create("platinum/1K.end.platinum-genomes-vcf-NA12879_S1.genome.vcf.gz");
+        indexFile(file77, new QueryOptions(), outputId);
+        indexFile(file78, new QueryOptions(), outputId);
+
+        removeFile(file77, new QueryOptions());
+
+        try {
+            removeFile(file77, new QueryOptions());
+        } catch (Exception e) {
+            assertTrue(e.getMessage(), e.getMessage().contains("Unable to remove variants from file"));
+        }
+        removeFile(file77, new QueryOptions(VariantStorageOptions.FORCE.key(), true));
+
+        try {
+            removeFile(file79, new QueryOptions(VariantStorageOptions.FORCE.key(), true));
+        } catch (Exception e) {
+            assertTrue(e.getMessage(), e.getMessage().contains("File not found in storage."));
+        }
+        Path file77Path = Paths.get(file77.getUri());
+        Path otherDir = file77Path.getParent().resolve("other_dir");
+        Files.createDirectory(otherDir);
+        Path otherFile = Files.copy(file77Path, otherDir.resolve(file77Path.getFileName()));
+        File file77_2 = create(studyFqn, otherFile.toUri(), "other_dir");
+
+        try {
+            removeFile(studyFqn, Collections.singletonList(file77_2.getPath()), new QueryOptions(VariantStorageOptions.FORCE.key(), true));
+        } catch (Exception e) {
+            assertTrue(e.getMessage(), e.getMessage().contains("Unable to remove variants from file"));
+            assertTrue(e.getMessage(), e.getMessage().contains("Instead, found file with same name but different path"));
+        }
+    }
+
 
     @Test
     public void testLoadAndRemoveMany() throws Exception {
@@ -127,8 +166,13 @@ private void removeFile(List<File> files, QueryOptions options) throws Exception
         Study study = catalogManager.getFileManager().getStudy(ORGANIZATION, files.get(0), sessionId);
         String studyId = study.getFqn();
 
+        removeFile(studyId, fileIds, options);
+    }
+
+    private void removeFile(String studyId, List<String> fileIds, QueryOptions options) throws Exception {
+
         Path outdir = Paths.get(opencga.createTmpOutdir(studyId, "_REMOVE_", sessionId));
-        variantManager.removeFile(studyId, fileIds, new QueryOptions(), outdir.toUri(), sessionId);
+        variantManager.removeFile(studyId, fileIds, new QueryOptions(options), outdir.toUri(), sessionId);
 //        assertEquals(files.size(), removedFiles.size());
 
         Cohort all = catalogManager.getCohortManager().search(studyId, new Query(CohortDBAdaptor.QueryParams.ID.key(),

opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java

@@ -23,6 +23,7 @@
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
+import java.util.concurrent.Callable;
 import java.util.stream.Collectors;
 
 
@@ -149,7 +150,7 @@ private CellBaseConfiguration validate(boolean autoComplete) throws IOException
         String inputVersion = getVersion();
         CellBaseDataResponse<SpeciesProperties> species;
         try {
-            species = cellBaseClient.getMetaClient().species();
+            species = retryMetaSpecies();
         } catch (RuntimeException e) {
             throw new IllegalArgumentException("Unable to access cellbase url '" + getURL() + "', version '" + inputVersion + "'", e);
         }
@@ -158,7 +159,7 @@ private CellBaseConfiguration validate(boolean autoComplete) throws IOException
                 // Version might be missing the starting "v"
                 cellBaseConfiguration.setVersion("v" + cellBaseConfiguration.getVersion());
                 cellBaseClient = newCellBaseClient(cellBaseConfiguration, getSpecies(), getAssembly());
-                species = cellBaseClient.getMetaClient().species();
+                species = retryMetaSpecies();
             }
         }
         if (species == null || species.firstResult() == null) {
@@ -308,7 +309,7 @@ private static String majorMinor(String version) {
     public String getVersionFromServer() throws IOException {
         if (serverVersion == null) {
             synchronized (this) {
-                ObjectMap result = retryMetaAbout(3);
+                ObjectMap result = retryMetaAbout();
                 if (result == null) {
                     throw new IOException("Unable to get version from server for cellbase " + toString());
                 }
@@ -322,12 +323,43 @@ public String getVersionFromServer() throws IOException {
         return serverVersion;
     }
 
-    private ObjectMap retryMetaAbout(int retries) throws IOException {
-        ObjectMap result = cellBaseClient.getMetaClient().about().firstResult();
-        if (result == null && retries > 0) {
-            // Retry
-            logger.warn("Unable to get version from server for cellbase " + toString() + ". Retrying...");
-            result = retryMetaAbout(retries - 1);
+    private ObjectMap retryMetaAbout() throws IOException {
+        return retry(3, () -> cellBaseClient.getMetaClient().about().firstResult());
+    }
+
+    private CellBaseDataResponse<SpeciesProperties> retryMetaSpecies() throws IOException {
+        return retry(3, () -> cellBaseClient.getMetaClient().species());
+    }
+
+    private <T> T retry(int retries, Callable<T> function) throws IOException {
+        if (retries <= 0) {
+            return null;
+        }
+        T result = null;
+        Exception e = null;
+        try {
+            result = function.call();
+        } catch (Exception e1) {
+            e = e1;
+        }
+        if (result == null) {
+            try {
+                // Retry
+                logger.warn("Unable to get reach cellbase " + toString() + ". Retrying...");
+                result = retry(retries - 1, function);
+            } catch (Exception e1) {
+                if (e == null) {
+                    e = e1;
+                } else {
+                    e.addSuppressed(e1);
+                }
+                if (e instanceof IOException) {
+                    throw (IOException) e;
+                } else {
+                    throw new IOException("Error reading from cellbase " + toString(), e);
+                }
+            }
+
         }
         return result;
     }