diff --git a/gui/src/components/api.js b/gui/src/components/api.js
index 6ebd3100bed0e6128e6db889cab1d67f93b97034..8dae504c8f4e6449ad5dab18c3d1962a84971525 100644
--- a/gui/src/components/api.js
+++ b/gui/src/components/api.js
@@ -377,7 +377,7 @@ class Api {
}
}
})
- return this.post('entries/edit_old', edit)
+ return this.post('entries/edit_v0', edit)
}
}
diff --git a/nomad/app/v1/models.py b/nomad/app/v1/models.py
index e6b10a4180d279e5b26f41c4e0ff291c8bc48c99..b1f7c9453f28f7ec4198ca0cb8d1e91a56ebd2bc 100644
--- a/nomad/app/v1/models.py
+++ b/nomad/app/v1/models.py
@@ -1098,19 +1098,6 @@ class MetadataEditRequest(WithQuery):
encountered errors etc.'''))
-class MetadataEditRequestResponse(MetadataEditRequest):
- error: Optional[str] = Field(
- description=strip('''
- An error description, if the validation failed, otherwise None.'''))
- feedback: Dict[str, str] = Field(
- default={},
- description=strip('''
- A dictionary specifying strings with feedback for specific actuins.
- The quantity name is used as key. The feedback may describe an error or just be
- an informative message. If multiple errors occur for the same quantity, only
- one error message is shown.'''))
-
-
class Files(BaseModel):
''' Configures the download of files. '''
compress: Optional[bool] = Field(
diff --git a/nomad/app/v1/routers/entries.py b/nomad/app/v1/routers/entries.py
index a53c6bd934cc0e3a5bdec8d79075cf91507f045b..a6f73db67d661f742a370b929ebd44c11d0bb67f 100644
--- a/nomad/app/v1/routers/entries.py
+++ b/nomad/app/v1/routers/entries.py
@@ -50,7 +50,7 @@ from ..utils import (
from ..models import (
Aggregation, Pagination, PaginationResponse, MetadataPagination, TermsAggregation,
WithQuery, WithQueryAndPagination, MetadataRequired, MetadataResponse, Metadata,
- MetadataEditRequest, MetadataEditRequestResponse, Files, Query, User, Owner,
+ MetadataEditRequest, Files, Query, User, Owner,
QueryParameters, metadata_required_parameters, files_parameters, metadata_pagination_parameters,
HTTPExceptionModel)
@@ -1174,7 +1174,7 @@ _editable_quantities = {
@router.post(
- '/edit_old',
+ '/edit_v0',
tags=[metadata_tag],
summary='Edit the user metadata of a set of entries',
response_model=EntryMetadataEditResponse,
@@ -1347,7 +1347,7 @@ async def post_entry_metadata_edit(
'/edit',
tags=[metadata_tag],
summary='Edit the user metadata of a set of entries',
- response_model=MetadataEditRequestResponse,
+ response_model=MetadataEditRequest,
response_model_exclude_unset=True,
response_model_exclude_none=True,
responses=create_responses(
@@ -1365,11 +1365,15 @@ async def post_entries_edit(
this endpoint; upload level attributes (like `upload_name`, `coauthors`, embargo
settings, etc) need to be set through the endpoint **uploads/upload_id/edit**.
- If the upload is published, the only operation permitted using this endpoint is to
- edit the members of datasets you own.
+ edit the entries in datasets that where created by the current user.
'''
edit_request_json = await request.json()
- response, status_code = proc.MetadataEditRequestHandler.edit_metadata(
- edit_request_json=edit_request_json, upload_id=None, user=user)
- if status_code != status.HTTP_200_OK and not data.verify_only:
- raise HTTPException(status_code=status_code, detail=response.error)
- return response
+ try:
+ verified_json = proc.MetadataEditRequestHandler.edit_metadata(
+ edit_request_json=edit_request_json, upload_id=None, user=user)
+ return verified_json
+ except RequestValidationError as e:
+ raise # A problem which we have handled explicitly. Fastapi does json conversion.
+ except Exception as e:
+ # The upload is processing or some kind of unexpected error has occured
+ raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))
diff --git a/nomad/app/v1/routers/uploads.py b/nomad/app/v1/routers/uploads.py
index fe3f68952e1952372cdf8a0461b1359dbd058ea2..1669ff1a6f85cb71b30308df83a48ef4e3e44899 100644
--- a/nomad/app/v1/routers/uploads.py
+++ b/nomad/app/v1/routers/uploads.py
@@ -25,6 +25,7 @@ from fastapi import (
APIRouter, Request, File, UploadFile, status, Depends, Path, Query as FastApiQuery,
HTTPException)
from fastapi.responses import StreamingResponse
+from fastapi.exceptions import RequestValidationError
from nomad import utils, config, files, datamodel
from nomad.files import UploadFiles, StagingUploadFiles, UploadBundle, is_safe_relative_path
@@ -35,7 +36,7 @@ from nomad.search import search
from .auth import create_user_dependency, generate_upload_token
from ..models import (
MetadataPagination, User, Direction, Pagination, PaginationResponse, HTTPExceptionModel,
- Files, files_parameters, WithQuery, MetadataEditRequest, MetadataEditRequestResponse)
+ Files, files_parameters, WithQuery, MetadataEditRequest)
from ..utils import (
parameter_dependency_from_model, create_responses, DownloadItem,
create_download_stream_zipped, create_download_stream_raw_file, create_stream_from_string)
@@ -957,7 +958,7 @@ async def put_upload_metadata(
@router.post(
'/{upload_id}/edit', tags=[metadata_tag],
summary='Updates the metadata of the specified upload.',
- response_model=MetadataEditRequestResponse,
+ response_model=MetadataEditRequest,
responses=create_responses(_upload_not_found, _not_authorized_to_upload, _bad_request),
response_model_exclude_unset=True,
response_model_exclude_none=True)
@@ -976,19 +977,23 @@ async def post_upload_edit(
- The embargo of a published upload is lifted by setting the `embargo_length` attribute
to 0.
- If the upload is published, the only operations permitted using this endpoint is to
- lift the embargo, i.e. set `embargo_length` to 0, and to edit the members of datasets
- you own.
+ lift the embargo, i.e. set `embargo_length` to 0, and to edit the entries in datasets
+ that where created by the current user.
- If a query is specified, it is not possible to edit upload level metadata (like
`upload_name`, `coauthors`, etc.), as the purpose of queries is to select only a
subset of the upload entries to edit, but changing upload level metadata would affect
**all** entries of the upload.
'''
edit_request_json = await request.json()
- response, status_code = MetadataEditRequestHandler.edit_metadata(
- edit_request_json=edit_request_json, upload_id=upload_id, user=user)
- if status_code != status.HTTP_200_OK and not data.verify_only:
- raise HTTPException(status_code=status_code, detail=response.error)
- return response
+ try:
+ verified_json = MetadataEditRequestHandler.edit_metadata(
+ edit_request_json=edit_request_json, upload_id=upload_id, user=user)
+ return verified_json
+ except RequestValidationError as e:
+ raise # A problem which we have handled explicitly. Fastapi does json conversion.
+ except Exception as e:
+ # The upload is processing or some kind of unexpected error has occured
+ raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))
@router.delete(
@@ -1139,6 +1144,41 @@ async def post_upload_action_process(
data=_upload_to_pydantic(upload))
+@router.post(
+ '/{upload_id}/action/lift-embargo', tags=[action_tag],
+ summary='Lifts the embargo of an upload.',
+ response_model=UploadProcDataResponse,
+ responses=create_responses(_upload_not_found, _not_authorized_to_upload, _bad_request),
+ response_model_exclude_unset=True,
+ response_model_exclude_none=True)
+async def post_upload_action_lift_embargo(
+ upload_id: str = Path(
+ ...,
+ description='The unique id of the upload to lift the embargo for.'),
+ user: User = Depends(create_user_dependency(required=True))):
+ ''' Lifts the embargo of an upload. '''
+ upload = _get_upload_with_write_access(
+ upload_id, user, include_published=True, published_requires_admin=False)
+ _check_upload_not_processing(upload)
+ if not upload.published:
+ raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=strip('''
+ Upload is not published, no embargo to lift.'''))
+ if not upload.with_embargo:
+ raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=strip('''
+ Upload has no embargo.'''))
+ # Lift the embargo using MetadataEditRequestHandler.edit_metadata
+ try:
+ MetadataEditRequestHandler.edit_metadata(
+ edit_request_json={'metadata': {'embargo_length': 0}}, upload_id=upload_id, user=user)
+ upload.reload()
+ return UploadProcDataResponse(
+ upload_id=upload_id,
+ data=_upload_to_pydantic(upload))
+ except Exception as e:
+ # Should only happen if the upload just started processing or something unexpected happens
+ raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))
+
+
@router.get(
'/{upload_id}/bundle', tags=[bundle_tag],
summary='Gets an *upload bundle* for the specified upload.',
@@ -1471,9 +1511,10 @@ def _get_upload_with_write_access(
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=strip('''
The specified upload_id was not found.'''))
upload = mongodb_query.first()
- if upload.main_author != str(user.user_id) and not user.is_admin:
- raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail=strip('''
- You do not have write access to the specified upload.'''))
+ if not user.is_admin and upload.main_author != str(user.user_id):
+ if not upload.coauthors or str(user.user_id) not in upload.coauthors:
+ raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail=strip('''
+ You do not have write access to the specified upload.'''))
if upload.published:
if not include_published:
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail=strip('''
diff --git a/nomad/cli/client/integrationtests.py b/nomad/cli/client/integrationtests.py
index 6ea53fe4adb4f658cd13e5cc7610efa930085e24..fe65b943280f09cc35273fa0d6b238772869d121 100644
--- a/nomad/cli/client/integrationtests.py
+++ b/nomad/cli/client/integrationtests.py
@@ -180,7 +180,7 @@ def integrationtests(auth: api.Auth, skip_parsers: bool, skip_publish: bool, ski
'datasets': [{'value': dataset}]}
response = api.post(
- 'entries/edit_old',
+ 'entries/edit_v0',
data=json.dumps(dict(actions=actions, **query_request_params)),
auth=auth)
assert response.status_code == 200, response.text
diff --git a/nomad/processing/data.py b/nomad/processing/data.py
index 44e1b41e2ee2336ace6899553cc51b91f24e346e..1afddfbf8282046639207161f695ad8fda95f71b 100644
--- a/nomad/processing/data.py
+++ b/nomad/processing/data.py
@@ -43,11 +43,13 @@ import yaml
import json
from functools import lru_cache
import requests
+from fastapi.exceptions import RequestValidationError
+from pydantic.error_wrappers import ErrorWrapper
from nomad import utils, config, infrastructure, search, datamodel, metainfo, parsing, client
from nomad.files import (
PathObject, UploadFiles, PublicUploadFiles, StagingUploadFiles, UploadBundle, create_tmp_dir)
-from nomad.processing.base import Proc, process, ProcessStatus, ProcessFailure
+from nomad.processing.base import Proc, process, ProcessStatus, ProcessFailure, ProcessAlreadyRunning
from nomad.parsing import Parser
from nomad.parsing.parsers import parser_dict, match_parser
from nomad.normalizing import normalizers
@@ -58,8 +60,7 @@ from nomad.archive import (
write_partial_archive_to_mongo, delete_partial_archives_from_mongo)
from nomad.datamodel.encyclopedia import EncyclopediaMetadata
from nomad.app.v1.models import (
- MetadataEditRequest, MetadataEditRequestResponse, And,
- Aggregation, TermsAggregation, MetadataPagination, MetadataRequired)
+ MetadataEditRequest, And, Aggregation, TermsAggregation, MetadataPagination, MetadataRequired)
from nomad.search import update_metadata as es_update_metadata
section_metadata = datamodel.EntryArchive.metadata.name
@@ -148,7 +149,7 @@ class MetadataEditRequestHandler:
@classmethod
def edit_metadata(
cls, edit_request_json: Dict[str, Any], upload_id: str,
- user: datamodel.User) -> Tuple[MetadataEditRequestResponse, int]:
+ user: datamodel.User) -> Dict[str, Any]:
'''
Method to verify and execute a generic request to edit metadata from a certain user.
The request is specified as a json dictionary. Optionally, the request could be restricted
@@ -161,32 +162,37 @@ class MetadataEditRequestHandler:
for execution, by initiating the @process :func:`edit_upload_metadata` for each affected
upload.
- The method returns a :class:`MetadataEditRequestResponse` with feedback about how it
- went, and a html style status code (int, 200 if successful, otherwise an error code).
- This method is designed to not raise any exceptions (rather, errors are reported
- by return values).
+ The method returns a json dictionary with verified data (references resolved to explicit
+ IDs, list actions always expressed as dicts with "op" and "values", etc), or raises
+ an exception, namely:
+ - A :class:`ValidationError` if the request json can't be parsed by pydantic
+ - A :class:`RequestValidationError` with information about validation failures and
+ their location (most errors should be of this type, provided that the json is valid)
+ - A :class:`ProcessAlreadyRunning` exception if one of the affected uploads has
+ a running process
+ - Some other type of exception, if something goes wrong unexpectedly (should hopefully
+ never happen)
'''
logger = utils.get_logger('nomad.processing.edit_metadata')
handler = MetadataEditRequestHandler(
logger, user, edit_request_json=edit_request_json, upload_id=upload_id)
# Validate the request
- handler.validate_request()
+ handler.validate_request() # Should raise errors if something looks wrong
- if not handler.error and not edit_request_json.get('verify_only'):
+ if not edit_request_json.get('verify_only'):
# Check if any of the affected uploads are processing
for upload in handler.affected_uploads:
upload.reload()
if upload.process_running:
- handler._fatal_error(f'Upload {upload.upload_id} is currently processing')
- return handler.create_request_response(), 400
+ raise ProcessAlreadyRunning(f'Upload {upload.upload_id} is currently processing')
# Looks good, try to trigger processing
for upload in handler.affected_uploads:
- try:
- upload.edit_upload_metadata(edit_request_json, user.user_id) # Trigger the process
- except Exception as e:
- handler._fatal_error(f'Failed to start process for upload {upload.upload_id}: {e}')
- return handler.create_request_response(), 400
- return handler.create_request_response(), handler.status_code
+ upload.edit_upload_metadata(edit_request_json, user.user_id) # Trigger the process
+ # All went well, return a verified json as response
+ verified_json = copy.deepcopy(handler.edit_request_json)
+ verified_json['metadata'] = handler.root_metadata
+ verified_json['entries'] = handler.entries_metadata
+ return verified_json
def __init__(
self, logger, user: datamodel.User,
@@ -203,12 +209,10 @@ class MetadataEditRequestHandler:
self.upload_files = upload_files
self.upload_id = upload_id
- self.status_code = 200 # A html style status code
- self.error: str = None # A description of the last error, if any.
- self.feedback: Dict[str, str] = {} # { quantity_name: feedback_msg }
- self.quantities_attempted_to_edit: Set[str] = set() # Quantities user has attempted to edit
+ self.errors: List[ErrorWrapper] = [] # A list of all encountered errors, if any
+ self.edit_attempt_locs: List[Tuple[str, ...]] = [] # locs where user has attempted to edit something
self.required_auth_level = AuthLevel.none # Maximum required auth level for the edit
- self.required_auth_level_quantity: str = None # An example of an edited quantity which requires this auth level
+ self.required_auth_level_locs: List[Tuple[str, ...]] = [] # locs where maximal auth level is needed
self.encountered_users: Dict[str, str] = {} # { ref: user_id | None }, ref = user_id | username | email
self.encountered_datasets: Dict[str, datamodel.Dataset] = {} # { ref : dataset | None }, ref = dataset_id | dataset_name
self.root_metadata: Dict[str, Any] = None # The metadata specified at the top/root level
@@ -222,42 +226,41 @@ class MetadataEditRequestHandler:
pass # TODO
def validate_request(self):
- ''' Validates the provided :class:`MetadataEditRequest` '''
- try:
- self.edit_request = MetadataEditRequest(**self.edit_request_json)
- except Exception as e:
- return self._fatal_error(f'Failed to parse request json: {e}')
+ ''' Validates the provided edit_request_json. '''
+ # Validate the request json. Will raise ValidationError if json is malformed
+ self.edit_request = MetadataEditRequest(**self.edit_request_json)
try:
if not self.upload_id and not self.edit_request.query:
- return self._fatal_error('Must specify `query`')
+ return self._loc_error('Must specify `query`', 'query')
if self.edit_request.entries and not self.edit_request.entries_key:
- return self._fatal_error('Must specify `entries_key` when specifying `entries`')
+ return self._loc_error('Must specify `entries_key` when specifying `entries`', 'entries_key')
can_edit_upload_fields = bool(self.upload_id and not self.edit_request.query)
if self.edit_request.metadata:
self.root_metadata = self._verify_metadata_edit_actions(
- self.edit_request.metadata.dict(), can_edit_upload_fields)
+ self.edit_request_json['metadata'], ('metadata',), can_edit_upload_fields)
if self.edit_request.entries:
- for key, entry_metadata in self.edit_request.entries.items():
- verified_metadata = self._verify_metadata_edit_actions(entry_metadata.dict(), False)
+ for key, entry_metadata in self.edit_request_json['entries'].items():
+ verified_metadata = self._verify_metadata_edit_actions(
+ entry_metadata, ('entries', key), False)
self.entries_metadata[key] = verified_metadata
- if not self.quantities_attempted_to_edit:
- return self._fatal_error('No fields to update specified')
+ if not self.edit_attempt_locs:
+ return self._loc_error('No fields to update specified', 'metadata')
if self.required_auth_level == AuthLevel.admin and not self.user.is_admin:
- return self._quantity_error(
- self.required_auth_level_quantity, 'Admin rights required', 401)
-
- embargo_length: int = None
- if can_edit_upload_fields and self.edit_request.metadata:
- embargo_length = self.edit_request.metadata.embargo_length # type: ignore
+ for loc in self.required_auth_level_locs:
+ self._loc_error('Admin rights required', loc)
+ return
+ embargo_length: int = self.root_metadata.get('embargo_length')
try:
self.affected_uploads = self._find_request_uploads()
except Exception as e:
- return self._fatal_error('Could not evaluate query: ' + str(e))
+ return self._loc_error('Could not evaluate query: ' + str(e), 'query')
if not self.affected_uploads:
- return self._fatal_error('No matching uploads/entries found', 404)
+ if self.edit_request.query:
+ return self._loc_error('No matching entries found', 'query')
+ return self._loc_error('No matching upload found', 'upload_id')
for upload in self.affected_uploads:
# Check permissions
coauthor = upload.coauthors and self.user.user_id in upload.coauthors
@@ -272,24 +275,33 @@ class MetadataEditRequestHandler:
else:
assert False, 'Invalid required_auth_level' # Should not happen
if not has_access:
- return self._quantity_error(
- self.required_auth_level_quantity,
- f'Editing quantity requires {self.required_auth_level} '
- f' access for upload {upload.upload_id}', 401)
+ for loc in self.required_auth_level_locs:
+ self._loc_error(
+ f'{self.required_auth_level} access required for upload '
+ f'{upload.upload_id}', loc)
+ return
# Other checks
if embargo_length is not None:
if upload.published and not admin and embargo_length != 0:
- self._quantity_error(
- 'embargo_length',
- f'Upload {upload.upload_id} is published, embargo can only be lifted')
+ self._loc_error(
+ f'Upload {upload.upload_id} is published, embargo can only be lifted',
+ ('metadata', 'embargo_length'))
if upload.published and not admin:
- for quantity_name in self.quantities_attempted_to_edit:
- if quantity_name not in ('embargo_length', 'datasets'):
- self._quantity_error(
- quantity_name,
- f'Cannot update, upload {upload.upload_id} is published.', 401)
+ has_invalid_edit = False
+ for edit_loc in self.edit_attempt_locs:
+ if edit_loc[-1] not in ('embargo_length', 'datasets'):
+ has_invalid_edit = True
+ self._loc_error(
+ f'Cannot update, upload {upload.upload_id} is published.', edit_loc)
+ if has_invalid_edit:
+ return
except Exception as e:
- return self._fatal_error(f'Unexpected exception: {e}')
+ # Something unexpected has gone wrong
+ self.logger.error(e)
+ raise
+ finally:
+ if self.errors:
+ raise RequestValidationError(errors=self.errors)
def get_upload_metadata_to_set(self, upload: 'Upload') -> Dict[str, Any]:
'''
@@ -324,22 +336,14 @@ class MetadataEditRequestHandler:
pass # TODO
return rv
- def _quantity_error(self, quantity_name: str, msg: str, error_code: int = 400):
- ''' Registers an error message for a specific quantity. '''
- self.status_code = error_code
- self.error = f'Error for quantity `{quantity_name}`: {msg}'
- self.feedback[quantity_name] = msg
- self.logger.error(self.error)
-
- def _fatal_error(self, error: str, status_code: int = 400):
- ''' Registers a general, unrecoverable error, not related to a specific quantity '''
- self.error = error
- self.status_code = status_code
- self.logger.error(error)
+ def _loc_error(self, msg: str, loc: Union[str, Tuple[str, ...]]):
+ ''' Registers a located error. '''
+ self.errors.append(ErrorWrapper(Exception(msg), loc=loc))
+ self.logger.error(msg, loc=loc)
def _verify_metadata_edit_actions(
- self, metadata_edit_actions: Dict[str, Any], can_edit_upload_fields: bool,
- auth_level: AuthLevel = None) -> Dict[str, Any]:
+ self, metadata_edit_actions: Dict[str, Any], loc: Tuple[str, ...],
+ can_edit_upload_fields: bool, auth_level: AuthLevel = None) -> Dict[str, Any]:
'''
Performs *basic* validation of a dictionary with metadata edit actions, and returns a
dictionary with the same structure, but containing only the *verified* actions. Verified
@@ -357,36 +361,36 @@ class MetadataEditRequestHandler:
for quantity_name, action in metadata_edit_actions.items():
if action is not None:
success, verified_action = self._verify_metadata_edit_action(
- quantity_name, action, can_edit_upload_fields, auth_level)
+ quantity_name, action, loc + (quantity_name,), can_edit_upload_fields, auth_level)
if success:
rv[quantity_name] = verified_action
return rv
def _verify_metadata_edit_action(
- self, quantity_name: str, action: Any, can_edit_upload_fields: bool,
- auth_level: AuthLevel) -> Tuple[bool, Any]:
+ self, quantity_name: str, action: Any, loc: Tuple[str, ...],
+ can_edit_upload_fields: bool, auth_level: AuthLevel) -> Tuple[bool, Any]:
'''
Performs basic validation of a single action. Returns (success, verified_action).
'''
definition = _editable_metadata.get(quantity_name)
if not definition:
- self._quantity_error(quantity_name, 'Unknown quantity')
+ self._loc_error('Unknown quantity', loc)
return False, None
- self.quantities_attempted_to_edit.add(quantity_name)
+ self.edit_attempt_locs.append(loc)
field_auth_level = getattr(definition, 'a_auth_level', AuthLevel.coauthor)
if auth_level is not None:
# Our auth level is known, check it immediately
if field_auth_level > auth_level:
- self._quantity_error(quantity_name, f'Requires {field_auth_level} privileges')
+ self._loc_error(f'{field_auth_level} privileges required', loc)
return False, None
if field_auth_level > self.required_auth_level:
self.required_auth_level = field_auth_level
- self.required_auth_level_quantity = quantity_name
+ self.required_auth_level_locs = [loc]
if quantity_name in _mongo_upload_metadata and not can_edit_upload_fields:
- self._quantity_error(quantity_name, 'Quantity can only be edited on the upload level')
+ self._loc_error('Quantity can only be edited on the upload level', loc)
return False, None
try:
@@ -401,9 +405,8 @@ class MetadataEditRequestHandler:
values = action['values']
assert op in ('set', 'add', 'remove'), 'op should be `set`, `add` or `remove`'
if quantity_name == 'datasets' and op == 'set':
- self._quantity_error(
- quantity_name,
- 'Only `add` and `remove` operations permitted for datasets')
+ self._loc_error(
+ 'Only `add` and `remove` operations permitted for datasets', loc)
return False, None
else:
op = 'set'
@@ -414,7 +417,7 @@ class MetadataEditRequestHandler:
verified_values = [self._verified_value(definition, v) for v in values]
return True, dict(op=op, values=verified_values)
except Exception as e:
- self._quantity_error(quantity_name, str(e))
+ self._loc_error(str(e), loc)
return False, None
def _verified_value(
@@ -591,21 +594,6 @@ class MetadataEditRequestHandler:
for entry in Calc.objects(upload_id=upload.upload_id):
yield entry
- def create_request_response(self) -> MetadataEditRequestResponse:
- ''' Creates a :class:`MetadataEditRequestResponse` with the validation results. '''
- # Create response by updating the request json with the verified values when possible
- verified_json = copy.deepcopy(self.edit_request_json)
- if self.root_metadata:
- verified_json['metadata'].update(self.root_metadata)
- if self.entries_metadata:
- for key, entry_metadata in verified_json['entries'].items():
- entry_metadata.update(self.entries_metadata.get(key, {}))
- # Create response
- response = MetadataEditRequestResponse(**verified_json)
- response.error = self.error
- response.feedback = self.feedback
- return response
-
class Calc(Proc):
'''
@@ -2031,8 +2019,7 @@ class Upload(Proc):
# Validate the request (the @process could have been invoked directly, without previous validation)
handler = MetadataEditRequestHandler(
logger, user, edit_request_json=edit_request_json, upload_id=self.upload_id)
- handler.validate_request()
- assert not handler.error, handler.error
+ handler.validate_request() # Should raise errors if something looks wrong
# Upload level metadata
old_with_embargo = self.with_embargo
diff --git a/tests/app/v1/routers/test_entries_edit.py b/tests/app/v1/routers/test_entries_edit.py
index 3089a0a173944a59346c5c15e72274151c63da68..baeb3ef66e5775e364d8fecb440e989357241e50 100644
--- a/tests/app/v1/routers/test_entries_edit.py
+++ b/tests/app/v1/routers/test_entries_edit.py
@@ -86,7 +86,7 @@ class TestEditRepo():
if verify:
data.update(verify=verify)
- return self.api.post('entries/edit_old', headers=self.test_user_auth, json=data)
+ return self.api.post('entries/edit_v0', headers=self.test_user_auth, json=data)
def assert_edit(self, rv, quantity: str, success: bool, message: bool, status_code: int = 200):
data = rv.json()
@@ -325,7 +325,7 @@ class TestEditRepo():
'test_user', dict(
query={'upload_id': 'id_unpublished_w'},
metadata=all_admin_entry_metadata,
- expected_status_code=401),
+ expected_error_loc=('metadata', 'entry_create_time')),
id='protected-not-admin'),
pytest.param(
'admin_user', dict(
@@ -338,7 +338,7 @@ class TestEditRepo():
'test_user', dict(
query={'upload_id': 'id_published_w'},
metadata=dict(comment='test comment'),
- expected_status_code=401),
+ expected_error_loc=('metadata', 'comment')),
id='published-not-admin'),
pytest.param(
None, dict(
@@ -358,7 +358,7 @@ class TestEditRepo():
'other_test_user', dict(
query={'upload_id': 'id_unpublished_w'},
metadata=dict(comment='test comment'),
- expected_status_code=404),
+ expected_error_loc=('query',)),
id='no-access'),
pytest.param(
'other_test_user', dict(
@@ -377,13 +377,13 @@ class TestEditRepo():
'test_user', dict(
query={'upload_id': 'id_unpublished_w'},
metadata=dict(upload_name='a test name'),
- expected_status_code=400),
+ expected_error_loc=('metadata', 'upload_name')),
id='query-cannot-edit-upload-data'),
pytest.param(
'test_user', dict(
query={'upload_create_time:lt': '2021-01-01'},
metadata=dict(comment='a test comment'),
- expected_status_code=404),
+ expected_error_loc=('query',)),
id='query-no-results')])
def test_post_entries_edit(
client, proc_infra, example_data_writeable, a_dataset, test_auth_dict, test_users_dict,
@@ -401,10 +401,10 @@ def test_post_entries_edit(
entries = kwargs.get('entries')
entries_key = kwargs.get('entries_key')
verify_only = kwargs.get('verify_only', False)
- expected_status_code = kwargs.get('expected_status_code', 200)
- if expected_status_code == 200 and not verify_only:
- affected_upload_ids = kwargs.get('affected_upload_ids')
- expected_metadata = kwargs.get('expected_metadata', metadata)
+ expected_error_loc = kwargs.get('expected_error_loc')
+ expected_status_code = kwargs.get('expected_status_code')
+ affected_upload_ids = kwargs.get('affected_upload_ids')
+ expected_metadata = kwargs.get('expected_metadata', metadata)
add_coauthor = kwargs.get('add_coauthor', False)
if add_coauthor:
@@ -419,8 +419,14 @@ def test_post_entries_edit(
url = 'entries/edit'
edit_start = datetime.utcnow().isoformat()[0:22]
response = client.post(url, headers=user_auth, json=edit_request_json)
- assert_response(response, expected_status_code)
- if expected_status_code == 200:
+ if expected_error_loc:
+ assert_response(response, 422)
+ error_locs = [tuple(d['loc']) for d in response.json()['detail']]
+ assert expected_error_loc in error_locs
+ elif expected_status_code not in (None, 200):
+ assert_response(response, expected_status_code)
+ else:
+ assert_response(response, 200)
assert_metadata_edited(
user, None, query, metadata, entries, entries_key, verify_only,
- expected_status_code, expected_metadata, affected_upload_ids, edit_start)
+ expected_metadata, affected_upload_ids, edit_start)
diff --git a/tests/app/v1/routers/test_uploads.py b/tests/app/v1/routers/test_uploads.py
index 545852ee1468a7b68204a128ffb591ebdf111d75..550285c07df0ffb8617e39c04a46676a4250599d 100644
--- a/tests/app/v1/routers/test_uploads.py
+++ b/tests/app/v1/routers/test_uploads.py
@@ -1064,12 +1064,12 @@ def test_put_upload_metadata(
pytest.param(
'test_user', 'id_unpublished_w', dict(
metadata=dict(main_author='lhofstadter'),
- expected_status_code=401),
+ expected_error_loc=('metadata', 'main_author')),
id='protected-not-admin'),
pytest.param(
'test_user', 'silly_value', dict(
metadata=dict(upload_name='test_name'),
- expected_status_code=404),
+ expected_error_loc=('upload_id',)),
id='bad-upload_id'),
pytest.param(
'admin_user', 'id_published_w', dict(
@@ -1078,7 +1078,7 @@ def test_put_upload_metadata(
pytest.param(
'test_user', 'id_published_w', dict(
metadata=dict(upload_name='test_name'),
- expected_status_code=401),
+ expected_error_loc=('metadata', 'upload_name')),
id='published-not-admin'),
pytest.param(
None, 'id_unpublished_w', dict(
@@ -1093,7 +1093,7 @@ def test_put_upload_metadata(
pytest.param(
'other_test_user', 'id_unpublished_w', dict(
metadata=dict(upload_name='test_name'),
- expected_status_code=401),
+ expected_error_loc=('metadata', 'upload_name')),
id='no-access'),
pytest.param(
'other_test_user', 'id_unpublished_w', dict(
@@ -1120,14 +1120,14 @@ def test_put_upload_metadata(
query={'and': [{'upload_create_time:gt': '2021-01-01'}, {'published': False}]},
owner='user',
metadata=dict(upload_name='a test name'),
- expected_status_code=400),
+ expected_error_loc=('metadata', 'upload_name')),
id='query-cannot-edit-upload-data'),
pytest.param(
'test_user', 'id_unpublished_w', dict(
query={'upload_create_time:lt': '2021-01-01'},
owner='user',
metadata=dict(comment='a test comment'),
- expected_status_code=404),
+ expected_error_loc=('query',)),
id='query-no-results')])
def test_post_upload_edit(
client, proc_infra, example_data_writeable, a_dataset, test_auth_dict, test_users_dict,
@@ -1145,16 +1145,17 @@ def test_post_upload_edit(
entries = kwargs.get('entries')
entries_key = kwargs.get('entries_key')
verify_only = kwargs.get('verify_only', False)
- expected_status_code = kwargs.get('expected_status_code', 200)
- if expected_status_code == 200 and not verify_only:
- affected_upload_ids = kwargs.get('affected_upload_ids', [upload_id])
- expected_metadata = kwargs.get('expected_metadata', metadata)
+ expected_error_loc = kwargs.get('expected_error_loc')
+ expected_status_code = kwargs.get('expected_status_code')
+ affected_upload_ids = kwargs.get('affected_upload_ids', [upload_id])
+ expected_metadata = kwargs.get('expected_metadata', metadata)
add_coauthor = kwargs.get('add_coauthor', False)
if add_coauthor:
upload = Upload.get(upload_id)
- upload.coauthors = [user.user_id]
- upload.save()
+ upload.edit_upload_metadata(
+ edit_request_json={'metadata': {'coauthors': user.user_id}}, user_id=upload.main_author)
+ upload.block_until_complete()
edit_request_json = dict(
query=query, owner=owner, metadata=metadata, entries=entries, entries_key=entries_key,
@@ -1162,11 +1163,17 @@ def test_post_upload_edit(
url = f'uploads/{upload_id}/edit'
edit_start = datetime.utcnow().isoformat()[0:22]
response = client.post(url, headers=user_auth, json=edit_request_json)
- assert_response(response, expected_status_code)
- if expected_status_code == 200:
+ if expected_error_loc:
+ assert_response(response, 422)
+ error_locs = [tuple(d['loc']) for d in response.json()['detail']]
+ assert expected_error_loc in error_locs
+ elif expected_status_code not in (None, 200):
+ assert_response(response, expected_status_code)
+ else:
+ assert_response(response, 200)
assert_metadata_edited(
user, upload_id, query, metadata, entries, entries_key, verify_only,
- expected_status_code, expected_metadata, affected_upload_ids, edit_start)
+ expected_metadata, affected_upload_ids, edit_start)
@pytest.mark.parametrize('mode, source_path, query_args, user, use_upload_token, test_limit, accept_json, expected_status_code', [
@@ -1405,6 +1412,38 @@ def test_post_upload_action_process(
assert_processing(client, upload_id, test_auth_dict['test_user'][0], check_files=False, published=True)
+@pytest.mark.parametrize('upload_id, user, preprocess, expected_status_code', [
+ pytest.param('id_published_w', 'test_user', None, 200, id='ok'),
+ pytest.param('id_published_w', 'other_test_user', None, 401, id='no-access'),
+ pytest.param('id_published_w', 'other_test_user', 'make-coauthor', 200, id='ok-coauthor'),
+ pytest.param('id_published_w', None, None, 401, id='no-credentials'),
+ pytest.param('id_published_w', 'invalid', None, 401, id='invalid-credentials'),
+ pytest.param('id_unpublished_w', 'test_user', None, 400, id='not-published'),
+ pytest.param('id_published_w', 'test_user', 'lift', 400, id='already-lifted')])
+def test_post_upload_action_lift_embargo(
+ client, proc_infra, example_data_writeable, test_auth_dict, test_users_dict,
+ upload_id, user, preprocess, expected_status_code):
+
+ user_auth, __token = test_auth_dict[user]
+ user = test_users_dict.get(user)
+
+ if preprocess:
+ if preprocess == 'lift':
+ metadata = {'embargo_length': 0}
+ elif preprocess == 'make-coauthor':
+ metadata = {'coauthors': user.user_id}
+ upload = Upload.get(upload_id)
+ upload.edit_upload_metadata(dict(metadata=metadata), config.services.admin_user_id)
+ upload.block_until_complete()
+
+ response = perform_post_upload_action(client, user_auth, upload_id, 'lift-embargo')
+ assert_response(response, expected_status_code)
+ if expected_status_code == 200:
+ assert_metadata_edited(
+ user, upload_id, None, None, None, None, False,
+ {'embargo_length': 0}, [upload_id], None)
+
+
@pytest.mark.parametrize('upload_id, user, expected_status_code', [
pytest.param('id_unpublished_w', 'test_user', 200, id='delete-own'),
pytest.param('id_unpublished_w', 'other_test_user', 401, id='delete-others-not-admin'),
diff --git a/tests/processing/test_edit_metadata.py b/tests/processing/test_edit_metadata.py
index 7f9d573bac05695d611e232cbb369583ecb233f1..4408e68ac34c1111138cd9ad5625c8326f55f237 100644
--- a/tests/processing/test_edit_metadata.py
+++ b/tests/processing/test_edit_metadata.py
@@ -18,6 +18,8 @@
import pytest
from datetime import datetime
+from fastapi.exceptions import RequestValidationError
+
from nomad import datamodel, metainfo
from nomad.processing import Upload, MetadataEditRequestHandler
from nomad.processing.data import _editable_metadata, _mongo_upload_metadata
@@ -63,34 +65,37 @@ def assert_edit_request(user, **kwargs):
entries = kwargs.get('entries')
entries_key = kwargs.get('entries_key', 'calc_id')
verify_only = kwargs.get('verify_only', False)
- expected_status_code = kwargs.get('expected_status_code', 200)
- if expected_status_code == 200 and not verify_only:
- affected_upload_ids = kwargs.get('affected_upload_ids', [upload_id])
- expected_metadata = kwargs.get('expected_metadata', metadata)
+ expected_error_loc = kwargs.get('expected_error_loc')
+ affected_upload_ids = kwargs.get('affected_upload_ids', [upload_id])
+ expected_metadata = kwargs.get('expected_metadata', metadata)
# Perform edit request
edit_request_json = dict(
query=query, owner=owner, metadata=metadata, entries=entries, entries_key=entries_key,
verify=verify_only)
edit_start = datetime.utcnow().isoformat()[0:22]
- _response, status_code = MetadataEditRequestHandler.edit_metadata(edit_request_json, upload_id, user)
+ try:
+ MetadataEditRequestHandler.edit_metadata(edit_request_json, upload_id, user)
+ except RequestValidationError as e:
+ error_locs = [error_dict['loc'] for error_dict in e.errors()]
# Validate result
- assert status_code == expected_status_code, 'Wrong status code returned'
- if status_code == 200 and not verify_only:
+ if expected_error_loc:
+ assert expected_error_loc in error_locs
+ if not expected_error_loc and not verify_only:
assert_metadata_edited(
user, upload_id, query, metadata, entries, entries_key, verify_only,
- expected_status_code, expected_metadata, affected_upload_ids, edit_start)
+ expected_metadata, affected_upload_ids, edit_start)
def assert_metadata_edited(
user, upload_id, query, metadata, entries, entries_key, verify_only,
- expected_status_code, expected_metadata, affected_upload_ids, edit_start):
+ expected_metadata, affected_upload_ids, edit_start):
for upload_id in affected_upload_ids:
upload = Upload.get(upload_id)
upload.block_until_complete()
for entry in upload.calcs:
assert entry.last_edit_time
- assert entry.last_edit_time.isoformat()[0:22] >= edit_start
+ assert edit_start is None or entry.last_edit_time.isoformat()[0:22] >= edit_start
entry_metadata_mongo = entry.mongo_metadata(upload).m_to_dict()
entry_metadata_es = search(owner=None, query={'calc_id': entry.calc_id}).data[0]
values_to_check = expected_metadata
@@ -189,22 +194,22 @@ def convert_to_comparable_value_single(quantity, value, format, user):
pytest.param(
dict(
metadata=dict(external_db='bad value'),
- expected_status_code=400),
+ expected_error_loc=('metadata', 'external_db')),
id='bad-external_db'),
pytest.param(
dict(
metadata=dict(coauthors='silly value'),
- expected_status_code=400),
+ expected_error_loc=('metadata', 'coauthors')),
id='bad-coauthor-ref'),
pytest.param(
dict(
metadata=dict(reviewers='silly value'),
- expected_status_code=400),
+ expected_error_loc=('metadata', 'reviewers')),
id='bad-reviewer-ref'),
pytest.param(
dict(
metadata=dict(datasets=['silly value']),
- expected_status_code=400),
+ expected_error_loc=('metadata', 'datasets')),
id='bad-dataset-ref'),
pytest.param(
dict(
@@ -225,7 +230,7 @@ def convert_to_comparable_value_single(quantity, value, format, user):
owner='user',
upload_id=None,
metadata=dict(comment='new comment'),
- expected_status_code=404),
+ expected_error_loc=('query',)),
id='query-no-results'),
pytest.param(
dict(
@@ -233,7 +238,7 @@ def convert_to_comparable_value_single(quantity, value, format, user):
owner='user',
upload_id=None,
metadata=dict(comment='new comment'),
- expected_status_code=401),
+ expected_error_loc=('metadata', 'comment')),
id='query-contains-published')])
def test_edit_metadata(proc_infra, purged_app, example_data_writeable, a_dataset, test_users_dict, kwargs):
kwargs['user'] = test_users_dict[kwargs.get('user', 'test_user')]
@@ -265,7 +270,7 @@ def test_admin_quantities(proc_infra, example_data_writeable, test_user, other_t
# try to do the same as a non-admin
for k, v in all_admin_metadata.items():
assert_edit_request(
- user=test_user, upload_id='id_unpublished_w', metadata={k: v}, expected_status_code=401)
+ user=test_user, upload_id='id_unpublished_w', metadata={k: v}, expected_error_loc=('metadata', k))
def test_query_cannot_set_upload_attributes(proc_infra, example_data_writeable, a_dataset, test_user):
@@ -277,7 +282,7 @@ def test_query_cannot_set_upload_attributes(proc_infra, example_data_writeable,
assert_edit_request(
user=test_user, query=query, owner='user', upload_id=upload_id,
metadata={k: v},
- expected_status_code=400)
+ expected_error_loc=('metadata', k))
# Attempting to edit an entry level attribute with query should always succeed
assert_edit_request(
user=test_user, query=query, owner='user', upload_id=None,