Skip to content

Commit

Permalink
Allow identity, ensure dimensions is returned, & use pre-commit.ci (
Browse files Browse the repository at this point in the history
#103)

Update tests with valid entity using 'identity'.
Ensure "identity" is respected everywhere.

Ensure "dimensions" is always returned.

Pre-commit is now run through pre-commit.ci.
  • Loading branch information
CasperWA authored May 8, 2024
1 parent 326de73 commit a4c1bb6
Show file tree
Hide file tree
Showing 15 changed files with 344 additions and 124 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
install_extras: "[dev]"

# pre-commit
python_version_pre-commit: "3.10"
run_pre-commit: false

# pylint & safety
python_version_pylint_safety: "3.10"
Expand Down
17 changes: 13 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
# To install the git pre-commit hook run:
# pre-commit install
# To update the pre-commit hooks run:
# pre-commit autoupdate
# pre-commit.ci
ci:
autofix_commit_msg: |
[pre-commit.ci] auto fixes from pre-commit hooks
For more information, see https://pre-commit.ci
autofix_prs: false
autoupdate_branch: 'main'
autoupdate_commit_msg: '[pre-commit.ci] pre-commit autoupdate'
autoupdate_schedule: 'weekly'
submodules: false

# hooks
repos:
# pre-commit-hooks supplies a multitude of small hooks
# To get an overview of them all as well as the ones used here, please see
Expand Down
2 changes: 1 addition & 1 deletion entities_service/cli/_utils/generics.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def get_namespace_name_version(entity: Entity | dict[str, Any]) -> tuple[str, st
The version is reversed to sort it in descending order (utilizing StrReversor).
"""
if isinstance(entity, dict):
uri = entity.get("uri", None) or (
uri = entity.get("uri", entity.get("identity", None)) or (
f"{entity.get('namespace', '')}/{entity.get('version', '')}"
f"/{entity.get('name', '')}"
)
Expand Down
18 changes: 1 addition & 17 deletions entities_service/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from entities_service.service.config import CONFIG
from entities_service.service.logger import setup_logger
from entities_service.service.routers import get_routers
from entities_service.service.utils import _get_entity

if TYPE_CHECKING: # pragma: no cover
from typing import Any
Expand Down Expand Up @@ -59,23 +60,6 @@ async def lifespan(_: FastAPI):
APP.include_router(router)


async def _get_entity(version: str, name: str, db: str | None = None) -> dict[str, Any]:
"""Utility function for the endpoints to retrieve an entity."""
uri = f"{str(CONFIG.base_url).rstrip('/')}"

if db:
uri += f"/{db}"

uri += f"/{version}/{name}"

entity = get_backend(db=db).read(uri)

if entity is None:
raise ValueError(f"Could not find entity: uri={uri}")

return entity


@APP.get(
"/{version}/{name}",
response_model=Entity,
Expand Down
11 changes: 7 additions & 4 deletions entities_service/models/soft.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ class SOFTEntity(BaseModel):
"The universal identifier for the entity. This MUST start with the base"
" URL."
),
validation_alias=AliasChoices("identity", "uri"),
),
] = None
description: Annotated[str, Field(description="Description of the entity.")] = ""
Expand Down Expand Up @@ -244,7 +245,7 @@ def _check_cross_dependent_fields(cls, data: Any) -> Any:
if (
isinstance(data, dict)
and any(data.get(_) is None for _ in ("name", "version", "namespace"))
and data.get("uri") is None
and data.get("uri", data.get("identity")) is None
):
error_message = (
"Either `name`, `version`, and `namespace` or `uri` must be set.\n"
Expand All @@ -254,13 +255,14 @@ def _check_cross_dependent_fields(cls, data: Any) -> Any:
if (
isinstance(data, dict)
and all(data.get(_) is not None for _ in ("name", "version", "namespace"))
and data.get("uri") is not None
and data["uri"] != f"{data['namespace']}/{data['version']}/{data['name']}"
and data.get("uri", data.get("identity")) is not None
and data.get("uri", data.get("identity", ""))
!= f"{data['namespace']}/{data['version']}/{data['name']}"
):
# Ensure that `uri` is consistent with `name`, `version`, and `namespace`.
diff = "\n ".join(
difflib.ndiff(
[data["uri"]],
[data.get("uri", data.get("identity", ""))],
[f"{data['namespace']}/{data['version']}/{data['name']}"],
)
)
Expand All @@ -269,4 +271,5 @@ def _check_cross_dependent_fields(cls, data: Any) -> Any:
f"`namespace`:\n\n {diff}\n\n"
)
raise ValueError(error_message)

return data
2 changes: 1 addition & 1 deletion entities_service/service/backend/mongodb.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ def _single_uri_query(self, uri: str) -> dict[str, Any]:
def _prepare_entity(self, entity: Entity | dict[str, Any]) -> dict[str, Any]:
"""Clean and prepare the entity for interactions with the MongoDB backend."""
if isinstance(entity, dict):
uri = entity.get("uri", None) or (
uri = entity.get("uri", entity.get("identity", None)) or (
f"{entity.get('namespace', '')}/{entity.get('version', '')}"
f"/{entity.get('name', '')}"
)
Expand Down
12 changes: 11 additions & 1 deletion entities_service/service/routers/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from entities_service.service.backend import get_backend
from entities_service.service.config import CONFIG
from entities_service.service.security import verify_token
from entities_service.service.utils import _add_dimensions

if TYPE_CHECKING: # pragma: no cover
from typing import Any
Expand Down Expand Up @@ -83,6 +84,12 @@ async def create_entities(
CONFIG.backend, auth_level="write", db=namespace
)

LOGGER.debug(
"Creating %s entities in namespace '%s'",
len(namespaced_entities),
namespace,
)

try:
created_namespaced_entities = namespaced_entities_backend.create(
namespaced_entities
Expand All @@ -97,7 +104,7 @@ async def create_entities(
"Already created entities: uris=%s",
", ".join(
(
entity.get("uri", "")
entity.get("uri", entity.get("identity", ""))
or (
f"{entity.get('namespace', '')}"
f"/{entity.get('version', '')}"
Expand All @@ -123,6 +130,9 @@ async def create_entities(
):
raise write_fail_exception

# Ensure the returned entities have the dimensions key
await _add_dimensions(created_namespaced_entities)

if isinstance(created_namespaced_entities, dict):
created_entities.append(created_namespaced_entities)
else:
Expand Down
50 changes: 50 additions & 0 deletions entities_service/service/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
"""Utility functions for the service."""

from __future__ import annotations

from typing import TYPE_CHECKING

from entities_service.service.backend import get_backend
from entities_service.service.config import CONFIG

if TYPE_CHECKING: # pragma: no cover
from typing import Any


async def _add_dimensions(entity: dict[str, Any] | list[dict[str, Any]]) -> None:
"""Utility function for the endpoints to add dimensions to an entity."""
if isinstance(entity, list):
for entity_ in entity:
await _add_dimensions(entity_)
return

if "dimensions" not in entity:
# SOFT5
if isinstance(entity["properties"], list):
entity["dimensions"] = []

# SOFT7
elif isinstance(entity["properties"], dict):
entity["dimensions"] = {}

else:
raise ValueError(f"Invalid entity: uri={entity['uri']}")


async def _get_entity(version: str, name: str, db: str | None = None) -> dict[str, Any]:
"""Utility function for the endpoints to retrieve an entity."""
uri = f"{str(CONFIG.base_url).rstrip('/')}"

if db:
uri += f"/{db}"

uri += f"/{version}/{name}"

entity = get_backend(db=db).read(uri)

if entity is None:
raise ValueError(f"Could not find entity: uri={uri}")

await _add_dimensions(entity)

return entity
44 changes: 27 additions & 17 deletions tests/cli/commands/test_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@ def test_upload_filepath(
core_namespace = str(CONFIG.base_url).rstrip("/")
current_namespace = f"{core_namespace}/{namespace}" if namespace else core_namespace

assert "uri" in raw_entity
id_key = "uri" if "uri" in raw_entity else "identity"
assert id_key in raw_entity

# Match raw_entity to a backend entity (always use 'uri')
raw_entity["uri"] = raw_entity.pop(id_key)

if namespace:
# Update the entity's namespace to the current namespace
Expand All @@ -82,7 +86,7 @@ def test_upload_filepath(

# Mock response for "Check if entity already exists"
httpx_mock.add_response(
url=raw_entity["uri"],
url=raw_entity[id_key],
status_code=404, # not found
)

Expand Down Expand Up @@ -176,11 +180,13 @@ def test_upload_directory(
directory = tmp_path / "valid_entities"
directory.mkdir(parents=True, exist_ok=True)
for index, raw_entity in enumerate(raw_entities):
id_key = "uri" if "uri" in raw_entity else "identity"

# Update the entity's namespace to the current namespace
if "namespace" in raw_entity:
raw_entity["namespace"] = current_namespace
if "uri" in raw_entity:
raw_entity["uri"] = raw_entity["uri"].replace(
if id_key in raw_entity:
raw_entity[id_key] = raw_entity[id_key].replace(
f"{core_namespace}/", f"{current_namespace}/"
)

Expand All @@ -196,9 +202,9 @@ def test_upload_directory(

# Mock response for "Check if entity already exists"
for raw_entity in raw_entities:
assert "uri" in raw_entity
assert any(_ in raw_entity for _ in ("uri", "identity"))
httpx_mock.add_response(
url=raw_entity["uri"],
url=raw_entity.get("uri", raw_entity.get("identity")),
status_code=404, # not found
)

Expand Down Expand Up @@ -322,14 +328,16 @@ def test_existing_entity_different_content(
core_namespace = str(CONFIG.base_url).rstrip("/")
current_namespace = f"{core_namespace}/{namespace}" if namespace else core_namespace

assert "uri" in raw_entity

if namespace:
# Update the entity's namespace to the current namespace
if "namespace" in raw_entity:
raw_entity["namespace"] = current_namespace
if "uri" in raw_entity:
raw_entity["uri"] = raw_entity["uri"].replace(
f"{core_namespace}/", f"{current_namespace}/"
)

raw_entity["uri"] = raw_entity["uri"].replace(
f"{core_namespace}/", f"{current_namespace}/"
)

# Write the updated entity to file
directory = tmp_path / "valid_entities"
Expand All @@ -345,7 +353,6 @@ def test_existing_entity_different_content(
)

# Mock response for "Check if entity already exists"
assert "uri" in raw_entity
httpx_mock.add_response(
url=raw_entity["uri"],
status_code=200, # ok
Expand Down Expand Up @@ -549,14 +556,16 @@ def test_existing_entity_errors(
core_namespace = str(CONFIG.base_url).rstrip("/")
current_namespace = f"{core_namespace}/{namespace}" if namespace else core_namespace

assert "uri" in raw_entity

if namespace:
# Update the entity's namespace to the current namespace
if "namespace" in raw_entity:
raw_entity["namespace"] = current_namespace
if "uri" in raw_entity:
raw_entity["uri"] = raw_entity["uri"].replace(
f"{core_namespace}/", f"{current_namespace}/"
)

raw_entity["uri"] = raw_entity["uri"].replace(
f"{core_namespace}/", f"{current_namespace}/"
)

# Write the updated entity to file
directory = tmp_path / "valid_entities"
Expand All @@ -572,7 +581,6 @@ def test_existing_entity_errors(
)

# Mock response for "Check if entity already exists"
assert "uri" in raw_entity
httpx_mock.add_response(
url=raw_entity["uri"],
status_code=200, # ok
Expand Down Expand Up @@ -859,7 +867,9 @@ def test_using_stdin(

valid_entities_dir = static_dir / "valid_entities"
entity_uris: list[str] = [
json.loads(filepath.read_text())["uri"]
json.loads(filepath.read_text()).get(
"uri", json.loads(filepath.read_text()).get("identity")
)
for filepath in valid_entities_dir.glob("*.json")
]

Expand Down
Loading

0 comments on commit a4c1bb6

Please sign in to comment.