From 7a03ac8afb8866fccc6ad0263a2ebd7be8407b85 Mon Sep 17 00:00:00 2001
From: Daniel K Lyons <dlyons@nrao.edu>
Date: Fri, 4 Sep 2020 09:58:19 -0600
Subject: [PATCH] Fix type of file content to be bytes, not strings. Move
 database connection into schema.

---
 .../versions/44d5bbbf2615_workspaces_init.py  |  6 +-
 services/workflow/src/workflow/server.py      | 23 +------
 shared/workspaces/src/workspaces/schema.py    | 62 ++++++++++++++++---
 3 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/schema/versions/44d5bbbf2615_workspaces_init.py b/schema/versions/44d5bbbf2615_workspaces_init.py
index 0b3922aa3..c73021e8c 100644
--- a/schema/versions/44d5bbbf2615_workspaces_init.py
+++ b/schema/versions/44d5bbbf2615_workspaces_init.py
@@ -21,7 +21,7 @@ def upgrade():
 
     op.create_table('workflow_templates',
                     sa.Column('filename', sa.String, primary_key=True, comment='the filename of the template'),
-                    sa.Column('content', sa.String, nullable=False, comment='the content of the template'),
+                    sa.Column('content', sa.LargeBinary, nullable=False, comment='the content of the template'),
                     sa.Column('workflow_name', sa.String, sa.ForeignKey('workflows.workflow_name'), primary_key=True,
                               comment='the name of the workflow this template belongs to'),
                     comment='Templates associated with workflows')
@@ -37,7 +37,7 @@ def upgrade():
                     sa.Column('workflow_request_id', sa.Integer, sa.ForeignKey('workflow_requests.workflow_request_id'),
                               comment='the id of the workflow request.'),
                     sa.Column('filename', sa.String, comment='the name of this file', nullable=False),
