*** WIP ***

code for updating spec files
some cleanup

tests pass, but we don't have all the tests we need.

crc/api.yml

@@ -1130,7 +1130,7 @@ paths:
       - name: file_id
         in: path
         required: true
-        description: The id of the file
+        description: The id of the spec file
         schema:
           type: integer
     get:

crc/api/file.py

@@ -164,7 +164,7 @@ def update_spec_file_data(file_id):
                        message=f'The workflow spec for id {file_model.workflow_spec_id} does not exist.')
 
     file = connexion.request.files['file']
-    file_model = SpecFileService.update_workflow_spec_file(workflow_spec_model, file_model, file.stream.read(), file.content_type)
+    SpecFileService().update_spec_file_data(workflow_spec_model, file_model.name, file.stream.read())
     return FileSchema().dump(to_file_api(file_model))
 
 
@@ -258,38 +258,23 @@ def update_file_info(file_id, body):
 
 
 def update_spec_file_info(file_id, body):
-    # schema = update_file_info(file_id, body)
     if file_id is None:
         raise ApiError('no_such_file', 'Please provide a valid File ID.')
     file_model = session.query(FileModel).filter(FileModel.id==file_id).first()
     if file_model is None:
         raise ApiError('unknown_file_model', 'The file_model "' + file_id + '" is not recognized.')
 
-    new_file_model = FileModelSchema().load(body, session=session)
-    session.add(new_file_model)
-    session.commit()
-    workflow_spec_model = session.query(WorkflowSpecModel).filter(WorkflowSpecModel.id==file_model.workflow_spec_id).first()
-    category_name = SpecFileService.get_spec_file_category_name(workflow_spec_model)
-    if category_name is not None:
-        sync_file_root = SpecFileService.get_sync_file_root()
-        file_path = os.path.join(sync_file_root,
-                                 category_name,
-                                 workflow_spec_model.display_name,
-                                 file_model.name)
-        SpecFileService.write_file_info_to_system(file_path, file_model)
+    new_file_model = SpecFileService().update_spec_file_info(file_model, body)
     return FileSchema().dump(to_file_api(new_file_model))
 
 
 def delete_file(file_id):
-    workflow_spec_id = session.query(FileModel.workflow_spec_id).filter(FileModel.id==file_id).scalar()
-    if workflow_spec_id is not None:
-        SpecFileService.delete_spec_file(file_id)
-    else:
-        FileService.delete_file(file_id)
+    FileService.delete_file(file_id)
 
 
 def delete_spec_file(file_id):
-    pass
+    SpecFileService.delete_spec_file(file_id)
+    print('crc.api.file: delete_spec_file')
 
 
 # TODO: Finish this

crc/services/spec_file_service.py

@@ -18,6 +18,19 @@ from uuid import UUID
 
 class SpecFileService(object):
 
+    """We store spec files on the file system. This allows us to take advantage of Git for
+       syncing and versioning.
+
+        We keep a record in the File table, but do not have a record in the FileData table.
+
+        For syncing purposes, we keep a copy of the File table info in a json file
+
+        This means there are 3 pieces we have to maintain; File table record, file on the file system,
+        and json file on the file system.
+
+        The files are stored in a directory whose path is determined by the category and spec names.
+    """
+
     #
     # Shared Methods
     #
@@ -27,6 +40,19 @@ class SpecFileService(object):
         app_root = app.root_path
         return os.path.join(app_root, '..', 'tests', dir_name)
 
+    @staticmethod
+    def get_path_from_spec_file_model(spec_file_model):
+        workflow_spec_model = session.query(WorkflowSpecModel).filter(
+            WorkflowSpecModel.id == spec_file_model.workflow_spec_id).first()
+        category_name = SpecFileService.get_spec_file_category_name(workflow_spec_model)
+        if category_name is not None:
+            sync_file_root = SpecFileService.get_sync_file_root()
+            file_path = os.path.join(sync_file_root,
+                                     category_name,
+                                     workflow_spec_model.display_name,
+                                     spec_file_model.name)
+            return file_path
+
     @staticmethod
     def write_file_data_to_system(file_path, file_data):
         os.makedirs(os.path.dirname(file_path), exist_ok=True)
@@ -66,10 +92,19 @@ class SpecFileService(object):
                 is_status=is_status,
             )
 
-        return SpecFileService.update_workflow_spec_file(workflow_spec, file_model, binary_data, content_type)
+        file_model = SpecFileService.update_workflow_spec_file_model(workflow_spec, file_model, binary_data, content_type)
+        file_path = SpecFileService().write_spec_file_data_to_system(workflow_spec, file_model.name, binary_data)
+        SpecFileService().write_spec_file_info_to_system(file_path, file_model)
+
+        return file_model
+
+    def update_workflow_spec_file(self, workflow_spec_model, file_model, file_data, content_type):
+        self.update_workflow_spec_file_model(workflow_spec_model, file_model, file_data, content_type)
+        self.update_spec_file_data(workflow_spec_model, file_model.name, file_data)
+        self.update_spec_file_info()
 
     @staticmethod
