Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/adding mset replacement functionality 20210615 #445

Open
wants to merge 65 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
b73c2c9
added MsetReplacementEvent model
cschu Jun 15, 2021
c61ae1d
fixed alembic complaint
cschu Jun 15, 2021
723ec8b
added alembic script for revision related to b73c2c91
cschu Jun 15, 2021
e53527b
pleasing flake8
cschu Jun 15, 2021
157ef00
Merge branch 'develop' into feature/adding_mset_replacement_functiona…
cschu Jun 16, 2021
0708ccb
Merge branch 'develop' into feature/adding_mset_replacement_functiona…
cschu Jun 16, 2021
3dbc0f4
fixed MsetReplacement/MetaDataSet key issues (with @lkuchenb)
cschu Jun 24, 2021
df35eba
merged upstream
cschu Jun 24, 2021
1b3a3fc
pleasing flake8
cschu Jun 24, 2021
9f09d96
Merge branch 'develop' into feature/adding_mset_replacement_functiona…
cschu Jun 24, 2021
729ddcc
added initial mset replacement logic
cschu Jun 24, 2021
85ade91
merged upstream
cschu Jun 24, 2021
2bd9b75
pleasing flake8 and mypy
cschu Jun 24, 2021
523208f
addressing @lkuchenb's code review
cschu Jun 25, 2021
9068be3
added user.can_update flag
cschu Jun 25, 2021
2392560
generated alembic script pertaining to 9068be3
cschu Jun 25, 2021
54d3b63
pleasing flake8
cschu Jun 25, 2021
1c9a668
pleasing flake8
cschu Jun 25, 2021
1ffefdb
Merge branch 'develop' into feature/adding_mset_replacement_functiona…
cschu Jun 29, 2021
bf20b41
fixing the fixtures
cschu Jun 29, 2021
67fe823
fixing the fixtures
cschu Jun 29, 2021
9accbed
addressing mypy error
cschu Jun 29, 2021
5db3d83
implemented code review requests (@lkuchenb)
cschu Jun 30, 2021
b92e67e
patched exposed event id
cschu Jun 30, 2021
307353f
actually fixed exposed event id
cschu Jul 1, 2021
f5d2483
added can_update to restricted fields, updated info request integrati…
cschu Jul 1, 2021
7a4c6de
added metadataset->msetreplacementevent relationships
cschu Jul 1, 2021
effc7d7
simplified check for already replaced metadatasets
cschu Jul 1, 2021
db6efbc
added back_populates args
cschu Jul 1, 2021
6c7199b
fixed issue with introducing can_update column for existing users
cschu Jul 1, 2021
621babc
Merge branch 'develop' into feature/adding_mset_replacement_functiona…
cschu Jul 1, 2021
55a11f7
openapi version bump (1.4.0)
cschu Jul 1, 2021
c6a340f
added/registered new api route; renamed information request operation…
cschu Jul 5, 2021
e0dfe33
added update_metadatasets protoype
cschu Jul 6, 2021
ce78f8a
Merge branch 'develop' into feature/adding_mset_replacement_functiona…
cschu Jul 6, 2021
4fecd28
pleasing flake8 and mypy
cschu Jul 6, 2021
115eff6
package bump (pleasing versioning checks)
cschu Jul 6, 2021
4912fdb
merged openapi.yaml with upstream changes
cschu Jul 6, 2021
36cd8ec
Apply suggestions from code review
cschu Jul 6, 2021
ac112b7
works
cschu Jul 6, 2021
4a35bce
weird.
cschu Jul 6, 2021
e0803da
weird.
cschu Jul 6, 2021
6f75123
weird
cschu Jul 6, 2021
d0b3f6b
refactored + working
cschu Jul 7, 2021
e77bf56
added preliminary test for metadataset replacement
cschu Jul 7, 2021
0289f2a
workaround to re-enable bulk-deletion test
cschu Jul 7, 2021
ac2b431
removed redundant label from msetreplacementevents (label is attached…
cschu Jul 14, 2021
3e1bfcd
cleaning
cschu Jul 14, 2021
c6f22a0
added can_update=True to initial user
cschu Jul 14, 2021
9e88373
metadatasetresponses now include replaced/replaces information (#462)
cschu Jul 14, 2021
d882ffe
added more testcases
cschu Jul 28, 2021
d395384
pleasing flake8
cschu Jul 28, 2021
b7cf1ed
pleasing mypy
cschu Jul 28, 2021
e47a56d
pleasing mypy
cschu Jul 28, 2021
b87e481
removed comment
cschu Jul 28, 2021
455ee05
added information about replaced metadatasets to datatable
cschu Jul 28, 2021
27163f5
Apply suggestions from @lkuchenb's code review
cschu Aug 3, 2021
306cf35
implemented suggestions from @lkuchenb's code review
cschu Aug 3, 2021
d353255
merge upstream (github code review)
cschu Aug 3, 2021
455626a
dealt with fallout from renaming msetreplacements-table
cschu Aug 3, 2021
00738a1
fixed bulk deletion test
cschu Aug 4, 2021
ea30854
fixed failing stage_and_submit test
cschu Aug 4, 2021
cbe39eb
pleasing mypy
cschu Aug 4, 2021
dd036cc
fixed issue that prevented new user registration requests to be compl…
cschu Aug 24, 2021
216a08b
Merge branch 'develop' into feature/adding_mset_replacement_functiona…
lkuchenb Nov 5, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions datameta/alembic/versions/20210625_f6a70402119f.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
"""added msetreplacements table, moved update-related fields from mset, added user.can_update flag

Revision ID: f6a70402119f
Revises: 7fdc829db18d
Create Date: 2021-06-25 15:27:37.638601

"""
from alembic import op
import sqlalchemy as sa
from sqlalchemy.dialects import postgresql

# revision identifiers, used by Alembic.
revision = 'f6a70402119f'
down_revision = '7fdc829db18d'
branch_labels = None
depends_on = None


def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.create_table(
'msetreplacements',
sa.Column('id', sa.Integer(), nullable=False),
sa.Column('uuid', postgresql.UUID(as_uuid=True), nullable=False),
sa.Column('user_id', sa.Integer(), nullable=False),
sa.Column('datetime', sa.DateTime(), nullable=False),
sa.Column('label', sa.String(length=140), nullable=False),
sa.Column('new_metadataset_id', sa.Integer(), nullable=False),
sa.ForeignKeyConstraint(['new_metadataset_id'], ['metadatasets.id'], name=op.f('fk_msetreplacements_new_metadataset_id_metadatasets')),
sa.ForeignKeyConstraint(['user_id'], ['users.id'], name=op.f('fk_msetreplacements_user_id_users')),
sa.PrimaryKeyConstraint('id', name=op.f('pk_msetreplacements')),
sa.UniqueConstraint('uuid', name=op.f('uq_msetreplacements_uuid'))
)
op.add_column('metadatasets', sa.Column('replaced_via_event_id', sa.Integer(), nullable=True))
op.drop_constraint('fk_metadatasets_replaced_by_id_metadatasets', 'metadatasets', type_='foreignkey')
op.create_foreign_key(op.f('fk_metadatasets_replaced_via_event_id_msetreplacements'), 'metadatasets', 'msetreplacements', ['replaced_via_event_id'], ['id'], use_alter=True)
op.drop_column('metadatasets', 'deprecated_label')
op.drop_column('metadatasets', 'replaced_by_id')
op.drop_column('metadatasets', 'is_deprecated')
op.add_column('users', sa.Column('can_update', sa.Boolean(create_constraint=False), nullable=False))
# ### end Alembic commands ###


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.drop_column('users', 'can_update')
op.add_column('metadatasets', sa.Column('is_deprecated', sa.BOOLEAN(), autoincrement=False, nullable=True))
op.add_column('metadatasets', sa.Column('replaced_by_id', sa.INTEGER(), autoincrement=False, nullable=True))
op.add_column('metadatasets', sa.Column('deprecated_label', sa.VARCHAR(), autoincrement=False, nullable=True))
op.drop_constraint(op.f('fk_metadatasets_replaced_via_event_id_msetreplacements'), 'metadatasets', type_='foreignkey')
op.create_foreign_key('fk_metadatasets_replaced_by_id_metadatasets', 'metadatasets', 'metadatasets', ['replaced_by_id'], ['id'])
op.drop_column('metadatasets', 'replaced_via_event_id')
op.drop_table('msetreplacements')
# ### end Alembic commands ###
53 changes: 48 additions & 5 deletions datameta/api/metadatasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.

from dataclasses import dataclass
from datameta.models.db import MsetReplacementEvent
from pyramid.httpexceptions import HTTPForbidden, HTTPNotFound, HTTPNoContent
from pyramid.view import view_config
from pyramid.request import Request
Expand All @@ -23,8 +24,7 @@
from .. import security, siteid, resource, validation
from ..models import MetaDatum, MetaDataSet, ServiceExecution, Service, MetaDatumRecord, Submission, File
from ..security import authz
import datetime
from datetime import timezone
from datetime import timezone, datetime
from ..resource import resource_by_id, resource_query_by_id, get_identifier
from ..utils import get_record_from_metadataset
from . import DataHolderBase
Expand Down Expand Up @@ -88,7 +88,7 @@ def render_record_values(metadata: Dict[str, MetaDatum], record: dict) -> dict:
continue
elif record_rendered[field] and metadata[field].datetimefmt:
# if MetaDatum is a datetime field, render the value in isoformat
record_rendered[field] = datetime.datetime.strptime(
record_rendered[field] = datetime.strptime(
record_rendered[field],
metadata[field].datetimefmt
).isoformat()
Expand Down Expand Up @@ -150,6 +150,14 @@ def post(request: Request) -> MetaDataSetResponse:
# Obtain string converted version of the record
record = record_to_strings(request.openapi_validated.body["record"])

replaces = request.openapi_validated.body.get("replaces")
replaces_label = request.openapi_validated.body.get("replaces_label")
lkuchenb marked this conversation as resolved.
Show resolved Hide resolved

if replaces and not replaces_label:
raise errors.get_validation_error(messages=['No reason (label) given for Metadataset replacement.']) # maybe label should be reason.
if not replaces and replaces_label:
raise errors.get_validation_error(messages=["No metadataset IDs specified but replacement reason (label) is given."])

# Query the configured metadata. We're only considering and allowing
# non-service metadata when creating a new metadataset.
metadata = get_all_metadata(db, include_service_metadata=False)
Expand All @@ -166,7 +174,42 @@ def post(request: Request) -> MetaDataSetResponse:
user_id = auth_user.id,
submission_id = None
)

db.add(mdata_set)

if replaces:
mset_repl_evt = MsetReplacementEvent(
user_id = auth_user.id,
datetime = datetime.utcnow(),
label = replaces_label,
new_metadataset_id = mdata_set.id
)
db.add(mset_repl_evt)

msets = [
(mset_id, resource_by_id(db, MetaDataSet, mset_id))
for mset_id in replaces
]

missing_msets = [("Invalid metadataset id.", mset_id) for mset_id, target_mset in msets if target_mset is None]

if missing_msets:
messages, entities = zip(*missing_msets)
raise errors.get_validation_error(messages=messages, entities=entities)

already_replaced = [
(f"MetaDataSet was already replaced via event {target_mset.replaced_via_event_id}", mset_id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This exposes an internal database ID. Also, I'd hide the event logic to the user and rather print the ID of the new mset (get_identifier).

for mset_id, target_mset in msets
if target_mset.replaced_via_event_id is not None
]

if already_replaced:
messages, entities = zip(*already_replaced)
raise errors.get_validation_error(messages=messages, entities=entities)

for _, target_mset in msets:
target_mset.replaced_via_event_id = mset_repl_evt.id
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires checking whether replaced_via_event_id was previously set. Only if it is None, a replacement is legal. Otherwise, a validation error should be returned.


db.flush()

# Add NULL values for service metadata
Expand Down Expand Up @@ -279,7 +322,7 @@ def get_metadatasets(request: Request) -> List[MetaDataSetResponse]:
query = query.filter(Submission.date < submitted_before.astimezone(timezone.utc))
if awaiting_service is not None:
if awaiting_service not in readable_services_by_id:
raise errors.get_validation_error(messages=['Invalid service ID specified'], fields=['awaitingServices'])
raise errors.get_validation_error(messages=['Invalid service ID specified'], fields=['awaitingService'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

query = query.outerjoin(ServiceExecution, and_(
MetaDataSet.id == ServiceExecution.metadataset_id,
ServiceExecution.service_id == readable_services_by_id[awaiting_service].id
Expand Down Expand Up @@ -444,7 +487,7 @@ def set_metadata_via_service(request: Request) -> MetaDataSetResponse:
service = service,
metadataset = metadataset,
user = auth_user,
datetime = datetime.datetime.utcnow()
datetime = datetime.utcnow()
)

db.add(sexec)
Expand Down
18 changes: 16 additions & 2 deletions datameta/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
openapi: 3.0.0
info:
description: DataMeta
version: 1.2.0
version: 1.3.0
title: DataMeta

servers:
Expand Down Expand Up @@ -563,7 +563,7 @@ paths:
format: date-time
- name: awaitingService
in: query
description: Identifier for a service. Restricts the result to metadatsets for which the specified service has not been executed yet.
description: Identifier for a service. Restricts the result to metadatasets for which the specified service has not been executed yet.
schema:
type: string
responses:
Expand Down Expand Up @@ -1441,6 +1441,12 @@ components:
additionalProperties: true
# a free-form object,
# any property is allowed
replaces:
type: array
items:
type: string
replaces_label:
type: string
required:
- record
additionalProperties: false
Expand Down Expand Up @@ -1730,6 +1736,8 @@ components:
type: boolean
siteRead:
type: boolean
canUpdate:
type: boolean
email:
type: string
group:
Expand All @@ -1745,6 +1753,7 @@ components:
- groupAdmin
- siteAdmin
- siteRead
- canUpdate
- email
- group
additionalProperties: false
Expand Down Expand Up @@ -1775,6 +1784,9 @@ components:
$ref: "#/components/schemas/Identifier"
name:
type: string
canUpdate:
type: boolean
nullable: true
required:
- id
- name
Expand All @@ -1796,6 +1808,8 @@ components:
type: boolean
enabled:
type: boolean
canUpdate:
type: boolean

MetaDataResponse:
type: object
Expand Down
14 changes: 13 additions & 1 deletion datameta/api/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class UserUpdateRequest(DataHolderBase):
siteAdmin: bool
enabled: bool
siteRead: bool
canUpdate: bool


@dataclass
Expand All @@ -48,6 +49,7 @@ class UserResponseElement(DataHolderBase):
site_admin: Optional[bool] = None
site_read: Optional[bool] = None
email: Optional[str] = None
can_update: Optional[bool] = None

@classmethod
def from_user(cls, target_user, requesting_user):
Expand All @@ -58,7 +60,8 @@ def from_user(cls, target_user, requesting_user):
"group_admin": target_user.group_admin,
"site_admin": target_user.site_admin,
"site_read": target_user.site_read,
"email": target_user.email
"email": target_user.email,
"can_update": target_user.can_update
})

return cls(id=get_identifier(target_user), name=target_user.fullname, group=get_identifier(target_user.group), **restricted_fields)
Expand All @@ -72,6 +75,7 @@ class WhoamiResponseElement(DataHolderBase):
group_admin: bool
site_admin: bool
site_read: bool
can_update: bool
email: str
group: dict

Expand All @@ -92,6 +96,7 @@ def get_whoami(request: Request) -> WhoamiResponseElement:
group_admin = auth_user.group_admin,
site_admin = auth_user.site_admin,
site_read = auth_user.site_read,
can_update = auth_user.can_update,
email = auth_user.email,
group = {"id": get_identifier(auth_user.group), "name": auth_user.group.name}
)
Expand Down Expand Up @@ -138,6 +143,7 @@ def put(request: Request):
site_admin = request.openapi_validated.body.get("siteAdmin")
enabled = request.openapi_validated.body.get("enabled")
site_read = request.openapi_validated.body.get("siteRead")
can_update = request.openapi_validated.body.get("canUpdate")

db = request.dbsession

Expand Down Expand Up @@ -176,6 +182,10 @@ def put(request: Request):
if name is not None and not authz.update_user_name(auth_user, target_user):
raise HTTPForbidden()

# The user has to be site admin to change update privileges for a user
if can_update is not None and not authz.update_user_can_update(auth_user):
raise HTTPForbidden()

# Now, make the corresponding changes
if group_id is not None:
new_group = resource_by_id(db, Group, group_id)
Expand All @@ -192,5 +202,7 @@ def put(request: Request):
target_user.enabled = enabled
if name is not None:
target_user.fullname = name
if can_update is not None:
target_user.can_update = can_update

return HTTPNoContent()
42 changes: 30 additions & 12 deletions datameta/models/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
Table
)
from sqlalchemy.dialects.postgresql import UUID
from sqlalchemy.orm import relationship, backref
from sqlalchemy.orm import relationship

from .meta import Base

Expand Down Expand Up @@ -82,6 +82,8 @@ class User(Base):
site_admin = Column(Boolean(create_constraint=False), nullable=False)
group_admin = Column(Boolean(create_constraint=False), nullable=False)
site_read = Column(Boolean(create_constraint=False), nullable=False)
can_update = Column(Boolean(create_constraint=False), nullable=False)

# Relationships
group = relationship('Group', back_populates='user')
metadatasets = relationship('MetaDataSet', back_populates='user')
Expand All @@ -90,6 +92,7 @@ class User(Base):
apikeys = relationship('ApiKey', back_populates='user')
services = relationship('Service', secondary=user_service_table, back_populates='users')
service_executions = relationship('ServiceExecution', back_populates='user')
mset_replacements = relationship('MsetReplacementEvent', back_populates='user')


class ApiKey(Base):
Expand Down Expand Up @@ -209,20 +212,35 @@ class MetaDatumRecord(Base):
class MetaDataSet(Base):
"""A MetaDataSet represents all metadata associated with *one* record"""
__tablename__ = 'metadatasets'
id = Column(Integer, primary_key=True)
site_id = Column(String(50), unique=True, nullable=False, index=True)
uuid = Column(UUID(as_uuid=True), unique=True, default=uuid.uuid4, nullable=False)
user_id = Column(Integer, ForeignKey('users.id'), nullable=False)
submission_id = Column(Integer, ForeignKey('submissions.id'), nullable=True)
is_deprecated = Column(Boolean, default=False)
deprecated_label = Column(String, nullable=True)
replaced_by_id = Column(Integer, ForeignKey('metadatasets.id'), nullable=True)
id = Column(Integer, primary_key=True)
site_id = Column(String(50), unique=True, nullable=False, index=True)
uuid = Column(UUID(as_uuid=True), unique=True, default=uuid.uuid4, nullable=False)
user_id = Column(Integer, ForeignKey('users.id'), nullable=False)
submission_id = Column(Integer, ForeignKey('submissions.id'), nullable=True)
replaced_via_event_id = Column(Integer, ForeignKey('msetreplacements.id', use_alter=True), nullable=True)

# Relationships
user = relationship('User', back_populates='metadatasets')
submission = relationship('Submission', back_populates='metadatasets')
metadatumrecords = relationship('MetaDatumRecord', back_populates='metadataset')
replaces = relationship('MetaDataSet', backref=backref('replaced_by', remote_side=[id]))
service_executions = relationship('ServiceExecution', back_populates = 'metadataset')
service_executions = relationship('ServiceExecution', back_populates ='metadataset')

replaced_via_event = relationship('MsetReplacementEvent', primaryjoin='MetaDataSet.replaced_via_event_id==MsetReplacementEvent.id')
replaces_via_event = relationship('MsetReplacementEvent', primaryjoin='MetaDataSet.id==MsetReplacementEvent.new_metadataset_id')


class MsetReplacementEvent(Base):
""" Stores information about an mset replacement event """
__tablename__ = 'msetreplacements'
cschu marked this conversation as resolved.
Show resolved Hide resolved
id = Column(Integer, primary_key=True)
uuid = Column(UUID(as_uuid=True), unique=True, default=uuid.uuid4, nullable=False)
user_id = Column(Integer, ForeignKey('users.id'), nullable=False)
datetime = Column(DateTime, nullable=False)
label = Column(String(140), nullable=False)
new_metadataset_id = Column(Integer, ForeignKey('metadatasets.id'), nullable=False)

# Relationships
user = relationship('User', back_populates='mset_replacements')


class ApplicationSetting(Base):
Expand All @@ -244,7 +262,7 @@ class Service(Base):
site_id = Column(String(50), unique=True, nullable=False, index=True)
name = Column(Text, nullable=True, unique=True)
# Relationships
users = relationship('User', secondary=user_service_table, back_populates='services')
users = relationship('User', secondary=user_service_table, back_populates='services')
# unfortunately, 'metadata' is a reserved keyword for sqlalchemy classes
service_executions = relationship('ServiceExecution', back_populates = 'service')
target_metadata = relationship('MetaDatum', back_populates = 'service')
Expand Down
4 changes: 4 additions & 0 deletions datameta/security/authz.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ def update_user_name(user, target_user):
))


def update_user_can_update(user):
return user.site_admin


def view_restricted_user_info(user, target_user):
return has_group_rights(user, target_user.group)

Expand Down
1 change: 1 addition & 0 deletions tests/integration/fixtures/holders.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class UserFixture(Entity):
site_admin : bool
site_read : bool
enabled : bool
can_update : bool


@dataclass
Expand Down
Loading