Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 21 additions & 4 deletions api/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import psycopg2
from django.conf import settings as django_conf_settings
from django.db.models import Func, Value
from django.utils import timezone
from omi.base import get_metadata_version
from omi.conversion import convert_metadata
from omi.validation import ValidationError, parse_metadata, validate_metadata
Expand Down Expand Up @@ -755,6 +756,8 @@ def column_alter(query, table_obj: Table, column):
+ read_pgid(query["name"])
).format(schema=table_obj.oedb_schema, table=table_obj.name, column=column)
perform_sql(sql)
if {"data_type", "is_nullable", "column_default", "name"} & set(query):
table_obj.stamp_data_modified()
return get_response_dict(success=True)


Expand All @@ -779,6 +782,7 @@ def column_add(table_obj: Table, column, description):
perform_sql(s.format(schema=edit_sa_table.schema, table=edit_sa_table.name))
perform_sql(s.format(schema=insert_sa_table.schema, table=insert_sa_table.name))

table_obj.stamp_data_modified()
return get_response_dict(success=True)


Expand Down Expand Up @@ -876,7 +880,10 @@ def table_change_column(column_definition):

sql_string = "".join(sql)

return perform_sql(sql_string)
result = perform_sql(sql_string)
if sql:
table_obj.stamp_data_modified()
return result


def table_change_constraint(constraint_definition):
Expand All @@ -897,6 +904,7 @@ def table_change_constraint(constraint_definition):

# There is a table named schema.table.
sql = []
changed = False

