diff --git a/CHANGELOG.md b/CHANGELOG.md index c0497770..77e74c57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Changed + +- Simplified Patch class and updated patch script creation including adding nest creation for merge patch [#420](https://github.com/stac-utils/stac-fastapi-elasticsearch-opensearch/pull/420) - Added default environment variable `STAC_ITEM_LIMIT` to SFEOS for result limiting of returned items and STAC collections [#419](https://github.com/stac-utils/stac-fastapi-elasticsearch-opensearch/pull/419) ## [v6.2.0] - 2025-08-27 diff --git a/stac_fastapi/core/stac_fastapi/core/base_database_logic.py b/stac_fastapi/core/stac_fastapi/core/base_database_logic.py index e3c4d64e..494b0dd7 100644 --- a/stac_fastapi/core/stac_fastapi/core/base_database_logic.py +++ b/stac_fastapi/core/stac_fastapi/core/base_database_logic.py @@ -48,6 +48,7 @@ async def json_patch_item( item_id: str, operations: List, base_url: str, + create_nest: bool = False, refresh: bool = True, ) -> Dict: """Patch a item in the database follows RF6902.""" @@ -94,6 +95,7 @@ async def json_patch_collection( collection_id: str, operations: List, base_url: str, + create_nest: bool = False, refresh: bool = True, ) -> Dict: """Patch a collection in the database follows RF6902.""" diff --git a/stac_fastapi/elasticsearch/stac_fastapi/elasticsearch/database_logic.py b/stac_fastapi/elasticsearch/stac_fastapi/elasticsearch/database_logic.py index 5f100980..3e07d251 100644 --- a/stac_fastapi/elasticsearch/stac_fastapi/elasticsearch/database_logic.py +++ b/stac_fastapi/elasticsearch/stac_fastapi/elasticsearch/database_logic.py @@ -886,6 +886,7 @@ async def merge_patch_item( item_id=item_id, operations=operations, base_url=base_url, + create_nest=True, refresh=refresh, ) @@ -895,6 +896,7 @@ async def json_patch_item( item_id: str, operations: List[PatchOperation], base_url: str, + create_nest: bool = False, refresh: bool = True, ) -> Item: """Database logic for json patching an item following RF6902. @@ -929,7 +931,7 @@ async def json_patch_item( else: script_operations.append(operation) - script = operations_to_script(script_operations) + script = operations_to_script(script_operations, create_nest=create_nest) try: search_response = await self.client.search( @@ -1265,6 +1267,7 @@ async def merge_patch_collection( collection_id=collection_id, operations=operations, base_url=base_url, + create_nest=True, refresh=refresh, ) @@ -1273,6 +1276,7 @@ async def json_patch_collection( collection_id: str, operations: List[PatchOperation], base_url: str, + create_nest: bool = False, refresh: bool = True, ) -> Collection: """Database logic for json patching a collection following RF6902. @@ -1300,7 +1304,7 @@ async def json_patch_collection( else: script_operations.append(operation) - script = operations_to_script(script_operations) + script = operations_to_script(script_operations, create_nest=create_nest) try: await self.client.update( diff --git a/stac_fastapi/opensearch/stac_fastapi/opensearch/database_logic.py b/stac_fastapi/opensearch/stac_fastapi/opensearch/database_logic.py index 4ff44ca0..0e6b7b6d 100644 --- a/stac_fastapi/opensearch/stac_fastapi/opensearch/database_logic.py +++ b/stac_fastapi/opensearch/stac_fastapi/opensearch/database_logic.py @@ -869,6 +869,7 @@ async def merge_patch_item( item_id=item_id, operations=operations, base_url=base_url, + create_nest=True, refresh=refresh, ) @@ -878,6 +879,7 @@ async def json_patch_item( item_id: str, operations: List[PatchOperation], base_url: str, + create_nest: bool = False, refresh: bool = True, ) -> Item: """Database logic for json patching an item following RF6902. @@ -912,7 +914,7 @@ async def json_patch_item( else: script_operations.append(operation) - script = operations_to_script(script_operations) + script = operations_to_script(script_operations, create_nest=create_nest) try: search_response = await self.client.search( @@ -1220,6 +1222,7 @@ async def merge_patch_collection( collection_id=collection_id, operations=operations, base_url=base_url, + create_nest=True, refresh=refresh, ) @@ -1228,6 +1231,7 @@ async def json_patch_collection( collection_id: str, operations: List[PatchOperation], base_url: str, + create_nest: bool = False, refresh: bool = True, ) -> Collection: """Database logic for json patching a collection following RF6902. @@ -1255,7 +1259,7 @@ async def json_patch_collection( else: script_operations.append(operation) - script = operations_to_script(script_operations) + script = operations_to_script(script_operations, create_nest=create_nest) try: await self.client.update( diff --git a/stac_fastapi/sfeos_helpers/stac_fastapi/sfeos_helpers/database/utils.py b/stac_fastapi/sfeos_helpers/stac_fastapi/sfeos_helpers/database/utils.py index 169298a8..2a9dc20e 100644 --- a/stac_fastapi/sfeos_helpers/stac_fastapi/sfeos_helpers/database/utils.py +++ b/stac_fastapi/sfeos_helpers/stac_fastapi/sfeos_helpers/database/utils.py @@ -76,7 +76,7 @@ def merge_to_operations(data: Dict) -> List: nested_operations = merge_to_operations(value) for nested_operation in nested_operations: - nested_operation.path = f"{key}.{nested_operation.path}" + nested_operation.path = f"{key}/{nested_operation.path}" operations.append(nested_operation) else: @@ -90,6 +90,7 @@ def check_commands( op: str, path: ElasticPath, from_path: bool = False, + create_nest: bool = False, ) -> None: """Add Elasticsearch checks to operation. @@ -101,25 +102,44 @@ def check_commands( """ if path.nest: - commands.add( - f"if (!ctx._source.containsKey('{path.nest}'))" - f"{{Debug.explain('{path.nest} does not exist');}}" - ) - - if path.index or op in ["remove", "replace", "test"] or from_path: - commands.add( - f"if (!ctx._source{path.es_nest}.containsKey('{path.key}'))" - f"{{Debug.explain('{path.key} does not exist in {path.nest}');}}" - ) - - if from_path and path.index is not None: - commands.add( - f"if ((ctx._source{path.es_location} instanceof ArrayList" - f" && ctx._source{path.es_location}.size() < {path.index})" - f" || (!(ctx._source{path.es_location} instanceof ArrayList)" - f" && !ctx._source{path.es_location}.containsKey('{path.index}')))" - f"{{Debug.explain('{path.path} does not exist');}}" - ) + part_nest = "" + for index, path_part in enumerate(path.parts): + + # Create nested dictionaries if not present for merge operations + if create_nest and not from_path: + value = "[:]" + for sub_part in reversed(path.parts[index + 1 :]): + value = f"['{sub_part}': {value}]" + + commands.add( + f"if (!ctx._source{part_nest}.containsKey('{path_part}'))" + f"{{ctx._source{part_nest}['{path_part}'] = {value};}}" + f"{'' if index == len(path.parts) - 1 else' else '}" + ) + + else: + commands.add( + f"if (!ctx._source{part_nest}.containsKey('{path_part}'))" + f"{{Debug.explain('{path_part} in {path.path} does not exist');}}" + ) + + part_nest += f"['{path_part}']" + + if from_path or op in ["remove", "replace", "test"]: + + if isinstance(path.key, int): + commands.add( + f"if ((ctx._source{path.es_nest} instanceof ArrayList" + f" && ctx._source{path.es_nest}.size() < {abs(path.key)})" + f" || (!(ctx._source{path.es_nest} instanceof ArrayList)" + f" && !ctx._source{path.es_nest}.containsKey('{path.key}')))" + f"{{Debug.explain('{path.key} does not exist in {path.nest}');}}" + ) + else: + commands.add( + f"if (!ctx._source{path.es_nest}.containsKey('{path.key}'))" + f"{{Debug.explain('{path.key} does not exist in {path.nest}');}}" + ) def remove_commands(commands: ESCommandSet, path: ElasticPath) -> None: @@ -130,15 +150,16 @@ def remove_commands(commands: ESCommandSet, path: ElasticPath) -> None: path (ElasticPath): Path to value to be removed """ - if path.index is not None: + commands.add(f"def {path.variable_name};") + if isinstance(path.key, int): commands.add( - f"def {path.variable_name} = ctx._source{path.es_location}.remove({path.index});" + f"if (ctx._source{path.es_nest} instanceof ArrayList)" + f"{{{path.variable_name} = ctx._source{path.es_nest}.remove({path.es_key});}} else " ) - else: - commands.add( - f"def {path.variable_name} = ctx._source{path.es_nest}.remove('{path.key}');" - ) + commands.add( + f"{path.variable_name} = ctx._source{path.es_nest}.remove('{path.key}');" + ) def add_commands( @@ -160,21 +181,22 @@ def add_commands( value = ( from_path.variable_name if operation.op == "move" - else f"ctx._source.{from_path.es_path}" + else f"ctx._source{from_path.es_path}" ) + else: value = f"params.{path.param_key}" params[path.param_key] = operation.value - if path.index is not None: + if isinstance(path.key, int): commands.add( - f"if (ctx._source{path.es_location} instanceof ArrayList)" - f"{{ctx._source{path.es_location}.{'add' if operation.op in ['add', 'move'] else 'set'}({path.index}, {value})}}" - f"else{{ctx._source.{path.es_path} = {value}}}" + f"if (ctx._source{path.es_nest} instanceof ArrayList)" + f"{{ctx._source{path.es_nest}.{'add' if operation.op in ['add', 'move'] else 'set'}({path.es_key}, {value});}}" + f" else ctx._source{path.es_nest}['{path.es_key}'] = {value};" ) else: - commands.add(f"ctx._source.{path.es_path} = {value};") + commands.add(f"ctx._source{path.es_path} = {value};") def test_commands( @@ -190,14 +212,23 @@ def test_commands( value = f"params.{path.param_key}" params[path.param_key] = operation.value + if isinstance(path.key, int): + commands.add( + f"if (ctx._source{path.es_nest} instanceof ArrayList)" + f"{{if (ctx._source{path.es_nest}[{path.es_key}] != {value})" + f"{{Debug.explain('Test failed `{path.path}`" + f" != ' + ctx._source{path.es_path});}}" + f"}} else " + ) + commands.add( - f"if (ctx._source.{path.es_path} != {value})" - f"{{Debug.explain('Test failed `{path.path}` | " - f"{operation.json_value} != ' + ctx._source.{path.es_path});}}" + f"if (ctx._source{path.es_path} != {value})" + f"{{Debug.explain('Test failed `{path.path}`" + f" != ' + ctx._source{path.es_path});}}" ) -def operations_to_script(operations: List) -> Dict: +def operations_to_script(operations: List, create_nest: bool = False) -> Dict: """Convert list of operation to painless script. Args: @@ -215,10 +246,16 @@ def operations_to_script(operations: List) -> Dict: ElasticPath(path=operation.from_) if hasattr(operation, "from_") else None ) - check_commands(commands=commands, op=operation.op, path=path) + check_commands( + commands=commands, op=operation.op, path=path, create_nest=create_nest + ) if from_path is not None: check_commands( - commands=commands, op=operation.op, path=from_path, from_path=True + commands=commands, + op=operation.op, + path=from_path, + from_path=True, + create_nest=create_nest, ) if operation.op in ["remove", "move"]: diff --git a/stac_fastapi/sfeos_helpers/stac_fastapi/sfeos_helpers/models/patch.py b/stac_fastapi/sfeos_helpers/stac_fastapi/sfeos_helpers/models/patch.py index ce49bdb2..f1430270 100644 --- a/stac_fastapi/sfeos_helpers/stac_fastapi/sfeos_helpers/models/patch.py +++ b/stac_fastapi/sfeos_helpers/stac_fastapi/sfeos_helpers/models/patch.py @@ -3,7 +3,7 @@ import re from typing import Any, Dict, Optional, Union -from pydantic import BaseModel, computed_field, model_validator +from pydantic import BaseModel, model_validator regex = re.compile(r"([^.' ]*:[^.'[ ]*)\.?") replacements = str.maketrans({"/": "", ".": "", ":": "", "[": "", "]": ""}) @@ -71,16 +71,23 @@ class ElasticPath(BaseModel): """ - path: str + parts: list[str] = [] + + path: Optional[str] = None + key: Optional[Union[str, int]] = None nest: Optional[str] = None - partition: Optional[str] = None - key: Optional[str] = None es_path: Optional[str] = None - es_nest: Optional[str] = None es_key: Optional[str] = None + es_nest: Optional[str] = None + + variable_name: Optional[str] = None + param_key: Optional[str] = None - index_: Optional[int] = None + class Config: + """Class config.""" + + frozen = True @model_validator(mode="before") @classmethod @@ -90,77 +97,28 @@ def validate_model(cls, data: Any): Args: data (Any): input data """ - data["path"] = data["path"].lstrip("/").replace("/", ".") - data["nest"], data["partition"], data["key"] = data["path"].rpartition(".") - - if data["key"].lstrip("-").isdigit() or data["key"] == "-": - data["index_"] = -1 if data["key"] == "-" else int(data["key"]) - data["path"] = f"{data['nest']}[{data['index_']}]" - data["nest"], data["partition"], data["key"] = data["nest"].rpartition(".") - - data["es_path"] = to_es(data["path"]) - data["es_nest"] = f".{to_es(data['nest'])}" if data["nest"] else "" - data["es_key"] = to_es(data["key"]) - - return data - - @computed_field # type: ignore[misc] - @property - def index(self) -> Union[int, str, None]: - """Compute location of path. - - Returns: - str: path index - """ - if self.index_ and self.index_ < 0: + data["parts"] = data["path"].lstrip("/").split("/") - return f"ctx._source.{self.location}.size() - {-self.index_}" + data["key"] = data["parts"].pop(-1) + data["nest"] = "/".join(data["parts"]) + data["path"] = data["nest"] + "/" + data["key"] - return self.index_ + data["es_key"] = data["key"] + data["es_nest"] = "".join([f"['{part}']" for part in data["parts"]]) + data["es_path"] = data["es_nest"] + f"['{data['es_key']}']" - @computed_field # type: ignore[misc] - @property - def location(self) -> str: - """Compute location of path. - - Returns: - str: path location - """ - return self.nest + self.partition + self.key - - @computed_field # type: ignore[misc] - @property - def es_location(self) -> str: - """Compute location of path. - - Returns: - str: path location - """ - if self.es_key and ":" in self.es_key: - return self.es_nest + self.es_key - return self.es_nest + self.partition + self.es_key - - @computed_field # type: ignore[misc] - @property - def variable_name(self) -> str: - """Variable name for scripting. - - Returns: - str: variable name - """ - if self.index is not None: - return f"{self.location.replace('.','_').replace(':','_')}_{self.index}" - - return ( - f"{self.nest.replace('.','_').replace(':','_')}_{self.key.replace(':','_')}" - ) - - @computed_field # type: ignore[misc] - @property - def param_key(self) -> str: - """Param key for scripting. + if data["key"].lstrip("-").isdigit() or data["key"] == "-": + data["key"] = -1 if data["key"] == "-" else int(data["key"]) + data["es_key"] = ( + f"ctx._source{data['es_nest']}.size() - {-data['key']}" + if data["key"] < 0 + else str(data["key"]) + ) + data["es_path"] = data["es_nest"] + f"[{data['es_key']}]" + + data[ + "variable_name" + ] = f"{data['nest'].replace('/','_').replace(':','_')}_{str(data['key']).replace(':','_')}" + data["param_key"] = data["path"].translate(replacements) - Returns: - str: param key - """ - return self.path.translate(replacements) + return data diff --git a/stac_fastapi/tests/clients/test_es_os.py b/stac_fastapi/tests/clients/test_es_os.py index df6dae36..206c51c0 100644 --- a/stac_fastapi/tests/clients/test_es_os.py +++ b/stac_fastapi/tests/clients/test_es_os.py @@ -280,6 +280,26 @@ async def test_merge_patch_item_remove(ctx, core_client, txn_client): assert "proj:epsg" not in updated_item["properties"] +@pytest.mark.asyncio +async def test_merge_patch_create_nest(ctx, core_client, txn_client): + item = ctx.item + collection_id = item["collection"] + item_id = item["id"] + await txn_client.patch_item( + collection_id=collection_id, + item_id=item_id, + patch={"properties": {"new": {"nest": "foo"}}}, + request=MockRequest(headers={"content-type": "application/merge-patch+json"}), + ) + + updated_item = await core_client.get_item( + item_id, collection_id, request=MockRequest + ) + assert "new" in updated_item["properties"] + assert "nest" in updated_item["properties"]["new"] + assert updated_item["properties"]["new"]["nest"] == "foo" + + @pytest.mark.asyncio async def test_json_patch_item_add(ctx, core_client, txn_client): item = ctx.item @@ -551,6 +571,29 @@ async def test_json_patch_item_remove(ctx, core_client, txn_client): ) +@pytest.mark.asyncio +async def test_json_patch_add_with_bad_nest(ctx, core_client, txn_client): + item = ctx.item + collection_id = item["collection"] + item_id = item["id"] + operations = [ + PatchAddReplaceTest.model_validate( + {"op": "add", "path": "/properties/bad/nest", "value": "foo"} + ), + ] + + with pytest.raises(HTTPException): + + await txn_client.patch_item( + collection_id=collection_id, + item_id=item_id, + patch=operations, + request=MockRequest( + headers={"content-type": "application/json-patch+json"} + ), + ) + + @pytest.mark.asyncio async def test_json_patch_item_test_wrong_value(ctx, core_client, txn_client): item = ctx.item