-    def update_workflow_spec_file(workflow_spec: WorkflowSpecModel, file_model: FileModel, binary_data, content_type):
+    def update_workflow_spec_file_model(workflow_spec: WorkflowSpecModel, file_model: FileModel, binary_data, content_type):
         # Verify the extension
         file_extension = FileService.get_extension(file_model.name)
         if file_extension not in FileType._member_names_:
@@ -93,12 +128,45 @@ class SpecFileService(object):
         session.add(file_model)
         session.commit()
 
-        # Write file to filesystem
-        SpecFileService().write_spec_file_to_system(workflow_spec, file_model, binary_data)
         return file_model
 
+    @staticmethod
+    def update_spec_file_data(workflow_spec, file_name, binary_data):
+        file_path = SpecFileService().write_spec_file_data_to_system(workflow_spec, file_name, binary_data)
+        return file_path
+
+    def update_spec_file_info(self, old_file_model, body):
+
+        file_data = self.get_spec_file_data(old_file_model.id)
+
+        old_file_path = self.get_path_from_spec_file_model(old_file_model)
+        self.delete_spec_file_data(old_file_path)
+        self.delete_spec_file_info(old_file_path)
+
+        new_file_model = FileModelSchema().load(body, session=session)
+        new_file_path = self.get_path_from_spec_file_model(new_file_model)
+        self.write_file_data_to_system(new_file_path, file_data.data)
+        self.write_file_info_to_system(new_file_path, new_file_model)
+        print('update_spec_file_info')
+        return new_file_model
+
+    @staticmethod
+    def delete_spec_file_data(file_path):
+        os.remove(file_path)
+
+    @staticmethod
+    def delete_spec_file_info(file_path):
+        json_file_path = f'{file_path}.json'
+        os.remove(json_file_path)
+
+    # Placeholder. Not sure if we need this.
+    # Might do this work in delete_spec_file
+    def delete_spec_file_model(self):
+        pass
+
     @staticmethod
     def delete_spec_file(file_id):
+        """This should remove the record in the file table, and both files on the filesystem."""
         sync_file_root = SpecFileService.get_sync_file_root()
         file_model = session.query(FileModel).filter(FileModel.id==file_id).first()
         workflow_spec_id = file_model.workflow_spec_id
@@ -116,7 +184,6 @@ class SpecFileService(object):
         try:
             os.remove(file_path)
             os.remove(json_file_path)
-            # os.rmdir(spec_directory_path)
             session.delete(file_model)
             session.commit()
         except IntegrityError as ie:
@@ -126,7 +193,7 @@ class SpecFileService(object):
             session.commit()
             app.logger.info("Failed to delete file, so archiving it instead. %i, due to %s" % (file_id, str(ie)))
 
-    def write_spec_file_data_to_system(self, workflow_spec_model, file_model, file_data):
+    def write_spec_file_data_to_system(self, workflow_spec_model, file_name, file_data):
         if workflow_spec_model is not None:
             category_name = self.get_spec_file_category_name(workflow_spec_model)
             if category_name is not None:
@@ -134,7 +201,7 @@ class SpecFileService(object):
                 file_path = os.path.join(sync_file_root,
                                          category_name,
                                          workflow_spec_model.display_name,
-                                         file_model.name)
+                                         file_name)
                 self.write_file_data_to_system(file_path, file_data)
                 return file_path
 

tests/base_test.py

@@ -262,7 +262,7 @@ class BaseTest(unittest.TestCase):
         workflow_spec_model = session.query(WorkflowSpecModel).filter(WorkflowSpecModel.id==file_model.workflow_spec_id).first()
         noise, file_extension = os.path.splitext(file_path)
         content_type = CONTENT_TYPES[file_extension[1:]]
-        SpecFileService.update_workflow_spec_file(workflow_spec_model, file_model, data, content_type)
+        SpecFileService().update_spec_file_data(workflow_spec_model, file_model.name, data)
 
     def create_user(self, uid="dhf8r", email="daniel.h.funk@gmail.com", display_name="Hoopy Frood"):
         user = session.query(UserModel).filter(UserModel.uid == uid).first()

tests/files/test_files_api.py

@@ -14,6 +14,8 @@ from crc.models.data_store import DataStoreModel
 from crc.services.document_service import DocumentService
 from example_data import ExampleDataLoader
 
+from sqlalchemy import column
+
 
 class TestFilesApi(BaseTest):
 
@@ -166,7 +168,6 @@ class TestFilesApi(BaseTest):
         self.assertFalse(file.primary)
         self.assertEqual(True, file.is_reference)
 
-
     def test_list_reference_files(self):
         ExampleDataLoader.clean_db()
 
@@ -190,16 +191,26 @@ class TestFilesApi(BaseTest):
 
     def test_update_file_info(self):
         self.load_example_data()
