Skip to content

Commit 42a1b55

Browse files
nhoeningBelhsanHmidaFlix6x
authored
fix: allow root assets to have non-unique names (#2226)
* fix: allow root assets to have non-unique names Signed-off-by: Nicolas Höning <nicolas@seita.nl> * fix: define account-scoped root asset index Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com> * fix: migrate account-scoped root asset index Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com> * fix: validate root asset names by account Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com> * test: cover root asset model uniqueness Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com> * test: cover root asset API uniqueness Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com> * test: avoid duplicate planning root asset name Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com> * Update documentation/changelog.rst Co-authored-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com> Signed-off-by: Mohamed Belhsan Hmida <149331360+BelhsanHmida@users.noreply.github.com> * fix: reuse toy root assets by account The Docker build workflow runs `flexmeasures add toy-account` first and later runs `flexmeasures add toy-account --kind process` against the same database. After adding account-scoped uniqueness for root asset names, the second command could no longer insert another top-level `toy-building` for the same account. Look up toy assets by the same identity that the database enforces instead: root assets by account/name and child assets by parent/name. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com> * test: cover repeated toy account setup Cover the Docker tutorial sequence where the battery toy account is created before extending the same account with process assets. The regression checks that the second command reuses the existing root `toy-building`, keeps one root `toy-process`, and still adds the expected process sensors. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com> * Update documentation/changelog.rst Co-authored-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com> Signed-off-by: Mohamed Belhsan Hmida <149331360+BelhsanHmida@users.noreply.github.com> * fix: enforce unique public root asset names Public root assets do not belong to an account namespace, so duplicate names there would share one global public namespace. Add a separate partial unique index for public root assets and validate that case in the asset schema instead of exempting account_id=None roots. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com> * test: cover public root asset name uniqueness Update the model-level duplicate public root asset test to expect the new global uniqueness rule. Add API coverage so duplicate public root names are rejected by schema validation before reaching the database. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com> --------- Signed-off-by: Nicolas Höning <nicolas@seita.nl> Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com> Signed-off-by: Mohamed Belhsan Hmida <149331360+BelhsanHmida@users.noreply.github.com> Co-authored-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com> Co-authored-by: Mohamed Belhsan Hmida <149331360+BelhsanHmida@users.noreply.github.com> Co-authored-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com>
1 parent f4da66d commit 42a1b55

10 files changed

Lines changed: 459 additions & 49 deletions

File tree

documentation/changelog.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ Infrastructure / Support
2121

2222
Bugfixes
2323
-----------
24+
* Allow root assets belonging to different accounts to share the same name, while keeping asset names unique among root assets within the same account and among children of the same parent [see `PR #2226 <https://www.github.com/FlexMeasures/flexmeasures/pull/2226>`_]
2425

2526

2627
v0.33.0 | June 1, 2026

flexmeasures/api/v3_0/tests/test_assets_api.py

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -614,24 +614,31 @@ def test_consultancy_user_without_consultant_role(
614614

615615

616616
@pytest.mark.parametrize(
617-
"parent_name, child_name, fails",
617+
"parent_name, child_name, pre_existing_root, fails",
618618
[
619-
("parent", "child_4", False),
620-
(None, "child_1", False),
621-
(None, "child_1", True),
622-
("parent", "child_1", True),
619+
("parent", "child_4", False, False),
620+
(None, "child_1", False, False),
621+
(None, "duplicate_root_name", True, True),
622+
("parent", "child_1", False, True),
623623
],
624624
)
625625
@pytest.mark.parametrize("requesting_user", ["test_admin_user@seita.nl"], indirect=True)
626626
def test_post_an_asset_with_existing_name(
627-
client, add_asset_with_children, parent_name, child_name, fails, requesting_user, db
627+
client,
628+
add_asset_with_children,
629+
parent_name,
630+
child_name,
631+
pre_existing_root,
632+
fails,
633+
requesting_user,
634+
db,
628635
):
629636
"""Catch DB error (Unique key violated) correctly.
630637
631638
Cases:
632639
1) Create a child asset
633640
2) Create an orphan asset with a name that already exists under a parent asset
634-
3) Create an orphan asset with an existing name.
641+
3) Create an orphan asset with an existing root asset name in the same account.
635642
4) Create a child asset with a name that already exists among its siblings.
636643
"""
637644

@@ -650,10 +657,22 @@ def get_asset_by_name(asset_name):
650657
post_data["account_id"] = requesting_user.account_id
651658

652659
if parent:
653-
post_data["parent_asset_id"] = parent.parent_asset_id
660+
post_data["parent_asset_id"] = parent.id
654661
else:
655662
post_data["parent_asset_id"] = None
656663

664+
if pre_existing_root:
665+
db.session.add(
666+
GenericAsset(
667+
name=child_name,
668+
generic_asset_type=add_asset_with_children[
669+
"child_1"
670+
].generic_asset_type,
671+
account_id=requesting_user.account_id,
672+
)
673+
)
674+
db.session.flush()
675+
657676
asset_creation_response = client.post(
658677
url_for("AssetAPI:post"),
659678
json=post_data,

flexmeasures/api/v3_0/tests/test_assets_api_fresh_db.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,64 @@ def test_post_an_asset_as_admin(client, setup_api_fresh_test_data, requesting_us
4343
assert asset.latitude == 30.1
4444

4545

46+
@pytest.mark.parametrize("requesting_user", ["test_admin_user@seita.nl"], indirect=True)
47+
def test_post_root_asset_allows_existing_name_in_different_account(
48+
client, setup_api_fresh_test_data, requesting_user, fresh_db
49+
):
50+
with AccountContext("Test Prosumer Account") as prosumer:
51+
existing_asset = prosumer.generic_assets[0]
52+
existing_name = existing_asset.name
53+
asset_type_id = existing_asset.generic_asset_type_id
54+
55+
with AccountContext("Test Supplier Account") as supplier:
56+
post_data = get_asset_post_data(
57+
account_id=supplier.id,
58+
asset_type_id=asset_type_id,
59+
)
60+
post_data["name"] = existing_name
61+
62+
response = client.post(
63+
url_for("AssetAPI:post"),
64+
json=post_data,
65+
)
66+
67+
assert response.status_code == 201
68+
assert response.json["name"] == existing_name
69+
assert response.json["account_id"] == post_data["account_id"]
70+
assert response.json["parent_asset_id"] is None
71+
72+
73+
@pytest.mark.parametrize("requesting_user", ["test_admin_user@seita.nl"], indirect=True)
74+
def test_post_public_root_asset_rejects_existing_name(
75+
client, setup_api_fresh_test_data, requesting_user, fresh_db
76+
):
77+
db = fresh_db
78+
with AccountContext("Test Prosumer Account") as prosumer:
79+
asset_type_id = prosumer.generic_assets[0].generic_asset_type_id
80+
81+
existing_name = "Existing public root asset"
82+
db.session.add(
83+
GenericAsset(
84+
name=existing_name,
85+
generic_asset_type_id=asset_type_id,
86+
account_id=None,
87+
)
88+
)
89+
db.session.flush()
90+
91+
post_data = get_asset_post_data(asset_type_id=asset_type_id)
92+
post_data["name"] = existing_name
93+
post_data.pop("account_id")
94+
95+
response = client.post(
96+
url_for("AssetAPI:post"),
97+
json=post_data,
98+
)
99+
100+
assert response.status_code == 422
101+
assert "already exists" in response.json["message"]["json"]["name"][0]
102+
103+
46104
@pytest.mark.parametrize("requesting_user", ["test_admin_user@seita.nl"], indirect=True)
47105
def test_edit_an_asset(client, setup_api_fresh_test_data, requesting_user, db):
48106
with AccountContext("Test Supplier Account") as supplier:
@@ -62,6 +120,34 @@ def test_edit_an_asset(client, setup_api_fresh_test_data, requesting_user, db):
62120
assert updated_asset.name == existing_asset.name
63121

64122

123+
@pytest.mark.parametrize("requesting_user", ["test_admin_user@seita.nl"], indirect=True)
124+
def test_patch_root_asset_rejects_existing_name_in_same_account(
125+
client, setup_api_fresh_test_data, requesting_user, fresh_db
126+
):
127+
db = fresh_db
128+
with AccountContext("Test Prosumer Account") as prosumer:
129+
existing_asset = prosumer.generic_assets[0]
130+
existing_name = existing_asset.name
131+
asset_type_id = existing_asset.generic_asset_type_id
132+
account_id = prosumer.id
133+
134+
other_asset = GenericAsset(
135+
name="Root asset to rename",
136+
generic_asset_type_id=asset_type_id,
137+
account_id=account_id,
138+
)
139+
db.session.add(other_asset)
140+
db.session.flush()
141+
142+
response = client.patch(
143+
url_for("AssetAPI:patch", id=other_asset.id),
144+
json={"name": existing_name},
145+
)
146+
147+
assert response.status_code == 422
148+
assert "already exists" in response.json["message"]["json"]["name"][0]
149+
150+
65151
@pytest.mark.parametrize("requesting_user", ["test_admin_user@seita.nl"], indirect=True)
66152
def test_delete_an_asset(client, setup_api_fresh_test_data, requesting_user, db):
67153
with AccountContext("Test Prosumer Account") as prosumer:

flexmeasures/cli/data_add.py

Lines changed: 64 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1675,6 +1675,53 @@ def launch_editor(filename: str) -> dict:
16751675
return content
16761676

16771677

1678+
def get_or_create_toy_asset(
1679+
asset_name: str,
1680+
asset_type: str,
1681+
asset_types: dict[str, GenericAssetType],
1682+
account_owner: Account,
1683+
location: tuple[float, float],
1684+
parent_asset_id: int | None = None,
1685+
flex_context: dict | None = None,
1686+
flex_model: dict | None = None,
1687+
**asset_attributes,
1688+
) -> GenericAsset:
1689+
asset_query = select(GenericAsset).filter_by(
1690+
name=asset_name,
1691+
account_id=account_owner.id,
1692+
)
1693+
if parent_asset_id is None:
1694+
asset_query = asset_query.filter(GenericAsset.parent_asset_id.is_(None))
1695+
else:
1696+
asset_query = asset_query.filter_by(parent_asset_id=parent_asset_id)
1697+
asset = db.session.execute(asset_query).scalar_one_or_none()
1698+
1699+
if asset is not None:
1700+
return asset
1701+
1702+
asset_kwargs: Dict[str, Any] = {}
1703+
if parent_asset_id is not None:
1704+
asset_kwargs["parent_asset_id"] = parent_asset_id
1705+
if flex_context is not None:
1706+
asset_kwargs["flex_context"] = flex_context
1707+
if flex_model is not None:
1708+
asset_kwargs["flex_model"] = flex_model
1709+
1710+
asset = GenericAsset(
1711+
name=asset_name,
1712+
generic_asset_type=asset_types[asset_type],
1713+
owner=account_owner,
1714+
latitude=location[0],
1715+
longitude=location[1],
1716+
attributes=asset_attributes,
1717+
**asset_kwargs,
1718+
)
1719+
db.session.add(asset)
1720+
db.session.flush()
1721+
click.echo(f"Created {repr(asset)}")
1722+
return asset
1723+
1724+
16781725
@fm_add_data.command("toy-account")
16791726
@with_appcontext
16801727
@click.option(
@@ -1745,6 +1792,7 @@ def add_toy_account(kind: str, name: str):
17451792
)
17461793

17471794
account_id = user.account_id
1795+
account_owner = db.session.get(Account, account_id)
17481796

17491797
def create_asset_with_one_sensor(
17501798
asset_name: str,
@@ -1756,23 +1804,16 @@ def create_asset_with_one_sensor(
17561804
flex_model: dict | None = None,
17571805
**asset_attributes,
17581806
):
1759-
asset_kwargs: Dict[str, Any] = {}
1760-
if parent_asset_id is not None:
1761-
asset_kwargs["parent_asset_id"] = parent_asset_id
1762-
if flex_context is not None:
1763-
asset_kwargs["flex_context"] = flex_context
1764-
if flex_model is not None:
1765-
asset_kwargs["flex_model"] = flex_model
1766-
1767-
asset = get_or_create_model(
1768-
GenericAsset,
1769-
name=asset_name,
1770-
generic_asset_type=asset_types[asset_type],
1771-
owner=db.session.get(Account, account_id),
1772-
latitude=location[0],
1773-
longitude=location[1],
1774-
attributes=asset_attributes,
1775-
**asset_kwargs,
1807+
asset = get_or_create_toy_asset(
1808+
asset_name=asset_name,
1809+
asset_type=asset_type,
1810+
asset_types=asset_types,
1811+
account_owner=account_owner,
1812+
location=location,
1813+
parent_asset_id=parent_asset_id,
1814+
flex_context=flex_context,
1815+
flex_model=flex_model,
1816+
**asset_attributes,
17761817
)
17771818

17781819
sensor_specs = dict(
@@ -1789,13 +1830,12 @@ def create_asset_with_one_sensor(
17891830
return sensor
17901831

17911832
# create building asset
1792-
building_asset = get_or_create_model(
1793-
GenericAsset,
1794-
name="toy-building",
1795-
generic_asset_type=asset_types["building"],
1796-
owner=db.session.get(Account, account_id),
1797-
latitude=location[0],
1798-
longitude=location[1],
1833+
building_asset = get_or_create_toy_asset(
1834+
asset_name="toy-building",
1835+
asset_type="building",
1836+
asset_types=asset_types,
1837+
account_owner=account_owner,
1838+
location=location,
17991839
flex_context={
18001840
"site-power-capacity": "500 kVA",
18011841
"consumption-price": {"sensor": day_ahead_sensor.id},

flexmeasures/cli/tests/test_data_add_fresh_db.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,51 @@ def test_add_account(
374374
assert result.exit_code == 1
375375

376376

377+
def test_add_process_toy_account_reuses_existing_root_assets(app, fresh_db):
378+
from flexmeasures.cli.data_add import add_toy_account
379+
380+
runner = app.test_cli_runner()
381+
result = runner.invoke(add_toy_account)
382+
assert result.exit_code == 0, result.output
383+
384+
result = runner.invoke(add_toy_account, ["--kind", "process"])
385+
assert result.exit_code == 0, result.output
386+
387+
toy_account = fresh_db.session.execute(
388+
select(Account).filter_by(name="Toy Account")
389+
).scalar_one()
390+
root_buildings = (
391+
fresh_db.session.execute(
392+
select(Asset).filter_by(
393+
name="toy-building",
394+
owner=toy_account,
395+
parent_asset_id=None,
396+
)
397+
)
398+
.scalars()
399+
.all()
400+
)
401+
root_processes = (
402+
fresh_db.session.execute(
403+
select(Asset).filter_by(
404+
name="toy-process",
405+
owner=toy_account,
406+
parent_asset_id=None,
407+
)
408+
)
409+
.scalars()
410+
.all()
411+
)
412+
413+
assert len(root_buildings) == 1
414+
assert len(root_processes) == 1
415+
assert {sensor.name for sensor in root_processes[0].sensors} == {
416+
"Power (Inflexible)",
417+
"Power (Breakable)",
418+
"Power (Shiftable)",
419+
}
420+
421+
377422
@pytest.mark.parametrize("storage_power_capacity", ["sensor", "quantity", None])
378423
@pytest.mark.parametrize("storage_efficiency", ["sensor", "quantity", None])
379424
def test_add_storage_schedule(
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
"""Allow duplicate root asset names.
2+
3+
Revision ID: c7b7f9019d4b
4+
Revises: b2c3d4e5f6a7
5+
Create Date: 2026-06-05 12:00:00.000000
6+
7+
"""
8+
9+
from alembic import op
10+
import sqlalchemy as sa
11+
12+
13+
# revision identifiers, used by Alembic.
14+
revision = "c7b7f9019d4b"
15+
down_revision = "b2c3d4e5f6a7"
16+
branch_labels = None
17+
depends_on = None
18+
19+
20+
def upgrade():
21+
op.drop_constraint(
22+
"generic_asset_name_parent_asset_id_key", "generic_asset", type_="unique"
23+
)
24+
op.create_index(
25+
"generic_asset_name_parent_asset_id_key",
26+
"generic_asset",
27+
["name", "parent_asset_id"],
28+
unique=True,
29+
postgresql_where=sa.text("parent_asset_id IS NOT NULL"),
30+
sqlite_where=sa.text("parent_asset_id IS NOT NULL"),
31+
)
32+
op.create_index(
33+
"generic_asset_root_account_id_name_key",
34+
"generic_asset",
35+
["account_id", "name"],
36+
unique=True,
37+
postgresql_where=sa.text("parent_asset_id IS NULL AND account_id IS NOT NULL"),
38+
sqlite_where=sa.text("parent_asset_id IS NULL AND account_id IS NOT NULL"),
39+
)
40+
op.create_index(
41+
"generic_asset_public_root_name_key",
42+
"generic_asset",
43+
["name"],
44+
unique=True,
45+
postgresql_where=sa.text("parent_asset_id IS NULL AND account_id IS NULL"),
46+
sqlite_where=sa.text("parent_asset_id IS NULL AND account_id IS NULL"),
47+
)
48+
49+
50+
def downgrade():
51+
op.drop_index("generic_asset_public_root_name_key", table_name="generic_asset")
52+
op.drop_index("generic_asset_root_account_id_name_key", table_name="generic_asset")
53+
op.drop_index("generic_asset_name_parent_asset_id_key", table_name="generic_asset")
54+
op.create_unique_constraint(
55+
"generic_asset_name_parent_asset_id_key",
56+
"generic_asset",
57+
["name", "parent_asset_id"],
58+
)

0 commit comments

Comments
 (0)