-                    sa.Column('content', sa.String, comment='the contents of the file', nullable=False),
+                    sa.Column('content', sa.LargeBinary, comment='the contents of the file', nullable=False),
                     comment='A man-to-many mapping table tracking which files were used for workflow requests.')
 
     op.create_unique_constraint('workflow_request_filenames_uniq', 'workflow_request_files',
@@ -45,7 +45,7 @@ def upgrade():
 
     # populate with some initial data
     op.execute("INSERT INTO workflows (workflow_name) VALUES ('null')")
-    op.execute("INSERT INTO workflow_templates (workflow_name, filename, content) VALUES ('null', 'null.sh', 'null {{arguments}}')")
+    op.execute("INSERT INTO workflow_templates (workflow_name, filename, content) VALUES ('null', 'null.sh', E'null {{arguments}}')")
 
 
 def downgrade():
diff --git a/services/workflow/src/workflow/server.py b/services/workflow/src/workflow/server.py
index 72c1ea70c..457015ac0 100644
--- a/services/workflow/src/workflow/server.py
+++ b/services/workflow/src/workflow/server.py
@@ -6,7 +6,7 @@ from pyramid_beaker import session_factory_from_settings
 from pyramid.renderers import JSONP
 from sqlalchemy import create_engine
 from sqlalchemy.orm import sessionmaker
-from workspaces.services import WorkflowInfo, WorkflowService
+from workspaces.services import WorkflowInfo, WorkflowService, get_session_factory, get_engine
 
 """
 Work done:
@@ -154,27 +154,6 @@ class WorkflowFilesRestService:
 # ---------------------------------------------------------
 
 
-def get_engine():
-    """
-    Generate the SQL Alchemy engine for us, using Capo.
-    :return:
-    """
-    capo = CapoConfig().settings('metadataDatabase')
-    url = capo.jdbcUrl.replace('jdbc:', '').replace('://', f'://{capo.jdbcUsername}:{capo.jdbcPassword}@')
-    return create_engine(url)
-
-
-def get_session_factory(engine):
-    """
-    Generate the SQL Alchemy session factory for us, using the supplied engine
-    :param engine:
-    :return:
-    """
-    factory = sessionmaker()
-    factory.configure(bind=engine)
-    return factory
-
-
 def get_tm_session(session_factory, transaction_manager):
     """
     Enable Zope's transaction manager on our session
diff --git a/shared/workspaces/src/workspaces/schema.py b/shared/workspaces/src/workspaces/schema.py
index 41e2f8199..b018f6e9c 100644
--- a/shared/workspaces/src/workspaces/schema.py
+++ b/shared/workspaces/src/workspaces/schema.py
@@ -7,7 +7,9 @@ from pathlib import Path
 from typing import Dict, List
 
 import sqlalchemy as sa
-from sqlalchemy.orm import relationship
+from pycapo import CapoConfig
+from sqlalchemy import create_engine
+from sqlalchemy.orm import relationship, sessionmaker
 from sqlalchemy.ext.declarative import declarative_base
 
 
@@ -16,15 +18,22 @@ class AbstractFile:
     Abstract file is exactly that, an abstract concept of what a file is, to be
     returned from various non-filesystem places.
     """
-    def __init__(self, filename, content):
+    filename: str
+    content: bytes
+
+    def __init__(self, filename: str, content: bytes):
         self.filename, self.content = filename, content
 
     def write_to(self, directory: Path):
-        raise NotImplementedError('Actually save the file into the directory passed')
+        (directory / self.filename).write_bytes(self.content)
 
     def __json__(self, request):
         return {'filename': self.filename, 'content': self.content}
 
+    @classmethod
+    def from_path(cls, path: Path) -> 'AbstractFile':
+        return cls(path.name, path.read_bytes())
+
 
 Base = declarative_base()
 
@@ -42,6 +51,9 @@ class Workflow(Base):
     def __json__(self, request):
         return {'workflow_name': self.workflow_name, 'templates': self.templates}
 
+    def __repr__(self):
+        return f'<Workflow workflow_name={self.workflow_name}>'
+
     def render_templates(self, argument: Dict, files: List[Path]) -> List[AbstractFile]:
         """
         Render the templates associated with this workflow
@@ -49,7 +61,7 @@ class Workflow(Base):
         :param files:    the files to be processed
         :return:         a list of rendered templates
         """
-        raise NotImplementedError(f'{self.__class__.__name__}.{inspect.stack()[0][3]}')
+        raise NotImplementedError
 
 
 class WorkflowTemplate(Base):
@@ -58,7 +70,7 @@ class WorkflowTemplate(Base):
     """
     __tablename__ = 'workflow_templates'
     filename = sa.Column('filename', sa.String, primary_key=True)
-    content = sa.Column('content', sa.String, nullable=False)
+    content = sa.Column('content', sa.LargeBinary, nullable=False)
     workflow_name = sa.Column('workflow_name', sa.String, sa.ForeignKey('workflows.workflow_name'), primary_key=True)
 
     def render(self, argument: Dict) -> AbstractFile:
@@ -68,10 +80,17 @@ class WorkflowTemplate(Base):
     def template_file(self) -> AbstractFile:
         return AbstractFile(self.filename, self.content)
 
+    @template_file.setter
+    def template_file(self, file: AbstractFile):
+        self.filename, self.content = file.filename, file.content
+
     def __json__(self, request):
         # Defer to the actual template file's contents for JSON conversion
         return self.template_file.__json__(request)
 
+    def __repr__(self):
+        return f'<WorkflowTemplate filename={self.filename} for workflow={self.workflow_name}>'
+
 
 class WorkflowRequest(Base):
     """
@@ -89,11 +108,40 @@ class WorkflowRequestFile(Base):
     A Workflow Request File is a file supplied by the user and attached to the request they have submitted.
     """
     __tablename__ = 'workflow_request_files'
-    workflow_request_id = sa.Column('workflow_request_id', sa.Integer, sa.ForeignKey('workflow_requests.workflow_request_id'),
+    workflow_request_id = sa.Column('workflow_request_id', sa.Integer,
+                                    sa.ForeignKey('workflow_requests.workflow_request_id'),
                                     primary_key=True)
     filename = sa.Column('filename', sa.String, primary_key=True)
-    content = sa.Column('content', sa.String, nullable=False)
+    content = sa.Column('content', sa.LargeBinary, nullable=False)
 
     @property
     def file(self) -> AbstractFile:
         return AbstractFile(self.filename, self.content)
+
+    @file.setter
+    def set_file(self, file: AbstractFile):
+        self.filename, self.content = file.filename, file.content
+
+    def __repr__(self):
+        return f'<WorkflowRequestFile filename={self.filename}>'
+
+
+def get_engine():
+    """
+    Generate the SQL Alchemy engine for us, using Capo.
+    :return:
+    """
+    capo = CapoConfig().settings('metadataDatabase')
+    url = capo.jdbcUrl.replace('jdbc:', '').replace('://', f'://{capo.jdbcUsername}:{capo.jdbcPassword}@')
+    return create_engine(url)
+
+
+def get_session_factory(engine):
+    """
+    Generate the SQL Alchemy session factory for us, using the supplied engine
+    :param engine:
+    :return:
+    """
+    factory = sessionmaker()
+    factory.configure(bind=engine)
+    return factory
-- 
GitLab