From 563ae4d7d3ffd24c811046421000645a7b011f52 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sun, 12 May 2019 20:13:57 +0200 Subject: [PATCH] Raise ObjectNotFound if file does not exist in object store And return an empty string if a path does not exist when calling `get_filename` or` get_extra_files_path` on a Dataset. --- lib/galaxy/model/__init__.py | 18 ++++++++++++------ lib/galaxy/objectstore/__init__.py | 5 ++++- test/unit/jobs/test_job_wrapper.py | 3 +++ test/unit/test_model_store.py | 1 + test/unit/tools/test_actions.py | 3 +++ .../tools/test_collect_primary_datasets.py | 3 +++ 6 files changed, 26 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index faebe5498d2..f0a3b589ee3 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -42,6 +42,7 @@ from sqlalchemy.orm import ( ) from sqlalchemy.schema import UniqueConstraint +import galaxy.exceptions import galaxy.model.metadata import galaxy.model.orm.now import galaxy.model.tags @@ -2156,8 +2157,10 @@ class Dataset(StorableObject, RepresentById): def get_file_name(self): if not self.external_filename: assert self.object_store is not None, "Object Store has not been initialized for dataset %s" % self.id - filename = self.object_store.get_filename(self) - return filename + if self.object_store.exists(self): + return self.object_store.get_filename(self) + else: + return '' else: filename = self.external_filename # Make filename absolute @@ -2175,7 +2178,9 @@ class Dataset(StorableObject, RepresentById): # actual database column so if SA instantiates this object - the # attribute won't exist yet. if not getattr(self, "external_extra_files_path", None): - return self.object_store.get_filename(self, dir_only=True, extra_dir=self._extra_files_rel_path) + if self.object_store.exists(self, dir_only=True, extra_dir=self._extra_files_rel_path): + return self.object_store.get_filename(self, dir_only=True, extra_dir=self._extra_files_rel_path) + return '' else: return os.path.abspath(self.external_extra_files_path) @@ -2266,11 +2271,12 @@ class Dataset(StorableObject, RepresentById): def full_delete(self): """Remove the file and extra files, marks deleted and purged""" # os.unlink( self.file_name ) - self.object_store.delete(self) + try: + self.object_store.delete(self) + except galaxy.exceptions.ObjectNotFound: + pass if self.object_store.exists(self, extra_dir=self._extra_files_rel_path, dir_only=True): self.object_store.delete(self, entire_dir=True, extra_dir=self._extra_files_rel_path, dir_only=True) - # if os.path.exists( self.extra_files_path ): - # shutil.rmtree( self.extra_files_path ) # TODO: purge metadata files self.deleted = True self.purged = True diff --git a/lib/galaxy/objectstore/__init__.py b/lib/galaxy/objectstore/__init__.py index b73a4a8ad56..6294ff77832 100644 --- a/lib/galaxy/objectstore/__init__.py +++ b/lib/galaxy/objectstore/__init__.py @@ -467,7 +467,10 @@ class DiskObjectStore(ObjectStore): # construct and return hashed path if os.path.exists(path): return path - return self._construct_path(obj, **kwargs) + path = self._construct_path(obj, **kwargs) + if not os.path.exists(path): + raise ObjectNotFound + return path def update_from_file(self, obj, file_name=None, create=False, **kwargs): """`create` parameter is not used in this implementation.""" diff --git a/test/unit/jobs/test_job_wrapper.py b/test/unit/jobs/test_job_wrapper.py index 5e370587116..50389f204ef 100644 --- a/test/unit/jobs/test_job_wrapper.py +++ b/test/unit/jobs/test_job_wrapper.py @@ -191,6 +191,9 @@ class MockObjectStore(object): def create(self, *args, **kwds): pass + def exists(self, *args, **kwargs): + return True + def get_filename(self, *args, **kwds): if kwds.get("base_dir", "") == "job_work": return self.working_directory diff --git a/test/unit/test_model_store.py b/test/unit/test_model_store.py index 8614658bfe7..ec2cb414c99 100644 --- a/test/unit/test_model_store.py +++ b/test/unit/test_model_store.py @@ -283,6 +283,7 @@ def test_import_export_composite_datasets(): h = model.History(name="Test History", user=u) d1 = _create_datasets(sa_session, h, 1, extension="html")[0] + app.object_store.create(d1.dataset, dir_only=True, extra_dir=d1.dataset._extra_files_rel_path) sa_session.add_all((h, d1)) sa_session.flush() diff --git a/test/unit/tools/test_actions.py b/test/unit/tools/test_actions.py index 3c23507ab6b..8628cd21205 100644 --- a/test/unit/tools/test_actions.py +++ b/test/unit/tools/test_actions.py @@ -274,6 +274,9 @@ class MockObjectStore(object): self.first_create = True self.object_store_id = "mycoolid" + def exists(self, *args, **kwargs): + return True + def create(self, dataset): self.created_datasets.append(dataset) if self.first_create: diff --git a/test/unit/tools/test_collect_primary_datasets.py b/test/unit/tools/test_collect_primary_datasets.py index 0f1814680b3..b926502e03f 100644 --- a/test/unit/tools/test_collect_primary_datasets.py +++ b/test/unit/tools/test_collect_primary_datasets.py @@ -408,6 +408,9 @@ class MockObjectStore(object): path = self.created_datasets[dataset] return os.stat(path).st_size + def exists(self, *args, **kwargs): + return True + def get_filename(self, dataset): return self.created_datasets[dataset]