Skip to content

Commit

Permalink
fix: repo design harmonizing, misc updates
Browse files Browse the repository at this point in the history
  • Loading branch information
@chisholm
chisholm committed Oct 3, 2024
1 parent 82637c5 commit d839bda
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 287 deletions.
41 changes: 30 additions & 11 deletions src/dioptra/restapi/db/repository/groups.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
assert_user_exists,
check_user_collision,
construct_sql_query_filters,
get_group_id,
group_exists,
user_exists,
)
Expand Down Expand Up @@ -193,6 +194,22 @@ def get_by_filters_paged(
page_length: int,
deletion_policy: DeletionPolicy = DeletionPolicy.NOT_DELETED,
) -> tuple[Sequence[User], int]:
"""
Get some groups according to search criteria.
Args:
filters: A structure representing search criteria. See
parse_search_text().
page_start: A row index where the returned page should start
page_length: A row count representing the page length; use <= 0
for unlimited length
deletion_policy: Whether to look at deleted groups, non-deleted
groups, or all groups
Returns:
A 2-tuple including a page of Group objects, and a count of the
total number of groups matching the criteria
"""
sql_filter = construct_sql_query_filters(filters, self.SEARCHABLE_FIELDS)

count_stmt = sa.select(sa.func.count()).select_from(Group)
Expand All @@ -214,7 +231,9 @@ def get_by_filters_paged(
page_stmt = _apply_deletion_policy(page_stmt, deletion_policy)
# *must* enforce a sort order for consistent paging
page_stmt = page_stmt.order_by(Group.group_id)
page_stmt = page_stmt.offset(page_start).limit(page_length)
page_stmt = page_stmt.offset(page_start)
if page_length > 0:
page_stmt = page_stmt.limit(page_length)

groups = self.session.scalars(page_stmt).all()

Expand Down Expand Up @@ -245,13 +264,13 @@ def num_groups(

return num_groups

def num_members(self, group: Group) -> int:
def num_members(self, group: Group | int) -> int:
"""
Get the number of members in the given group. This is done in a way
that's hopefully more efficient than len(group.members).
Args:
group: A group
group: A Group object or group_id integer primary key value
Returns:
A member count
Expand All @@ -264,10 +283,11 @@ def num_members(self, group: Group) -> int:

# len(group.members) might require actually reading all the rows and
# translating them into objects. I hope to avoid that.
group_id = get_group_id(group)
num_members_stmt = (
sa.select(sa.func.count())
.select_from(GroupMember)
.where(GroupMember.group_id == group.group_id)
.where(GroupMember.group_id == group_id)
)

num_members = self.session.scalar(num_members_stmt)
Expand All @@ -278,13 +298,13 @@ def num_members(self, group: Group) -> int:

return num_members

def num_managers(self, group: Group) -> int:
def num_managers(self, group: Group | int) -> int:
"""
Get the number of managers in the given group. This is done in a way
that's hopefully more efficient than len(group.managers).
Args:
group: A group
group: A Group object or group_id integer primary key value
Returns:
A manager count
Expand All @@ -297,10 +317,11 @@ def num_managers(self, group: Group) -> int:

# len(group.members) might require actually reading all the rows and
# translating them into objects. I hope to avoid that.
group_id = get_group_id(group)
num_members_stmt = (
sa.select(sa.func.count())
.select_from(GroupManager)
.where(GroupManager.group_id == group.group_id)
.where(GroupManager.group_id == group_id)
)

num_managers = self.session.scalar(num_members_stmt)
Expand All @@ -317,8 +338,7 @@ def add_manager(
"""
Add a user to the managership of the given group. If the user is
already a manager, this is a no-op. Permissions are ignored in that
case. To modify permissions for an existing manager, see
UserRepository.set_manager_permissions().
case.
Args:
group: A group
Expand Down Expand Up @@ -408,8 +428,7 @@ def add_member(
"""
Add a user to the membership of the given group. If the user is
already a member, this is a no-op. Permissions are ignored in that
case. To modify permissions for an existing member, see
UserRepository.set_member_permissions().
case.
Args:
group: A group
Expand Down
11 changes: 7 additions & 4 deletions src/dioptra/restapi/db/repository/queues.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def get(
Get the latest snapshot of the given queue resource.
Args:
resource_ids: An ID or iterable of IDs of queue resource IDs
resource_ids: A single or iterable of queue resource IDs
deletion_policy: Whether to look at deleted queues, non-deleted
queue, or all queues
Expand Down Expand Up @@ -290,14 +290,15 @@ def get_by_filters_paged(
deletion_policy: DeletionPolicy = DeletionPolicy.NOT_DELETED,
) -> tuple[Sequence[Queue], int]:
"""
Get a page of queues according to more complex criteria.
Get some queues according to search criteria.
Args:
group: Limit queues to those owned by this group; None to not limit
the search
filters: Search criteria, see parse_search_text()
page_start: Zero-based row index where the page should start
page_length: Maximum number of rows in the page
page_length: Maximum number of rows in the page; use <= 0 for
unlimited length
sort_by: Sort criterion; must be a key of SORTABLE_FIELDS. None
to sort in an implementation-dependent way.
descending: Whether to sort in descending order; only applicable
Expand Down Expand Up @@ -364,7 +365,9 @@ def get_by_filters_paged(
sort_criteria = Queue.resource_snapshot_id
page_stmt = page_stmt.order_by(sort_criteria)

page_stmt = page_stmt.offset(page_start).limit(page_length)
page_stmt = page_stmt.offset(page_start)
if page_length > 0:
page_stmt = page_stmt.limit(page_length)

queues = self.session.scalars(page_stmt).all()

Expand Down
161 changes: 25 additions & 136 deletions src/dioptra/restapi/db/repository/users.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
assert_user_exists,
check_user_collision,
construct_sql_query_filters,
get_group_id,
get_user_id,
user_exists,
)

Expand Down Expand Up @@ -229,7 +231,8 @@ def get_by_filters_paged(
filters: A structure representing search criteria. See
parse_search_text().
page_start: A row index where the returned page should start
page_length: A row count representing the page length
page_length: A row count representing the page length; use <= 0
for unlimited length
deletion_policy: Whether to look at deleted users, non-deleted
users, or all users
Expand Down Expand Up @@ -258,7 +261,9 @@ def get_by_filters_paged(
page_stmt = _apply_deletion_policy(page_stmt, deletion_policy)
# *must* enforce a sort order for consistent paging
page_stmt = page_stmt.order_by(User.user_id)
page_stmt = page_stmt.offset(page_start).limit(page_length)
page_stmt = page_stmt.offset(page_start)
if page_length > 0:
page_stmt = page_stmt.limit(page_length)

users = self.session.scalars(page_stmt).all()

Expand Down Expand Up @@ -289,47 +294,15 @@ def num_users(

return num_users

def get_page(
self,
page_start: int,
page_length: int,
deletion_policy: DeletionPolicy = DeletionPolicy.NOT_DELETED,
) -> Sequence[User]:
"""
Get a "page" of users. The page start is a user index (not a page
index). Use index zero to start at the beginning. Using a page start
that's beyond the last user will result in an empty sequence, not an
error. The number of users returned will not be larger than
page_length (if it is > 0), but might be smaller.
Args:
page_start: A user start index (use 0 for the first page)
page_length: Page length, in terms of number of users per page.
If <= 0, don't limit page length (get all remaining users)
deletion_policy: Whether to look at deleted users, non-deleted
users, or all users
Returns:
A sequence of users
"""
stmt = sa.select(User)
stmt = _apply_deletion_policy(stmt, deletion_policy)

# *must* enforce a sort order for consistent paging
stmt = stmt.order_by(User.user_id).offset(page_start)

if page_length > 0:
stmt = stmt.limit(page_length)

return self.session.scalars(stmt).all()

def get_member_permissions(self, user: User, group: Group) -> GroupMember | None:
def get_member_permissions(
self, user: User | int, group: Group | int
) -> GroupMember | None:
"""
Get a user's permissions with respect to the given group.
Args:
group: A group
user: A user
group: A Group object or group_id integer primary key value
user: A User object or user_id integer primary key value
Returns:
A GroupMember object which contains the permissions, or None if
Expand All @@ -341,17 +314,22 @@ def get_member_permissions(self, user: User, group: Group) -> GroupMember | None
assert_group_exists(self.session, group, DeletionPolicy.NOT_DELETED)
assert_user_exists(self.session, user, DeletionPolicy.NOT_DELETED)

membership = self.session.get(GroupMember, (user.user_id, group.group_id))
group_id = get_group_id(group)
user_id = get_user_id(user)

membership = self.session.get(GroupMember, (user_id, group_id))

return membership

def get_manager_permissions(self, user: User, group: Group) -> GroupManager | None:
def get_manager_permissions(
self, user: User | int, group: Group | int
) -> GroupManager | None:
"""
Get a user's group manager permissions with respect to the given group.
Args:
group: A group
user: A user
group: A Group object or group_id integer primary key value
user: A User object or user_id integer primary key value
Returns:
A GroupManager object which contains the permissions, or None if
Expand All @@ -363,101 +341,12 @@ def get_manager_permissions(self, user: User, group: Group) -> GroupManager | No
assert_group_exists(self.session, group, DeletionPolicy.NOT_DELETED)
assert_user_exists(self.session, user, DeletionPolicy.NOT_DELETED)

manager = self.session.get(GroupManager, (user.user_id, group.group_id))

return manager
group_id = get_group_id(group)
user_id = get_user_id(user)

def set_member_permissions(
self,
user: User,
group: Group,
read: bool | None = None,
write: bool | None = None,
share_read: bool | None = None,
share_write: bool | None = None,
) -> None:
"""
Set a user's permissions with respect to the given group. Use None as
a permission value if needed, to leave a permission as-is.
Args:
group: A group
user: A user
read: The read permission to set
write: The write permission to set
share_read: The share_read permission to set
share_write: The share_write permission to set
Raises:
Exception: If the user or group don't exist, or if the given user
is not a member of the given group
"""
manager = self.session.get(GroupManager, (user_id, group_id))

# TODO: is this method really necessary? Users could call
# get_member_permissions() to get a GroupMember object and then
# modify permissions themselves.

assert_group_exists(self.session, group, DeletionPolicy.NOT_DELETED)
assert_user_exists(self.session, user, DeletionPolicy.NOT_DELETED)

membership = self.session.get(GroupMember, (user.user_id, group.group_id))

if membership:
if read is not None:
membership.read = read
if write is not None:
membership.write = write
if share_read is not None:
membership.share_read = share_read
if share_write is not None:
membership.share_write = share_write

else:
raise Exception(
f"Not a member: user={user.user_id}, group={group.group_id}"
)

def set_manager_permissions(
self,
user: User,
group: Group,
owner: bool | None = None,
admin: bool | None = None,
) -> None:
"""
Set a manager's permissions with respect to the given group. Use None
as a permission value if needed, to leave a permission as-is.
Args:
group: A group
user: A user
owner: The owner permission to set
admin: The admin permission to set
Raises:
Exception: if the user or group don't exist, or if the given user
is not a manager of the given group
"""

# TODO: is this method really necessary? Users could call
# get_manager_permissions() to get a GroupManager object and then
# modify permissions themselves.

assert_group_exists(self.session, group, DeletionPolicy.NOT_DELETED)
assert_user_exists(self.session, user, DeletionPolicy.NOT_DELETED)

manager = self.session.get(GroupManager, (user.user_id, group.group_id))

if manager:
if owner is not None:
manager.owner = owner
if admin is not None:
manager.admin = admin

else:
raise Exception(
f"Not a manager: user={user.user_id}, group={group.group_id}"
)
return manager


def _apply_deletion_policy(
Expand Down
Loading

0 comments on commit d839bda

Please sign in to comment.