if "ADD" in get_or_403(constraint_definition, "action"):
ctype = get_or_403(constraint_definition, "constraint_type").lower()
Expand Down Expand Up @@ -925,6 +933,7 @@ def table_change_constraint(constraint_definition):
raise APIError("Not supported")
# FIXME: check permissions
constraint.create(_get_engine())
changed = True
elif "DROP" in constraint_definition["action"]:
sql.append(
'ALTER TABLE "{schema}"."{table}" DROP CONSTRAINT "{constraint_name}"'.format( # noqa
Expand All @@ -936,7 +945,10 @@ def table_change_constraint(constraint_definition):

sql_string = "".join(sql)

return perform_sql(sql_string)
result = perform_sql(sql_string)
if changed or sql:
table_obj.stamp_data_modified()
return result


"""
Expand Down Expand Up @@ -1399,6 +1411,8 @@ def add_type(d, type):
_apply_stack(cursor, sa_table, change_batch, prev_type)
if artificial_connection:
connection.commit()
if changes:
table_obj.stamp_data_modified()
except Exception:
if artificial_connection:
connection.rollback()
Expand Down Expand Up @@ -1497,7 +1511,8 @@ def set_table_metadata(table: str, metadata):
"""saves metadata as json string on table comment.

The one metadata write path: it also recomputes the stored Publish gate
verdict (``Table.publishable``) in the same save.
verdict (``Table.publishable``) and stamps ``Table.metadata_modified``,
both in the same save.

Args:
table(str): name of table
Expand Down Expand Up @@ -1525,8 +1540,10 @@ def set_table_metadata(table: str, metadata):

django_table_obj = Table.objects.get(name=table)
django_table_obj.oemetadata = metadata_obj # type: ignore
# the Publish gate's verdict on the metadata just written, saved with it
# the Publish gate's verdict on the metadata just written, and when it
# was written, saved with it
django_table_obj.publishable = is_publishable(django_table_obj)
django_table_obj.metadata_modified = timezone.now()
django_table_obj.save()

# ---------------------------------------
Expand Down
240 changes: 240 additions & 0 deletions api/tests/test_modification_stamps.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,240 @@
"""
SPDX-FileCopyrightText: 2026 Jonas Huber <https://github.com/jh-RLI> © Reiner Lemoine Institut
SPDX-License-Identifier: AGPL-3.0-or-later

When a Table's content last changed (#2557, spec #2551): every Modification
stamps one of two halves, ``data_modified`` or ``metadata_modified``, and
status changes stamp neither. Driven through the write paths a client uses,
and asserted on what the database holds afterwards.

The queued column and constraint changes are the one exception: the queue's
own apply (``apply_queued_column``) raises before it reaches the change
(``get_column_changes`` reads a column ``api_columns`` does not have, #2490),
so no request can get there. Its two writers are called directly instead.
""" # noqa: 501

from copy import deepcopy

from django.urls import reverse
from django.utils import timezone
from oemetadata.v2.v20.example import OEMETADATA_V20_EXAMPLE

from api.actions import table_change_column, table_change_constraint
from api.error import APIError
from api.tests import APITestCaseWithTable
from dataedit.models import Table
from oeplatform.settings import TOPIC_SCENARIO


class StampTestCase(APITestCaseWithTable):
test_table = "test_table_modification_stamps"

def setUp(self):
self.started = timezone.now()
super().setUp()

def stamps(self):
table = Table.objects.get(name=self.test_table)
return table.data_modified, table.metadata_modified

def clear(self):
"""Forget every stamp, so the next write is the only one that can
have set one."""
Table.objects.filter(name=self.test_table).update(
data_modified=None, metadata_modified=None
)

def assertJustNow(self, stamp):
self.assertIsNotNone(stamp)
self.assertTrue(self.started <= stamp <= timezone.now(), stamp)

def assertStamped(self, data=False, metadata=False):
data_modified, metadata_modified = self.stamps()
for stamped, stamp in ((data, data_modified), (metadata, metadata_modified)):
if stamped:
self.assertJustNow(stamp)
else:
self.assertIsNone(stamp)

def write_rows(self):
self.api_req(
"post",
path="rows/new",
data={"query": [{"id": 1, "name": "one"}]},
exp_code=201,
)


class MetadataHalfTests(StampTestCase):
def test_creating_a_table_stamps_its_metadata_only(self):
"""Creation writes metadata (the template, if none was sent) through
the one metadata write path; it writes no data."""
self.assertStamped(metadata=True)

def test_a_metadata_write_stamps_the_metadata_half(self):
self.clear()
self.api_req("post", path="meta/", data=deepcopy(OEMETADATA_V20_EXAMPLE))
self.assertStamped(metadata=True)

def test_the_stamp_moves_with_every_metadata_write(self):
self.api_req("post", path="meta/", data=deepcopy(OEMETADATA_V20_EXAMPLE))
_, first = self.stamps()
self.api_req("post", path="meta/", data=deepcopy(OEMETADATA_V20_EXAMPLE))
_, second = self.stamps()
self.assertGreater(second, first)

def test_refused_metadata_stamps_nothing(self):
self.clear()
self.api_req(
"post", path="meta/", data={"resources": "not a list"}, exp_code=400
)
self.assertStamped()


class DataHalfTests(StampTestCase):
def test_applying_new_rows_stamps_the_data_half(self):
self.clear()
self.write_rows()
self.assertStamped(data=True)

def test_deleting_a_row_stamps_the_data_half(self):
self.write_rows()
self.clear()
self.api_req("delete", path="rows/1")
self.assertStamped(data=True)

def test_a_bulk_upload_stamps_the_data_half(self):
self.clear()
response = self.client.post(
f"/api/v0/tables/{self.test_table}/bulk-upload/?delimiter=comma",
data=b"id,name\n1,one\n2,two\n",
content_type="text/csv",
HTTP_AUTHORIZATION=f"Token {self.token}",
)
self.assertEqual(response.status_code, 201, response.content)
self.assertStamped(data=True)

def test_a_failed_bulk_upload_stamps_nothing(self):
self.clear()
response = self.client.post(
f"/api/v0/tables/{self.test_table}/bulk-upload/?delimiter=comma",
data=b"id,no_such_column\n1,one\n",
content_type="text/csv",
HTTP_AUTHORIZATION=f"Token {self.token}",
)
self.assertEqual(response.status_code, 400, response.content)
self.assertStamped()

def test_adding_a_column_stamps_the_data_half(self):
self.clear()
self.api_req(
"put",
path="columns/added",
data={"query": {"data_type": "varchar", "character_maximum_length": 30}},
exp_code=201,
)
self.assertStamped(data=True)

def test_altering_a_column_stamps_the_data_half(self):
self.clear()
self.api_req(
"post",
path="columns/name",
data={"query": {"data_type": "text"}},
)
self.assertStamped(data=True)

def test_an_alteration_naming_nothing_to_alter_stamps_nothing(self):
self.clear()
self.api_req("post", path="columns/name", data={"query": {"comment": "x"}})
self.assertStamped()

def test_a_queued_column_change_stamps_the_data_half(self):
"""A new column: the queue's alter branch cannot run on any existing
column (it reads ``is_nullable`` as text, and it is a bool)."""
self.clear()
result = table_change_column(
{
"c_table": self.test_table,
"column_name": "queued",
"new_name": None,
"data_type": "text",
}
)
self.assertTrue(result["success"])
self.assertStamped(data=True)

def test_a_queued_change_that_fails_stamps_nothing(self):
self.clear()
with self.assertRaises(APIError):
table_change_column(
{
"c_table": self.test_table,
"column_name": "queued",
"new_name": None,
"data_type": "no_such_type",
}
)
self.assertStamped()

def test_a_queued_constraint_change_stamps_the_data_half(self):
self.clear()
result = table_change_constraint(
{
"c_table": self.test_table,
"action": "DROP",
"constraint_name": f"{self.test_table}_pkey",
}
)
self.assertTrue(result["success"])
self.assertStamped(data=True)


class StatusChangesStampNothingTests(StampTestCase):
"""Publish, unpublish, embargo and role changes are status, not content:
each has its own column, and counting them would move a Table up the
list for something nobody changed in it."""

def setUp(self):
super().setUp()
# the license the Publish gate wants, written before the stamps go
self.api_req("post", path="meta/", data=deepcopy(OEMETADATA_V20_EXAMPLE))
self.clear()

def publish(self, embargo="none"):
self.api_req(
"post",
path=f"move_publish/{TOPIC_SCENARIO}/",
data={"embargo": {"duration": embargo}},
)

def test_publishing(self):
self.publish()
self.assertTrue(Table.objects.get(name=self.test_table).is_publish)
self.assertStamped()

def test_publishing_under_an_embargo(self):
self.publish(embargo="6_months")
self.assertTrue(Table.objects.get(name=self.test_table).embargos.exists())
self.assertStamped()

def test_unpublishing(self):
self.publish()
self.clear()
self.api_req("post", path="unpublish")
self.assertFalse(Table.objects.get(name=self.test_table).is_publish)
self.assertStamped()

def test_granting_a_role(self):
self.client.force_login(self.user)
response = self.client.post(
reverse("dataedit:table-permission", kwargs={"table": self.test_table}),
{"mode": "add_user", "name": self.other_user.name},
)
self.assertEqual(response.status_code, 200)
self.assertTrue(
Table.objects.get(name=self.test_table)
.userpermission_set.filter(holder=self.other_user)
.exists()
)
self.assertStamped()
1 change: 1 addition & 0 deletions api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1848,6 +1848,7 @@ def post(self, request: Request, table: str) -> JsonLikeResponse:
)
raise

table_obj.stamp_data_modified()
event = _record_bulk_load_event(
table_obj,
request.user,
Expand Down
27 changes: 23 additions & 4 deletions benchmarks/tables_tab/seed.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,14 @@
under embargo, 15-20 % have no title, and Topics sit mostly on published
Tables. Deterministic for a given account key.

Only the columns that exist are seeded (no Modified or Created yet). Every
Table carries ``metadata_kb`` of oemetadata, because the page query decodes
each row's whole document for the live Publish gate; real documents range
from a few KB to several hundred.
Modified is seeded as it will look once #2558 has backfilled the data half:
about 15 % unknown, 45 % data only (marked "data"), 40 % both halves. The
stamps come from their own random stream, so the rest of an account is
seeded as before. Created is not seeded yet.

Every Table carries ``metadata_kb`` of oemetadata, because the page query
decodes each row's whole document for the live Publish gate; real documents
range from a few KB to several hundred.

Used by ``run.py`` and, for the browser check, from ``manage.py shell``::

Expand Down Expand Up @@ -295,6 +299,20 @@ def user(name):
)

now = timezone.now()
stamps = random.Random(f"{key}-{account.n}-modified")

def modified():
"""``(data_modified, metadata_modified)``, in the mix above."""
roll = stamps.random()
if roll < 0.15:
return None, None
data = now - timedelta(
days=stamps.randint(0, 900), minutes=stamps.randint(0, 1439)
)
if roll < 0.6:
return data, None
return data, now - timedelta(days=stamps.randint(0, 30))

plans = []
for _ in range(account.n):
while True:
Expand Down Expand Up @@ -348,6 +366,7 @@ def user(name):
),
is_publish=published,
oemetadata=metadata(license_name, metadata_kb),
**dict(zip(("data_modified", "metadata_modified"), modified())),
),
embargo=embargo,
review=review,
Expand Down
Loading
Loading