-
Notifications
You must be signed in to change notification settings - Fork 66
Track created_by and updated_by in Node and Revisions #1255
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
base: main
Are you sure you want to change the base?
Changes from 11 commits
22dda3f
a4b1464
17e2e95
66f8e6c
f329d17
4f53904
47139ea
c29b1d6
505796b
93a13a4
8eefbe2
61b68ce
f7be73a
7c88164
6c7d09a
434433e
a4a95b2
1173829
e7cdf53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,3 +85,6 @@ uv.lock | |
| # pixi environments | ||
| .pixi/* | ||
| !.pixi/config.toml | ||
|
|
||
| # copilot files | ||
| copilot-instructions.md | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| import asyncio | ||
| import numpy | ||
| import pytest | ||
| import time | ||
|
|
||
| from tiled.access_control.access_tags import AccessTagsCompiler | ||
| from tiled.access_control.scopes import ALL_SCOPES | ||
| from tiled.client import Context, from_context | ||
| from tiled.server.app import build_app_from_config | ||
|
|
||
| from .utils import enter_username_password | ||
|
|
||
| arr = numpy.ones((5, 5)) | ||
|
|
||
| server_config = { | ||
| "access_control": { | ||
danielballan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "access_policy": "tiled.access_control.access_policies:TagBasedAccessPolicy", | ||
| "args": { | ||
| "provider": "toy", | ||
| "tags_db": { | ||
| "uri": "file:compiled_tags_mem?mode=memory&cache=shared" # in-memory and shareable | ||
| }, | ||
| "access_tags_parser": "tiled.access_control.access_tags:AccessTagsParser", | ||
| }, | ||
| }, | ||
| "authentication": { | ||
| "tiled_admins": [{"provider": "toy", "id": "admin"}], | ||
| "allow_anonymous_access": True, | ||
| "secret_keys": ["SECRET"], | ||
| "providers": [ | ||
| { | ||
| "provider": "toy", | ||
| "authenticator": "tiled.authenticators:DictionaryAuthenticator", | ||
| "args": { | ||
| "users_to_passwords": { | ||
| "alice": "alice", | ||
| "bob": "bob", | ||
| "admin": "admin", | ||
| }, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| } | ||
|
|
||
|
|
||
| access_tag_config = { | ||
| "roles": { | ||
| "facility_admin": { | ||
| "scopes": [ | ||
| "read:data", | ||
| "read:metadata", | ||
| "write:data", | ||
| "write:metadata", | ||
| "delete:node", | ||
| "delete:revision", | ||
| "create:node", | ||
| "register", | ||
| ] | ||
| }, | ||
| }, | ||
| "tags": { | ||
| "alice_tag": { | ||
| "users": [ | ||
| { | ||
| "name": "alice", | ||
| "role": "facility_admin", | ||
| }, | ||
| { | ||
| "name": "bob", | ||
| "role": "facility_admin", | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| "tag_owners": { | ||
| "alice_tag": { | ||
| "users": [ | ||
| { | ||
| "name": "alice", | ||
| }, | ||
| { | ||
| "name": "bob", | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
|
|
||
| def group_parser(groupname): | ||
| return { | ||
| "physicists": ["alice", "bob"], | ||
| }[groupname] | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def compile_access_tags_db(): | ||
| access_tags_compiler = AccessTagsCompiler( | ||
| ALL_SCOPES, | ||
| access_tag_config, | ||
| {"uri": "file:compiled_tags_mem?mode=memory&cache=shared"}, | ||
| group_parser, | ||
| ) | ||
|
|
||
| access_tags_compiler.load_tag_config() | ||
| access_tags_compiler.compile() | ||
| yield access_tags_compiler | ||
| access_tags_compiler.connection.close() | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def access_control_test_context_factory(tmpdir_module, compile_access_tags_db): | ||
| config = { | ||
| "trees": [ | ||
| { | ||
| "tree": "tiled.catalog:in_memory", | ||
| "args": { | ||
| "named_memory": "catalog_foo", | ||
| "writable_storage": str(tmpdir_module / "foo"), | ||
| "top_level_access_blob": {"tags": ["alice_tag"]}, | ||
| }, | ||
| "path": "/foo", | ||
| }, | ||
| ], | ||
| } | ||
|
|
||
| config.update(server_config) | ||
| contexts = [] | ||
| clients = {} | ||
|
|
||
| def _create_and_login_context(username, password=None, api_key=None): | ||
| if not any([password, api_key]): | ||
| raise ValueError("Please provide either 'password' or 'api_key' for auth") | ||
|
|
||
| if client := clients.get(username, None): | ||
| return client | ||
| app = build_app_from_config(config) | ||
| context = Context.from_app( | ||
| app, uri=f"http://local-tiled-app-{username}/api/v1", api_key=api_key | ||
| ) | ||
| contexts.append(context) | ||
| client = from_context(context, remember_me=False) | ||
| clients[username] = client | ||
| if api_key is None: | ||
| with enter_username_password(username, password): | ||
| client.context.login(remember_me=False) | ||
| return client | ||
|
|
||
| yield _create_and_login_context | ||
|
|
||
| for context in contexts: | ||
| context.close() | ||
|
|
||
|
|
||
| async def coro_test(c, keys): | ||
| child_node = await c.context.http_client.app.state.root_tree[ | ||
| keys[0] | ||
| ].lookup_adapter([keys[1]]) | ||
| return child_node | ||
|
|
||
|
|
||
| def test_created_and_updated_info(access_control_test_context_factory): | ||
| """ | ||
| Test that created_by and updated_by fields are correctly set | ||
| on node creation and metadata update. | ||
| """ | ||
|
|
||
| alice_client = access_control_test_context_factory("alice", "alice") | ||
| bob_client = access_control_test_context_factory("bob", "bob") | ||
|
|
||
| top = "foo" | ||
| for data in ["data_M"]: | ||
| # Create a new node and check created_by and updated_by | ||
| alice_client[top].write_array( | ||
| arr, | ||
| key=data, | ||
| metadata={"description": "initial"}, | ||
| access_tags=["alice_tag"], | ||
| ) | ||
| coro_obj = coro_test(alice_client, [top, data]) | ||
| result = asyncio.run(coro_obj) | ||
| # When the array is first created, created_by and updated_by should be the same | ||
| assert result.node.created_by == "alice" | ||
| assert result.node.updated_by == "alice" | ||
| assert result.node.time_created.date() == result.node.time_updated.date() | ||
|
|
||
| time.sleep(1) # ensure time_updated is different | ||
| bob_client[top][data].replace_metadata(metadata={"description": "updated"}) | ||
| coro_obj = coro_test(bob_client, [top, data]) | ||
| result = asyncio.run(coro_obj) | ||
| # After Bob updates the metadata, updated_by should be Bob, created_by should remain Alice | ||
| assert result.node.created_by != result.node.updated_by | ||
| assert result.node.time_created != result.node.time_updated | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -404,28 +404,18 @@ async def approx_len(self) -> Optional[int]: | |
|
|
||
| if self.context.engine.dialect.name == "postgresql": | ||
| async with self.context.session() as db: | ||
| parent_and_freqs = await db.execute( | ||
| text( | ||
| """ | ||
| parent_and_freqs = await db.execute(text(""" | ||
| SELECT unnest(most_common_vals::text::int[])::int AS parent, | ||
| unnest(most_common_freqs) AS freq | ||
| FROM pg_stats | ||
| WHERE schemaname = 'public' AND tablename = 'nodes' AND attname = 'parent'; | ||
| """ | ||
| ) | ||
| ) | ||
| """)) | ||
| for parent, freq in parent_and_freqs: | ||
| if parent == self.node.id: | ||
| total = ( | ||
| await db.execute( | ||
| text( | ||
| """ | ||
| total = (await db.execute(text(""" | ||
| SELECT reltuples::bigint FROM pg_class | ||
| WHERE oid = 'public.nodes'::regclass; | ||
| """ | ||
| ) | ||
| ) | ||
| ).scalar_one() | ||
| """))).scalar_one() | ||
| return int(total * freq) | ||
| else: | ||
| return None # Statistics can not be obtained | ||
|
|
@@ -654,10 +644,12 @@ async def create_node( | |
| specs=None, | ||
| data_sources=None, | ||
| access_blob=None, | ||
| created_by="", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think None (NULL in SQL) may be a more appropriate default than "". |
||
| ): | ||
| access_blob = access_blob or {} | ||
| key = key or self.context.key_maker() | ||
| data_sources = data_sources or [] | ||
| time_created = datetime.now() | ||
|
|
||
| node = orm.Node( | ||
| key=key, | ||
|
|
@@ -666,6 +658,10 @@ async def create_node( | |
| structure_family=structure_family, | ||
| specs=specs or [], | ||
| access_blob=access_blob, | ||
| time_created=time_created, | ||
| time_updated=time_created, | ||
| created_by=created_by, | ||
| updated_by=created_by, | ||
| ) | ||
| async with self.context.session() as db: | ||
| # TODO Consider using nested transitions to ensure that | ||
|
|
@@ -1027,7 +1023,13 @@ async def delete_revision(self, number): | |
| await db.commit() | ||
|
|
||
| async def replace_metadata( | ||
| self, metadata=None, specs=None, access_blob=None, *, drop_revision=False | ||
| self, | ||
| metadata=None, | ||
| specs=None, | ||
| access_blob=None, | ||
| updated_by="", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to |
||
| *, | ||
| drop_revision=False, | ||
| ): | ||
| values = {} | ||
| if metadata is not None: | ||
|
|
@@ -1038,6 +1040,8 @@ async def replace_metadata( | |
| values["specs"] = specs | ||
| if access_blob is not None: | ||
| values["access_blob"] = access_blob | ||
| values["time_updated"] = datetime.now() | ||
| values["updated_by"] = updated_by | ||
| async with self.context.session() as db: | ||
| if not drop_revision: | ||
| current = ( | ||
|
|
@@ -1063,6 +1067,8 @@ async def replace_metadata( | |
| access_blob=current.access_blob, | ||
| node_id=current.id, | ||
| revision_number=next_revision_number, | ||
| time_updated=current.time_updated, | ||
| updated_by=current.updated_by, | ||
| ) | ||
| db.add(revision) | ||
| await db.execute( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.