Skip to content
Open
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
2 changes: 1 addition & 1 deletion schema/snapcraft.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
],
"default": null,
"description": "The identifier to a desktop ID within an external appstream file.",
"examples": ["org.canonical.foo"],
"examples": ["org.canonical.foo", "org.kde.ghostwriter", "io.github.user.project"],
"title": "Common-Id"
},
"bus-name": {
Expand Down
24 changes: 21 additions & 3 deletions snapcraft/models/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -452,11 +452,15 @@ class App(models.CraftBaseModel):
common_id: str | None = pydantic.Field(
default=None,
description="The identifier to a desktop ID within an external appstream file.",
examples=["org.canonical.foo"],
examples=["org.canonical.foo", "org.kde.ghostwriter", "io.github.user.project"],
)
"""The identifier to a desktop ID within an external appstream file.
"""The unique identifier for a project.

See :ref:`how-to-configure-package-information` for details.
The common-id must be in reverse DNS format and consist only of alphanumeric characters and the
following special characters: ``., _, -``.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
following special characters: ``., _, -``.
following special characters: ``.``, ``_``, ``-``.


See :ref:`how-to-configure-package-information` and the `Appstream specification
<https://www.freedesktop.org/software/appstream/docs/sect-Metadata-Application.html#tag-id-desktopapp>`_ for more information.
"""

bus_name: str | None = pydantic.Field(
Expand Down Expand Up @@ -1024,6 +1028,20 @@ def _validate_extensions(cls, extensions: list[str]) -> list[str]:
)
return extensions

@pydantic.field_validator("common_id")
Copy link
Collaborator

Choose a reason for hiding this comment

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

@lengau, please correct me here, but I believe the preferred way to add new regex validation is like this:

https://github.com/canonical/craft-application/blob/cfd4558920c6ac936d642059b26c9693b0b45828/craft_application/models/constraints.py#L162-L197

@classmethod
def _validate_common_id(cls, common_id: str) -> str:
common_id_re = re.compile(r"^[a-zA-Z][a-zA-Z0-9_]*(\.[a-zA-Z][a-zA-Z0-9_]*)+$")
if not common_id_re.match(common_id):
raise ValueError(
f"{common_id!r} is not a valid common id. Common IDs must be"
" reverse DNS ids. Checkout the freedesktop specs for more info"
" https://www.freedesktop.org/software/appstream/docs/"
"sect-Metadata-Application.html#tag-id-desktopapp"
)

return common_id

@pydantic.field_validator("success_exit_status")
@classmethod
def _validate_success_exit_status(
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/meta/test_snap_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ def complex_project():
app1:
command: bin/mytest
autostart: test-app.desktop
common-id: test-common-id
common-id: org.test.app
bus-name: test-bus-name
completer: test-completer
stop-command: test-stop-command
Expand Down Expand Up @@ -470,7 +470,7 @@ def test_complex_snap_yaml(complex_project, new_dir):
app1:
command: bin/mytest
autostart: test-app.desktop
common-id: test-common-id
common-id: org.test.app
bus-name: test-bus-name
completer: test-completer
stop-command: test-stop-command
Expand Down
14 changes: 11 additions & 3 deletions tests/unit/models/test_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -1014,11 +1014,19 @@ def test_app_autostart(self, autostart, app_yaml_data):
with pytest.raises(pydantic.ValidationError, match=error):
Project.unmarshal(data)

def test_app_common_id(self, app_yaml_data):
data = app_yaml_data(common_id="test-common-id")
def test_app_valid_common_id(self, app_yaml_data):
Copy link
Collaborator

Choose a reason for hiding this comment

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

To exercise the regex, can you do a pytest.mark.parameterize for this test and the next test to test a few valid and invalid common ids?

And can you add id's for the invalid ones to explain them? (for example, @pytest.param("te$t", id="invalid-character"))

data = app_yaml_data(common_id="org.test.app")
project = Project.unmarshal(data)
assert project.apps is not None
assert project.apps["app1"].common_id == "test-common-id"
assert project.apps["app1"].common_id == "org.test.app"

def test_app_invalid_common_id(self, app_yaml_data):
data = app_yaml_data(common_id="_invalid")
error = (
"apps.app1.common_id\n Value error, '_invalid' is not a valid common id"
)
with pytest.raises(pydantic.ValidationError, match=error):
Project.unmarshal(data)

def test_app_completer(self, app_yaml_data):
data = app_yaml_data(completer="test-completer")
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/parts/test_setup_assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ def test_setup_assets_happy(self, desktop_file, yaml_data, new_dir):
"apps": {
"app1": {
"command": "test.sh",
"common-id": "my-test",
"common-id": "my.test.sh",
"desktop": "test.desktop",
},
},
Expand Down Expand Up @@ -365,7 +365,7 @@ def test_setup_assets_icon_in_assets_dir(self, desktop_file, yaml_data, new_dir)
"apps": {
"app1": {
"command": "test.sh",
"common-id": "my-test",
"common-id": "my.test.sh",
"desktop": "test.desktop",
},
},
Expand Down Expand Up @@ -435,7 +435,7 @@ def test_setup_assets_remote_icon(self, desktop_file, yaml_data, new_dir):
"apps": {
"app1": {
"command": "test.sh",
"common-id": "my-test",
"common-id": "my.test.sh",
"desktop": "test.desktop",
},
},
Expand Down
Loading