-        file: FileModel = session.query(FileModel).filter(FileModel.is_reference==False).first()
-        file.name = "silly_new_name.bpmn"
+        file: FileModel = session.query(FileModel).filter(column('workflow_spec_id').isnot(None)).first()
+        file_model = FileModel(id=file.id,
+                               name="silly_new_name.bpmn",
+                               type=file.type,
+                               content_type=file.content_type,
+                               is_reference=file.is_reference,
+                               primary=file.primary,
+                               primary_process_id=file.primary_process_id,
+                               workflow_id=file.workflow_id,
+                               workflow_spec_id=file.workflow_spec_id,
+                               archived=file.archived)
+        # file.name = "silly_new_name.bpmn"
 
         rv = self.app.put('/v1.0/spec_file/%i' % file.id,
                           content_type="application/json",
-                          data=json.dumps(FileModelSchema().dump(file)), headers=self.logged_in_headers())
+                          data=json.dumps(FileModelSchema().dump(file_model)), headers=self.logged_in_headers())
         self.assert_success(rv)
         db_file = session.query(FileModel).filter_by(id=file.id).first()
         self.assertIsNotNone(db_file)
-        self.assertEqual(file.name, db_file.name)
+        self.assertEqual("silly_new_name.bpmn", db_file.name)
 
     def test_load_valid_url_for_files(self):
         self.load_example_data()
@@ -222,24 +233,31 @@ class TestFilesApi(BaseTest):
         file_json_1 = json.loads(rv_1.get_data(as_text=True))
         self.assertEqual(80, file_json_1['size'])
 
-        data['file'] = io.BytesIO(self.minimal_bpmn("efghijk")), 'my_new_file.bpmn'
-        rv_2 = self.app.put('/v1.0/spec_file/%i/data' % file_json_1['id'], data=data, follow_redirects=True,
-                            content_type='multipart/form-data', headers=self.logged_in_headers())
+        file_id = file_json_1['id']
+        rv_2 = self.app.get('/v1.0/spec_file/%i/data' % file_id, headers=self.logged_in_headers())
         self.assert_success(rv_2)
-        self.assertIsNotNone(rv_2.get_data())
-        file_json_2 = json.loads(rv_2.get_data(as_text=True))
-        self.assertEqual(FileType.bpmn.value, file_json_2['type'])
-        self.assertEqual("application/octet-stream", file_json_2['content_type'])
-        self.assertEqual(spec.id, file_json_2['workflow_spec_id'])
+        rv_data_2 = rv_2.get_data()
+        self.assertIsNotNone(rv_data_2)
+        self.assertEqual(self.minimal_bpmn("abcdef"), rv_data_2)
+
+        data['file'] = io.BytesIO(self.minimal_bpmn("efghijk")), 'my_new_file.bpmn'
+        rv_3 = self.app.put('/v1.0/spec_file/%i/data' % file_id, data=data, follow_redirects=True,
+                            content_type='multipart/form-data', headers=self.logged_in_headers())
+        self.assert_success(rv_3)
+        self.assertIsNotNone(rv_3.get_data())
+        file_json_3 = json.loads(rv_3.get_data(as_text=True))
+        self.assertEqual(FileType.bpmn.value, file_json_3['type'])
+        self.assertEqual("application/octet-stream", file_json_3['content_type'])
+        self.assertEqual(spec.id, file_json_3['workflow_spec_id'])
 
         # Assure it is updated in the database and properly persisted.
-        file_model = session.query(FileModel).filter(FileModel.id == file_json_2['id']).first()
+        file_model = session.query(FileModel).filter(FileModel.id == file_id).first()
         file_data = SpecFileService().get_spec_file_data(file_model.id)
         self.assertEqual(81, len(file_data.data))
 
-        rv_3 = self.app.get('/v1.0/spec_file/%i/data' % file_json_2['id'], headers=self.logged_in_headers())
-        self.assert_success(rv_3)
-        data = rv_3.get_data()
+        rv_4 = self.app.get('/v1.0/spec_file/%i/data' % file_id, headers=self.logged_in_headers())
+        self.assert_success(rv_4)
+        data = rv_4.get_data()
         self.assertIsNotNone(data)
         self.assertEqual(self.minimal_bpmn("efghijk"), data)
 

tests/test_lookup_service.py

@@ -8,6 +8,7 @@ from crc import session, app
 from crc.models.file import FileDataModel, FileModel, LookupFileModel, LookupDataModel, CONTENT_TYPES
 from crc.models.workflow import WorkflowSpecModel
 from crc.services.lookup_service import LookupService
+from crc.services.reference_file_service import ReferenceFileService
 from crc.services.spec_file_service import SpecFileService
 from crc.services.workflow_processor import WorkflowProcessor
 
@@ -53,9 +54,9 @@ class TestLookupService(BaseTest):
         file = open(file_path, 'rb')
         if file_model.workflow_spec_id is not None:
             workflow_spec_model = session.query(WorkflowSpecModel).filter(WorkflowSpecModel.id==file_model.workflow_spec_id).first()
-            SpecFileService.update_workflow_spec_file(workflow_spec_model, file_model, file.read(), CONTENT_TYPES['xlsx'])
+            SpecFileService().update_spec_file_data(workflow_spec_model, file_model.name, file.read())
         elif file_model.is_reference:
-            SpecFileService().update_reference_file(file_model, file.read())
+            ReferenceFileService().update_reference_file(file_model, file.read())
         else:
             FileService.update_file(file_model, file.read(), CONTENT_TYPES['xlsx'])
         file.close()