From 48f432c49cf02dce679ec8f2b085d81ca0ddf0e3 Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 26 Feb 2024 11:16:33 +0000 Subject: [PATCH 1/9] Add Environment.add_tag() method --- softpack_core/artifacts.py | 13 ++++++-- softpack_core/schemas/environment.py | 46 ++++++++++++++++++++++++++- softpack_core/service.py | 6 +++- tests/integration/test_environment.py | 39 +++++++++++++++++++++-- 4 files changed, 97 insertions(+), 7 deletions(-) diff --git a/softpack_core/artifacts.py b/softpack_core/artifacts.py index 987f3bb..b7ae4bd 100644 --- a/softpack_core/artifacts.py +++ b/softpack_core/artifacts.py @@ -13,6 +13,7 @@ import pygit2 import strawberry +import yaml from box import Box from fastapi import UploadFile @@ -72,6 +73,7 @@ class Artifacts: builder_out = "builder.out" module_file = "module" readme_file = "README.md" + meta_file = "meta.yml" built_by_softpack_file = ".built_by_softpack" built_by_softpack = Type.softpack.value generated_from_module_file = ".generated_from_module" @@ -154,6 +156,11 @@ def spec(self) -> Box: map(lambda p: Package.from_name(p), info.packages) ) + meta = Box() + if Artifacts.meta_file in self.obj: + meta = Box.from_yaml(self.obj[Artifacts.meta_file].data) + info["tags"] = getattr(meta, "tags", []) + return info def __iter__(self) -> Iterator["Artifacts.Object"]: @@ -322,7 +329,7 @@ def iter(self) -> Iterable: return itertools.chain.from_iterable(map(self.environments, folders)) - def get(self, path: Path, name: str) -> Optional[pygit2.Tree]: + def get(self, path: Path, name: str) -> Optional[Object]: """Return the environment at the specified name and path. Args: @@ -330,10 +337,10 @@ def get(self, path: Path, name: str) -> Optional[pygit2.Tree]: name: the name of the environment folder Returns: - pygit2.Tree: a pygit2.Tree or None + Object: an Object or None """ try: - return self.tree(str(self.environments_folder(str(path), name))) + return self.Object(Path(path, name), self.tree(str(self.environments_folder(str(path), name)))) except KeyError: return None diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index 5e0f9ae..e6ed9f8 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -52,6 +52,11 @@ class UpdateEnvironmentSuccess(Success): """Environment successfully updated.""" +@strawberry.type +class AddTagSuccess(Success): + """Successfully added tag to environment.""" + + @strawberry.type class DeleteEnvironmentSuccess(Success): """Environment successfully deleted.""" @@ -109,6 +114,15 @@ class BuilderError(Error): ], ) +AddTagResponse = strawberry.union( + "AddTagResponse", + [ + AddTagSuccess, + InvalidInputError, + EnvironmentNotFoundError, + ], +) + DeleteResponse = strawberry.union( "DeleteResponse", [ @@ -255,6 +269,7 @@ class Environment: type: Type packages: list[Package] state: Optional[State] + tags: list[str] artifacts = Artifacts() requested: Optional[datetime.datetime] = None @@ -263,7 +278,7 @@ class Environment: avg_wait_secs: Optional[float] = None @classmethod - def iter(cls) -> Iterable["Environment"]: + def iter(cls) -> list["Environment"]: """Get an iterator over all Environment objects. Returns: @@ -323,6 +338,7 @@ def from_artifact(cls, obj: Artifacts.Object) -> Optional["Environment"]: state=spec.state, readme=spec.get("readme", ""), type=spec.get("type", ""), + tags=spec.tags, ) except KeyError: return None @@ -496,6 +512,34 @@ def check_env_exists( name=path.name, ) + @classmethod + def add_tag(cls, name: str, path: str, tag: str) -> AddTagResponse: # type: ignore + environment_path = Path(path, name) + response = cls.check_env_exists(environment_path) + if response is not None: + return response + + if re.fullmatch(r"[a-zA-Z0-9 ]+", tag) is None: + return InvalidInputError(message="Tags must contain only alphanumerics and spaces") + + box = cls.artifacts.get(Path(path), name).spec() + tags = set(box.tags) + if tag in tags: + return AddTagSuccess(message="Tag already present") + tags.add(tag) + + metadata = yaml.dump({"tags": list(tags)}) + tree_oid = cls.artifacts.create_file( + environment_path, + cls.artifacts.meta_file, + metadata, + overwrite=True + ) + cls.artifacts.commit_and_push( + tree_oid, "create environment folder" + ) + return AddTagSuccess(message="Tag successfully added") + @classmethod def delete(cls, name: str, path: str) -> DeleteResponse: # type: ignore """Delete an Environment. diff --git a/softpack_core/service.py b/softpack_core/service.py index e9f11ab..ff38101 100644 --- a/softpack_core/service.py +++ b/softpack_core/service.py @@ -125,4 +125,8 @@ async def resend_pending_builds( # type: ignore[no-untyped-def] response.status_code = 500 message = "Failed to trigger all resends" - return {"message": message, "successes": successes, "failures": failures} + return { + "message": message, + "successes": successes, + "failures": failures, + } diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index 478fd9c..0322fdb 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -1,4 +1,4 @@ -"""Copyright (c) 2023 Genome Research Ltd. +"""Copyright (c) 2023, 2024 Genome Research Ltd. This source code is licensed under the MIT license found in the LICENSE file in the root directory of this source tree. @@ -16,6 +16,7 @@ from softpack_core.artifacts import Artifacts from softpack_core.schemas.environment import ( + AddTagSuccess, BuilderError, CreateEnvironmentSuccess, DeleteEnvironmentSuccess, @@ -469,6 +470,7 @@ async def test_create_from_module(httpx_post, testable_env_input): ) assert isinstance(result, EnvironmentNotFoundError) + def test_environmentinput_from_path(): for path in ( "users/any1/envName", @@ -485,4 +487,37 @@ def test_environmentinput_from_path(): "users/any1/..envName-1", "users/any1/../envName-1.1", ]: - assert EnvironmentInput.from_path(path).validate() is not None \ No newline at end of file + assert EnvironmentInput.from_path(path).validate() is not None + + +def test_tagging(httpx_post, testable_env_input: EnvironmentInput) -> None: + example_env = Environment.iter()[0] + assert example_env.tags == [] + + name, path = example_env.name, example_env.path + result = Environment.add_tag(name, path, tag="test") + assert isinstance(result, AddTagSuccess) + assert result.message == "Tag successfully added" + + result = Environment.add_tag("foo", "users/xyz", tag="test") + assert isinstance(result, EnvironmentNotFoundError) + + result = Environment.add_tag(name, path, tag="../") + assert isinstance(result, InvalidInputError) + + example_env = Environment.iter()[0] + assert len(example_env.tags) == 1 + assert example_env.tags[0] == "test" + + result = Environment.add_tag(name, path, tag="second test") + assert isinstance(result, AddTagSuccess) + + example_env = Environment.iter()[0] + assert list(sorted(example_env.tags)) == list(sorted(["test", "second test"])) + + result = Environment.add_tag(name, path, tag="test") + assert isinstance(result, AddTagSuccess) + assert result.message == "Tag already present" + + example_env = Environment.iter()[0] + assert list(sorted(example_env.tags)) == list(sorted(["test", "second test"])) From 9357a00a0460c08cf1bc8db73281b300f4282aac Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 26 Feb 2024 11:28:05 +0000 Subject: [PATCH 2/9] Fix tests/linting --- softpack_core/artifacts.py | 6 ++-- softpack_core/schemas/environment.py | 40 +++++++++++++++++-------- tests/integration/test_builderupload.py | 4 +-- tests/integration/test_environment.py | 8 +++-- 4 files changed, 40 insertions(+), 18 deletions(-) diff --git a/softpack_core/artifacts.py b/softpack_core/artifacts.py index b7ae4bd..30dc558 100644 --- a/softpack_core/artifacts.py +++ b/softpack_core/artifacts.py @@ -13,7 +13,6 @@ import pygit2 import strawberry -import yaml from box import Box from fastapi import UploadFile @@ -340,7 +339,10 @@ def get(self, path: Path, name: str) -> Optional[Object]: Object: an Object or None """ try: - return self.Object(Path(path, name), self.tree(str(self.environments_folder(str(path), name)))) + return self.Object( + Path(path, name), + self.tree(str(self.environments_folder(str(path), name))), + ) except KeyError: return None diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index e6ed9f8..e896ebe 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -11,7 +11,7 @@ from dataclasses import dataclass from pathlib import Path from traceback import format_exception_only -from typing import Iterable, List, Optional, Tuple, Union, cast +from typing import List, Optional, Tuple, Union, cast import httpx import starlette.datastructures @@ -79,6 +79,7 @@ class EnvironmentNotFoundError(Error): path: str name: str + message: str = "No environment with this path and name found." @strawberry.type @@ -507,22 +508,42 @@ def check_env_exists( return None return EnvironmentNotFoundError( - message="No environment with this path and name found.", path=str(path.parent), name=path.name, ) @classmethod - def add_tag(cls, name: str, path: str, tag: str) -> AddTagResponse: # type: ignore + def add_tag( + cls, name: str, path: str, tag: str + ) -> AddTagResponse: # type: ignore + """Add a tag to an Environment. + + Tags must be composed solely of alphanumerics and spaces. + + Adding a tag that already exists is not an error. + + Args: + name: the name of of environment + path: the path of the environment + tag: the tag to add + + Returns: + A message confirming the success or failure of the operation. + """ environment_path = Path(path, name) response = cls.check_env_exists(environment_path) if response is not None: return response if re.fullmatch(r"[a-zA-Z0-9 ]+", tag) is None: - return InvalidInputError(message="Tags must contain only alphanumerics and spaces") + return InvalidInputError( + message="Tags must contain only alphanumerics and spaces" + ) - box = cls.artifacts.get(Path(path), name).spec() + tree = cls.artifacts.get(Path(path), name) + if tree is None: + return EnvironmentNotFoundError(path=path, name=name) + box = tree.spec() tags = set(box.tags) if tag in tags: return AddTagSuccess(message="Tag already present") @@ -530,14 +551,9 @@ def add_tag(cls, name: str, path: str, tag: str) -> AddTagResponse: # type: ign metadata = yaml.dump({"tags": list(tags)}) tree_oid = cls.artifacts.create_file( - environment_path, - cls.artifacts.meta_file, - metadata, - overwrite=True - ) - cls.artifacts.commit_and_push( - tree_oid, "create environment folder" + environment_path, cls.artifacts.meta_file, metadata, overwrite=True ) + cls.artifacts.commit_and_push(tree_oid, "create environment folder") return AddTagSuccess(message="Tag successfully added") @classmethod diff --git a/tests/integration/test_builderupload.py b/tests/integration/test_builderupload.py index b250177..a752b43 100644 --- a/tests/integration/test_builderupload.py +++ b/tests/integration/test_builderupload.py @@ -54,5 +54,5 @@ def test_builder_upload(testable_env_input): tree = Environment.artifacts.get(env_parent, env_name) assert tree is not None - assert tree[softpackYaml].data == softpackYamlContents - assert tree[spackLock].data == spackLockContents + assert tree.get(softpackYaml).data == softpackYamlContents + assert tree.get(spackLock).data == spackLockContents diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index 0322fdb..de70674 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -513,11 +513,15 @@ def test_tagging(httpx_post, testable_env_input: EnvironmentInput) -> None: assert isinstance(result, AddTagSuccess) example_env = Environment.iter()[0] - assert list(sorted(example_env.tags)) == list(sorted(["test", "second test"])) + assert list(sorted(example_env.tags)) == list( + sorted(["test", "second test"]) + ) result = Environment.add_tag(name, path, tag="test") assert isinstance(result, AddTagSuccess) assert result.message == "Tag already present" example_env = Environment.iter()[0] - assert list(sorted(example_env.tags)) == list(sorted(["test", "second test"])) + assert list(sorted(example_env.tags)) == list( + sorted(["test", "second test"]) + ) From 3178f754c384c242cdc61810e41021439098b9d2 Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 26 Feb 2024 11:30:35 +0000 Subject: [PATCH 3/9] Expose new addTag mutation --- schema.graphql | 8 ++++++++ softpack_core/schemas/environment.py | 1 + 2 files changed, 9 insertions(+) diff --git a/schema.graphql b/schema.graphql index 98f7454..61bc7ee 100644 --- a/schema.graphql +++ b/schema.graphql @@ -3,6 +3,12 @@ schema { mutation: SchemaMutation } +union AddTagResponse = AddTagSuccess | InvalidInputError | EnvironmentNotFoundError + +type AddTagSuccess implements Success { + message: String! +} + type BuilderError implements Error { message: String! } @@ -31,6 +37,7 @@ type Environment { type: Type! packages: [Package!]! state: State + tags: [String!]! requested: DateTime buildStart: DateTime buildDone: DateTime @@ -86,6 +93,7 @@ type PackageMultiVersion { type SchemaMutation { createEnvironment(env: EnvironmentInput!): CreateResponse! deleteEnvironment(name: String!, path: String!): DeleteResponse! + addTag(name: String!, path: String!, tag: String!): AddTagResponse! createFromModule(file: Upload!, modulePath: String!, environmentPath: String!): CreateResponse! updateFromModule(file: Upload!, modulePath: String!, environmentPath: String!): UpdateResponse! } diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index e896ebe..2a5fa2f 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -809,6 +809,7 @@ class Mutation: createEnvironment: CreateResponse = Environment.create # type: ignore deleteEnvironment: DeleteResponse = Environment.delete # type: ignore + addTag: AddTagResponse = Environment.add_tag # type: ignore # writeArtifact: WriteArtifactResponse = ( # type: ignore # Environment.write_artifact # ) From 726d9a658a115b5737f1cf5f49b344cb70d55fb0 Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 26 Feb 2024 11:58:49 +0000 Subject: [PATCH 4/9] Allow setting tags via createEnvironment mutation --- softpack_core/schemas/environment.py | 22 +++++++++++++++++----- tests/integration/test_environment.py | 10 ++++++++++ 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index 2a5fa2f..e95d47e 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -8,7 +8,7 @@ import io import re import statistics -from dataclasses import dataclass +from dataclasses import dataclass, field from pathlib import Path from traceback import format_exception_only from typing import List, Optional, Tuple, Union, cast @@ -161,6 +161,7 @@ class EnvironmentInput: path: str description: str packages: list[PackageInput] + tags: list[str] = field(default_factory=list) def validate(self) -> Union[None, InvalidInputError]: """Validate all values. @@ -171,7 +172,11 @@ def validate(self) -> Union[None, InvalidInputError]: Returns: None if good, or InvalidInputError if not all values supplied. """ - if any(len(value) == 0 for value in vars(self).values()): + if any( + len(value) == 0 + for key, value in vars(self).items() + if key != "tags" + ): return InvalidInputError(message="all fields must be filled in") if not re.fullmatch("^[a-zA-Z0-9_-][a-zA-Z0-9_.-]*$", self.name): @@ -460,7 +465,7 @@ def create_new_env( name=env.name, ) - # Create folder with place-holder file + # Create folder with initial files new_folder_path = Path(env.path, env.name) try: softpack_definition = dict( @@ -470,13 +475,20 @@ def create_new_env( for pkg in env.packages ], ) - ymlData = yaml.dump(softpack_definition) + definitionData = yaml.dump(softpack_definition) + + meta = dict(tags=sorted(set(env.tags))) + metaData = yaml.dump(meta) tree_oid = cls.artifacts.create_files( new_folder_path, [ (env_type, ""), # e.g. .built_by_softpack - (cls.artifacts.environments_file, ymlData), # softpack.yml + ( + cls.artifacts.environments_file, + definitionData, + ), # softpack.yml + (cls.artifacts.meta_file, metaData), ], True, ) diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index de70674..20cb10d 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -52,6 +52,7 @@ def test_create(httpx_post, testable_env_input: EnvironmentInput) -> None: Package(name="pkg_test2"), Package(name="pkg_test3", version="3.1"), ], + tags=["foo", "foo", "bar"], ) ) assert isinstance(result, CreateEnvironmentSuccess) @@ -71,6 +72,10 @@ def test_create(httpx_post, testable_env_input: EnvironmentInput) -> None: "packages": ["pkg_test"], } assert yaml.safe_load(ymlFile.data.decode()) == expected_yaml + meta_yml = file_in_remote(dir / Environment.artifacts.meta_file) + expected_meta_yml = {"tags": []} + actual_meta_yml = yaml.safe_load(meta_yml.data.decode()) + assert actual_meta_yml == expected_meta_yml dir = Path( Environment.artifacts.environments_root, @@ -87,6 +92,11 @@ def test_create(httpx_post, testable_env_input: EnvironmentInput) -> None: } assert yaml.safe_load(ymlFile.data.decode()) == expected_yaml + meta_yml = file_in_remote(dir / Environment.artifacts.meta_file) + expected_meta_yml = {"tags": ["bar", "foo"]} + actual_meta_yml = yaml.safe_load(meta_yml.data.decode()) + assert actual_meta_yml == expected_meta_yml + result = Environment.create(testable_env_input) testable_env_input.name = orig_input_name assert isinstance(result, CreateEnvironmentSuccess) From 5dcc2004b6c69e7ba392b52048fe1b1a98ff749d Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 26 Feb 2024 14:42:38 +0000 Subject: [PATCH 5/9] Allow some punctuation, but not trailing/leading spaces, in tags --- softpack_core/schemas/environment.py | 8 +++++--- tests/integration/test_environment.py | 6 ++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index e95d47e..0c43eb0 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -530,7 +530,8 @@ def add_tag( ) -> AddTagResponse: # type: ignore """Add a tag to an Environment. - Tags must be composed solely of alphanumerics and spaces. + Tags must be composed solely of alphanumerics, dots, underscores, + dashes, and spaces. Adding a tag that already exists is not an error. @@ -547,9 +548,10 @@ def add_tag( if response is not None: return response - if re.fullmatch(r"[a-zA-Z0-9 ]+", tag) is None: + if re.fullmatch(r"[a-zA-Z0-9 ._-]+", tag.strip()) is None: return InvalidInputError( - message="Tags must contain only alphanumerics and spaces" + message="Tags must contain only alphanumerics, dots, " + "underscores, dashes, and spaces" ) tree = cls.artifacts.get(Path(path), name) diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index 20cb10d..185e648 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -515,6 +515,12 @@ def test_tagging(httpx_post, testable_env_input: EnvironmentInput) -> None: result = Environment.add_tag(name, path, tag="../") assert isinstance(result, InvalidInputError) + result = Environment.add_tag(name, path, tag="") + assert isinstance(result, InvalidInputError) + + result = Environment.add_tag(name, path, tag=" ") + assert isinstance(result, InvalidInputError) + example_env = Environment.iter()[0] assert len(example_env.tags) == 1 assert example_env.tags[0] == "test" From cc238c95e4181dde6c3cc48767b0b3e05e02b0cd Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 26 Feb 2024 14:44:57 +0000 Subject: [PATCH 6/9] Disallow internal runs of spaces in tags --- softpack_core/schemas/environment.py | 5 +++++ tests/integration/test_environment.py | 3 +++ 2 files changed, 8 insertions(+) diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index 0c43eb0..9a0fb38 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -554,6 +554,11 @@ def add_tag( "underscores, dashes, and spaces" ) + if re.search(r"\s\s", tag) is not None: + return InvalidInputError( + message="Tags must not contain runs of multiple spaces" + ) + tree = cls.artifacts.get(Path(path), name) if tree is None: return EnvironmentNotFoundError(path=path, name=name) diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index 185e648..9e03b8e 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -521,6 +521,9 @@ def test_tagging(httpx_post, testable_env_input: EnvironmentInput) -> None: result = Environment.add_tag(name, path, tag=" ") assert isinstance(result, InvalidInputError) + result = Environment.add_tag(name, path, tag="foo bar") + assert isinstance(result, InvalidInputError) + example_env = Environment.iter()[0] assert len(example_env.tags) == 1 assert example_env.tags[0] == "test" From 6ce153d899bfccd1e0acdf2af144dac83d007ca3 Mon Sep 17 00:00:00 2001 From: ash Date: Tue, 27 Feb 2024 10:16:23 +0000 Subject: [PATCH 7/9] Make tags optional in createEnvironment mutation --- schema.graphql | 1 + softpack_core/schemas/environment.py | 53 +++++++++++++++++++-------- tests/integration/test_environment.py | 13 +++++++ 3 files changed, 51 insertions(+), 16 deletions(-) diff --git a/schema.graphql b/schema.graphql index 61bc7ee..443d4ed 100644 --- a/schema.graphql +++ b/schema.graphql @@ -55,6 +55,7 @@ input EnvironmentInput { path: String! description: String! packages: [PackageInput!]! + tags: [String!] = null } type EnvironmentNotFoundError implements Error { diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index 9a0fb38..0d5eb48 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -8,7 +8,7 @@ import io import re import statistics -from dataclasses import dataclass, field +from dataclasses import dataclass from pathlib import Path from traceback import format_exception_only from typing import List, Optional, Tuple, Union, cast @@ -141,6 +141,32 @@ class BuilderError(Error): ) +def validate_tag(tag: str) -> Union[None, InvalidInputError]: + """If the given tag is invalid, return an error describing why, else None. + + Tags must be composed solely of alphanumerics, dots, underscores, + dashes, and spaces, and not contain runs of multiple spaces or + leading/trailing whitespace. + """ + if tag != tag.strip(): + return InvalidInputError( + message="Tags must not contain leading or trailing whitespace" + ) + + if re.fullmatch(r"[a-zA-Z0-9 ._-]+", tag) is None: + return InvalidInputError( + message="Tags must contain only alphanumerics, dots, " + "underscores, dashes, and spaces" + ) + + if re.search(r"\s\s", tag) is not None: + return InvalidInputError( + message="Tags must not contain runs of multiple spaces" + ) + + return None + + @strawberry.input class PackageInput(Package): """A Strawberry input model representing a package.""" @@ -161,7 +187,7 @@ class EnvironmentInput: path: str description: str packages: list[PackageInput] - tags: list[str] = field(default_factory=list) + tags: Optional[list[str]] = None def validate(self) -> Union[None, InvalidInputError]: """Validate all values. @@ -198,6 +224,10 @@ def validate(self) -> Union[None, InvalidInputError]: "alphanumerics, dash, and underscore" ) + for tag in self.tags or []: + if (response := validate_tag(tag)) is not None: + return response + return None @classmethod @@ -477,7 +507,7 @@ def create_new_env( ) definitionData = yaml.dump(softpack_definition) - meta = dict(tags=sorted(set(env.tags))) + meta = dict(tags=sorted(set(env.tags or []))) metaData = yaml.dump(meta) tree_oid = cls.artifacts.create_files( @@ -530,8 +560,7 @@ def add_tag( ) -> AddTagResponse: # type: ignore """Add a tag to an Environment. - Tags must be composed solely of alphanumerics, dots, underscores, - dashes, and spaces. + Tags must be valid as defined by validate_tag(). Adding a tag that already exists is not an error. @@ -544,20 +573,12 @@ def add_tag( A message confirming the success or failure of the operation. """ environment_path = Path(path, name) - response = cls.check_env_exists(environment_path) + response: Optional[Error] = cls.check_env_exists(environment_path) if response is not None: return response - if re.fullmatch(r"[a-zA-Z0-9 ._-]+", tag.strip()) is None: - return InvalidInputError( - message="Tags must contain only alphanumerics, dots, " - "underscores, dashes, and spaces" - ) - - if re.search(r"\s\s", tag) is not None: - return InvalidInputError( - message="Tags must not contain runs of multiple spaces" - ) + if (response := validate_tag(tag)) is not None: + return response tree = cls.artifacts.get(Path(path), name) if tree is None: diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index 9e03b8e..86c3896 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -110,6 +110,19 @@ def test_create(httpx_post, testable_env_input: EnvironmentInput) -> None: assert file_in_remote(path) +def test_create_no_tags(httpx_post, testable_env_input): + testable_env_input.tags = None + result = Environment.create(testable_env_input) + assert isinstance(result, CreateEnvironmentSuccess) + + +def test_create_illegal_tags(httpx_post, testable_env_input): + for tag in [" ", " ", " leading whitespace", "trailing whitespace "]: + testable_env_input.tags = [tag] + result = Environment.create(testable_env_input) + assert isinstance(result, InvalidInputError) + + def test_create_name_empty_disallowed(httpx_post, testable_env_input): testable_env_input.name = "" result = Environment.create(testable_env_input) From 143add2de5ff378d5b2886a06ce23cf4e2b6d95e Mon Sep 17 00:00:00 2001 From: ash Date: Wed, 28 Feb 2024 13:58:25 +0000 Subject: [PATCH 8/9] Sort saved tags on addTag --- softpack_core/schemas/environment.py | 2 +- tests/integration/test_environment.py | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index 0d5eb48..8e989d8 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -589,7 +589,7 @@ def add_tag( return AddTagSuccess(message="Tag already present") tags.add(tag) - metadata = yaml.dump({"tags": list(tags)}) + metadata = yaml.dump({"tags": sorted(tags)}) tree_oid = cls.artifacts.create_file( environment_path, cls.artifacts.meta_file, metadata, overwrite=True ) diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index 86c3896..6c07d03 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -545,15 +545,12 @@ def test_tagging(httpx_post, testable_env_input: EnvironmentInput) -> None: assert isinstance(result, AddTagSuccess) example_env = Environment.iter()[0] - assert list(sorted(example_env.tags)) == list( - sorted(["test", "second test"]) - ) + assert example_env.tags == ["second test", "test"] result = Environment.add_tag(name, path, tag="test") assert isinstance(result, AddTagSuccess) assert result.message == "Tag already present" example_env = Environment.iter()[0] - assert list(sorted(example_env.tags)) == list( - sorted(["test", "second test"]) - ) + assert example_env.tags == ["second test", "test"] + From 252b48c2c3e817378c05f4db7a0f84b50deb5c1e Mon Sep 17 00:00:00 2001 From: ash Date: Wed, 28 Feb 2024 14:28:10 +0000 Subject: [PATCH 9/9] Formatting --- tests/integration/test_environment.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index 6c07d03..d59fb5d 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -553,4 +553,3 @@ def test_tagging(httpx_post, testable_env_input: EnvironmentInput) -> None: example_env = Environment.iter()[0] assert example_env.tags == ["second test", "test"] -