Merge pull request #538 from dblommesteijn/master#537-merge-1.1

Master#537 merge 1.1

invenio/b2share/modules/b2deposit/b2share_deposit_handler.py

@@ -31,7 +31,7 @@
 
 from b2share_model.HTML5ModelConverter import HTML5ModelConverter
 from b2share_model import metadata_classes
-import b2share_marc_handler as mh
+import b2share_marc_handler
 
 
 
@@ -104,7 +104,7 @@ def addmeta(request, sub_id):
     meta_form = MetaForm(request.form, meta)
 
     if meta_form.validate_on_submit():
-        recid, marc = mh.create_marc(
+        recid, marc = b2share_marc_handler.create_marc(
             request.form, sub_id, current_user['email'])
         tmp_file = write_marc_to_temp_file(marc)
         # all usual tasks have priority 0; we want the bibuploads to run first

invenio/b2share/modules/b2deposit/b2share_marc_handler.py

@@ -37,46 +37,48 @@ def add_basic_fields(rec, form, email):
     """
     # why aren't subfields a dictionary?!
     try:
-        if form['title']:
+        if form.get('title'):
             record_add_field(rec, '245', subfields=[('a', remove_html_markup(form['title']))])
 
-        if form['creator']:
+        if form.get('creator'):
             fields = form.getlist('creator')
             for f in fields:
                 if f and not f.isspace():
                     record_add_field(rec, '100', subfields=[('a', remove_html_markup(f.strip()))])
 
-        if form['domain']:
+        if form.get('domain'):
             record_add_field(rec, '980', subfields=[('a', remove_html_markup(form['domain']))])
+
         pubfields = []
-        if form['publisher']:
+        if form.get('publisher'):
             pubfields.append(('b', remove_html_markup(form['publisher'])))
         if form.get('publication_date'):
             pubfields.append(('c', remove_html_markup(form['publication_date'])))
         if pubfields:
             record_add_field(rec, '260', subfields=pubfields)
+
         record_add_field(rec, '856', ind1='0', subfields=[('f', email)])
 
         if 'open_access' in form:
             record_add_field(rec, '542', subfields=[('l', 'open')])
         else:
             record_add_field(rec, '542', subfields=[('l', 'restricted')])
 
-        if form['licence']:
+        if form.get('licence'):
             record_add_field(rec, '540', subfields=[('a', remove_html_markup(form['licence']))])
         record_add_field(rec, '520', subfields=[('a', remove_html_markup(form['description']))])
 
-        if form['contact_email']:
+        if form.get('contact_email'):
             record_add_field(rec,'270',subfields=[('m', remove_html_markup(form['contact_email']))])
 
-        if form['keywords']:
+        if form.get('keywords'):
             for kw in form['keywords'].split(','):
                 if kw and not kw.isspace():
                     record_add_field(rec, '653',
                                  ind1='1',
                                  subfields=[('a', remove_html_markup(kw.strip()))])
 
-        if 'contributors' in form and form['contributors']:
+        if form.get('contributors'):
             fields = form.getlist('contributors')
             for f in fields:
                 if f and not f.isspace():
@@ -85,16 +87,16 @@ def add_basic_fields(rec, form, email):
         record_add_field(rec, '546', subfields=[('a', remove_html_markup(form['language']))])
 
         # copying zenodo here, but I don't think 980 is the right MARC field
-        if 'resource_type' in form and form['resource_type']:
+        if form.get('resource_type'):
             fields = form.getlist('resource_type')
             for f in fields:
                 record_add_field(rec, '980', subfields=[('a', remove_html_markup(form['resource_type']))])
 
-        if 'alternate_identifier' in form and form['alternate_identifier']:
+        if form.get('alternate_identifier'):
             record_add_field(rec, '024',
                              subfields=[('a', remove_html_markup(form['alternate_identifier']))])
 
-        if 'version' in form and form['version']:
+        if form.get('version'):
             record_add_field(rec, '250', subfields=[('a', remove_html_markup(form['version']))])
 
         CFG_SITE_NAME = current_app.config.get("CFG_SITE_NAME")
@@ -112,19 +114,11 @@ def create_recid():
     return run_sql("INSERT INTO bibrec(creation_date, modification_date) "
                    "values(NOW(), NOW())")
 
-
-def add_file_info(rec, form, email, sub_id, recid):
-    """
-    Adds the path to the file and access rights to ther record.
-    """
+def get_depositing_files_metadata(deposit_id):
     CFG_B2SHARE_UPLOAD_FOLDER = current_app.config.get("CFG_B2SHARE_UPLOAD_FOLDER")
-    upload_dir = os.path.join(CFG_B2SHARE_UPLOAD_FOLDER, sub_id)
+    upload_dir = os.path.join(CFG_B2SHARE_UPLOAD_FOLDER, deposit_id)
     files = os.listdir(upload_dir)
-    if 'open_access' in form:
-        fft_status = 'firerole: allow any\n'
-    else:
-        fft_status = 'firerole: allow email "{0}"\ndeny all'.format(
-            email)
+    ret = []
     for f in files:
         path = os.path.join(upload_dir, f)
         if f.startswith('metadata_'):
@@ -139,7 +133,21 @@ def add_file_info(rec, form, email, sub_id, recid):
         else:
             current_app.logger.error('Submitted file \'%s\' is missing metadata file, using default' % f)
             metadata = dict(name=f, file=path, size=str(os.path.getsize(path)))
+        ret.append(metadata)
+    return ret
+
 
+def add_file_info(rec, form, email, sub_id, recid):
+    """
+    Adds the path to the file and access rights to ther record.
+    """
+    if 'open_access' in form:
+        fft_status = 'firerole: allow any\n'
+    else:
+        fft_status = 'firerole: allow email "{0}"\ndeny all'.format(
+            email)
+    for metadata in get_depositing_files_metadata(sub_id):
+        path = metadata['file']
         record_add_field(rec, 'FFT',
                          subfields=[('a', path),
                          ('n', metadata['name']), # name of the file
@@ -153,7 +161,7 @@ def add_file_info(rec, form, email, sub_id, recid):
         #seems to be impossible to add file size data, thought this would work
 
         CFG_SITE_SECURE_URL = current_app.config.get("CFG_SITE_SECURE_URL")
-        url = "{0}/record/{1}/files/{2}".format(CFG_SITE_SECURE_URL, recid, f)
+        url = "{0}/record/{1}/files/{2}".format(CFG_SITE_SECURE_URL, recid, metadata['name'])
         record_add_field(rec, '856', ind1='4',
                          subfields=[('u', url),
                                     ('s', str(os.path.getsize(path))),

invenio/b2share/modules/b2deposit/b2share_model/model.py

@@ -85,16 +85,14 @@ def __init__(self):
                                     optional_fields=self.optional_fields))]
         self.field_args['title'] = {
             'placeholder': "Title of the resource",
-            'description':
-            'The title of the uploaded resource - a name that ' +\
-            'indicates the content to be expected.'
+            'description': 'The title of the uploaded resource - a name ' +\
+                           'that indicates the content to be expected.'
         }
         self.field_args['description'] = {
-            'description':
-            'A more elaborate description of the resource. ' +\
-            'Focus on a description of ' +\
-            'content making it easy for others to find it and to ' +\
-            'interpret its relevance quickly.'
+            'description': 'A more elaborate description of the resource. ' +\
+                           'Focus on a description of content making it ' +\
+                           'easy for others to find it and to interpret ' +\
+                           'its relevance quickly.'
         }
         self.field_args['publisher'] = {
             'hidden': True,
@@ -115,8 +113,7 @@ def __init__(self):
         }
         self.field_args['version'] = {
             'placeholder': 'e.g. v1.02',
-            'description':
-            'Denote the version of the resource.'
+            'description': 'Denote the version of the resource.'
         }
         self.field_args['licence'] = {
             'description': 'Specify the license under which this data set '+\
@@ -126,23 +123,21 @@ def __init__(self):
         }
         self.field_args['keywords'] = {
             'placeholder': "keyword1, keyword2, ...",
-            'description':
-            'A comma separated list of keywords that ' +\
-            'characterize the content.'
+            'description': 'A comma separated list of keywords that ' +\
+                           'characterize the content.'
         }
         self.field_args['open_access'] = {
-            'description':
-            'Indicate whether the resource is open or access ' +\
-            'is restricted. In case of restricted access the uploaded files ' +\
-            'will not be public, however the metadata will be.'
+            'description': 'Indicate whether the resource is open or ' +\
+                           'access is restricted. In case of restricted ' +\
+                           'access the uploaded files will not be public, ' +\
+                           'however the metadata will be.'
         }
         self.field_args['contributors'] = {
             'placeholder': 'contributor',
             'cardinality': 'n',
-            'description':
-            'A semicolon separated list of all other ' +\
-            'contributors. Mention all ' +\
-            'other persons that were relevant in the creation of the resource.'
+            'description': 'A semicolon separated list of all other ' +\
+                           'contributors. Mention all other persons that ' +\
+                           'were relevant in the creation of the resource.'
         }
         self.field_args['language'] = {
             'hidden': True,
@@ -154,23 +149,21 @@ def __init__(self):
             'data_provide': 'select',
             'cardinality': 'n',
             'data_source': ['Text', 'Image', 'Video', 'Other'],
-            'description':
-            'Select the type of the resource.'
+            'description': 'Select the type of the resource.'
         }
         self.field_args['alternate_identifier'] = {
             'placeholder': 'Other reference, such as URI, ISBN, etc.',
-            'description':
-            'Any kind of other reference such as a URN, URI or an ISBN number.'
+            'description': 'Any kind of other reference such as a URN, URI ' +\
+                           'or an ISBN number.'
         }
         self.field_args['creator'] = {
             'placeholder': 'author',
             'cardinality': 'n',
-            'description':
-            'A semicolon separated list of authors of the resource.'
+            'description': 'The author(s) of the resource.'
         }
         self.field_args['contact_email'] = {
             'placeholder': 'contact email',
-             'description': 'Contact email information for this record'
+            'description': 'Contact email information for this record'
         }
 
 def _create_metadata_class(cfg):
@@ -180,47 +173,31 @@ def _create_metadata_class(cfg):
     if not hasattr(cfg, 'fields'):
         cfg.fields = []
 
-    # TODO: this can be done in a simpler and clearer way now
-    def basic_field_iter():
+    def basic_fields():
+        return [f['name'] for f in cfg.fields if not f.get('extra')]
 
-        #Normal field if extra is false or not set
-        for f in cfg.fields:
-            try:
-                if not f['extra']:
-                    yield f['name']
-            except KeyError:
-                yield f['name']
-
-    def optional_field_iter():
-
-        for f in cfg.fields:
-            try:
-                if f['extra']:
-                    yield f['name']
-            except KeyError:
-                pass
+    def optional_fields():
+        return [f['name'] for f in cfg.fields if f.get('extra')]
 
     def __init__(self):
         super(type(self), self).__init__()
         if len(cfg.fields) > 0:
-            self.fieldsets.append(FieldSet(
-                cfg.domain,
-                basic_fields=list(basic_field_iter()),
-                optional_fields=list(optional_field_iter())))
+            self.fieldsets.append(
+                FieldSet(cfg.domain, basic_fields=basic_fields(), 
+                                     optional_fields=optional_fields()))
 
     clsname = cfg.domain + "Metadata"
 
     args = {'__init__': __init__,
             '__tablename__': cfg.table_name,
             '__mapper_args__': {'polymorphic_identity': cfg.table_name},
-            'id': db.Column(
-                db.Integer, db.ForeignKey('submission_metadata.id'),
-                primary_key=True),
+            'id': db.Column(db.Integer, 
+                            db.ForeignKey('submission_metadata.id'),
+                            primary_key=True),
             'field_args': {}}
 
     #The following function and call just add all external attrs manually
     def is_external_attr(n):
-
         # don't like this bit; problem is we don't want to include the
         # db import and I don't know how to exclude them except via name
         if n in ['db', 'fields']:
@@ -236,25 +213,13 @@ def is_external_attr(n):
     for f in cfg.fields:
         nullable = not f.get('required', False)
         args[f['name']] = db.Column(f['col_type'], nullable=nullable)
-        # Doesn't seem pythonic, but show me a better way
-        args['field_args'][f['name']] = {}
-        if 'display_text' in f:
-            args['field_args'][f['name']]['label'] = f.get('display_text')
-        if 'description' in f:
-            args['field_args'][f['name']]['description'] = f.get('description')
-        if 'data_provide' in f:
-            args['field_args'][f['name']]['data_provide'] = f.get('data_provide')
-        if 'data_source' in f:
-            args['field_args'][f['name']]['data_source'] = f.get('data_source')
-        if 'default' in f:
-            args['field_args'][f['name']]['default'] = f.get('default')
-        if 'placeholder' in f:
-            args['field_args'][f['name']]['placeholder'] = f.get('placeholder')
-        if 'value' in f:
-            args['field_args'][f['name']]['value'] = f.get('value')
-        if 'other' in f:
-            args['field_args'][f['name']]['other'] = f.get('other')
-        if 'cardinality' in f:
-            args['field_args'][f['name']]['cardinality'] = f.get('cardinality')
+        field_dict = {}
+        for k in f:
+            if k in ['description', 'data_provide', 'data_source', 'default', 
+                     'placeholder', 'value', 'other', 'cardinality']:
+                field_dict[k] = f.get(k)
+            elif k == 'display_text':
+                field_dict['label'] = f.get(k)
+        args['field_args'][f['name']] = field_dict
 
     return type(clsname, (SubmissionMetadata,), args)

invenio/b2share/modules/b2deposit/b2share_upload_handler.py

@@ -92,7 +92,7 @@ def upload(request, sub_id):
 
         if (chunks is None) or (int(chunk) == int(chunks) - 1):
             '''All chunks have been uploaded! Start merging the chunks!'''
-            merge_chunks_and_create_metadata(upload_dir, name, md5, safename)
+            filename = merge_chunks_and_create_metadata(upload_dir, name, md5, safename)
         return filename
 
 def merge_chunks_and_create_metadata(upload_dir, name, md5, safename):
@@ -110,14 +110,17 @@ def merge_chunks_and_create_metadata(upload_dir, name, md5, safename):
             with open(chunk, 'rb') as chunk_fd:
                 shutil.copyfileobj(chunk_fd, destination)
             os.remove(chunk)
+    create_file_metadata(upload_dir, name, file_unique_name, file_path)
 
+def create_file_metadata(upload_dir, name, file_unique_name, file_path):
     size = os.path.getsize(file_path)
     file_metadata = dict(name=name, file=file_path, size=size)
 
     # create a metadata_<uuid> file to store pickled metadata
     metadata_file_path = os.path.join(upload_dir, 'metadata_' + file_unique_name)
     pickle.dump(file_metadata, open(metadata_file_path, 'wb'))
     current_app.logger.info("finished uploading: %s, size %d, in %s" % (name, size, file_path))
+    return file_unique_name
 
 def delete(request, sub_id):
     """
@@ -170,14 +173,14 @@ def get_file(request, sub_id):
                             "CFG_B2SHARE_UPLOAD_FOLDER")
 
     filename = request.args.get('filename')
+    f = os.path.join(CFG_B2SHARE_UPLOAD_FOLDER, sub_id, filename)
     # make sure that request doesn't go outside the CFG_B2SHARE_UPLOAD_FOLDER
     if not os.path.samefile(
                             CFG_B2SHARE_UPLOAD_FOLDER,
                             os.path.commonprefix([CFG_B2SHARE_UPLOAD_FOLDER,
-                              os.path.realpath(filename)])):
+                              os.path.realpath(f)])):
         return "File " + filename + " not found", 404
 
-    f = os.path.join(CFG_B2SHARE_UPLOAD_FOLDER, sub_id, filename)
     if (os.path.isfile(f)):
         return send_file(f, attachment_filename=filename, as_attachment=True)
     else: