Skip to content

fix type for advanced freetext and allow free-text for Item search #263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 8, 2025
Merged

fix type for advanced freetext and allow free-text for Item search #263

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2fd8c8f
fix type for advanced freetext
vincentsarago Jul 10, 2025
bb91b68
update from main
vincentsarago Jul 24, 2025
c0767f3
add tests and remove free-text from method annotations
vincentsarago Jul 24, 2025
bbf0cb5
add advanced tests
vincentsarago Jul 24, 2025
b5fdaba
add failing tests
vincentsarago Jul 25, 2025
947a977
fix and enable free-text for items
vincentsarago Aug 1, 2025
eaf6dd8
Merge branch 'main' into patch/allow-advanced-free-text-ext
vincentsarago Aug 4, 2025
af4b331
Merge branch 'main' into patch/allow-advanced-free-text-ext
vincentsarago Aug 8, 2025
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
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Fixed

- fix root-path handling when setting via env var or on app instance
- Allow `q` parameter to be a `str` not a `list[str]` for Advanced Free-Text extension

### Changed

Expand Down
36 changes: 25 additions & 11 deletions stac_fastapi/pgstac/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ async def all_collections( # noqa: C901
sortby: Optional[str] = None,
filter_expr: Optional[str] = None,
filter_lang: Optional[str] = None,
q: Optional[List[str]] = None,
**kwargs,
**kwargs: Any,
Copy link
Member Author

Choose a reason for hiding this comment

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

In this PR we changed and we now forward kwargs to _clean_search_args method.

Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if POST /search with {"q": 123} is submitted? Does q make it all the way to the DB and raises due to invalid types? There won't be any early API model validation of the parameter?

) -> Collections:
"""Cross catalog search (GET).

Expand Down Expand Up @@ -86,7 +85,7 @@ async def all_collections( # noqa: C901
sortby=sortby,
filter_query=filter_expr,
filter_lang=filter_lang,
q=q,
**kwargs,
)

async with request.app.state.get_connection(request, "r") as conn:
Expand Down Expand Up @@ -157,7 +156,10 @@ async def all_collections( # noqa: C901
)

async def get_collection(
self, collection_id: str, request: Request, **kwargs
self,
collection_id: str,
request: Request,
**kwargs: Any,
) -> Collection:
"""Get collection by id.

Expand Down Expand Up @@ -202,7 +204,9 @@ async def get_collection(
return Collection(**collection)

async def _get_base_item(
self, collection_id: str, request: Request
self,
collection_id: str,
request: Request,
) -> Dict[str, Any]:
"""Get the base item of a collection for use in rehydrating full item collection properties.

Expand Down Expand Up @@ -359,7 +363,7 @@ async def item_collection(
filter_expr: Optional[str] = None,
filter_lang: Optional[str] = None,
token: Optional[str] = None,
**kwargs,
**kwargs: Any,
) -> ItemCollection:
"""Get all items from a specific collection.

Expand Down Expand Up @@ -391,6 +395,7 @@ async def item_collection(
filter_lang=filter_lang,
fields=fields,
sortby=sortby,
**kwargs,
)

try:
Expand All @@ -417,7 +422,11 @@ async def item_collection(
return ItemCollection(**item_collection)

async def get_item(
self, item_id: str, collection_id: str, request: Request, **kwargs
self,
item_id: str,
collection_id: str,
request: Request,
**kwargs: Any,
) -> Item:
"""Get item by id.

Expand Down Expand Up @@ -445,7 +454,10 @@ async def get_item(
return Item(**item_collection["features"][0])

async def post_search(
self, search_request: PgstacSearch, request: Request, **kwargs
self,
search_request: PgstacSearch,
request: Request,
**kwargs: Any,
) -> ItemCollection:
"""Cross catalog search (POST).

Expand Down Expand Up @@ -489,7 +501,7 @@ async def get_search(
filter_expr: Optional[str] = None,
filter_lang: Optional[str] = None,
token: Optional[str] = None,
**kwargs,
**kwargs: Any,
) -> ItemCollection:
"""Cross catalog search (GET).

Expand All @@ -516,6 +528,7 @@ async def get_search(
sortby=sortby,
filter_query=filter_expr,
filter_lang=filter_lang,
**kwargs,
)

try:
Expand Down Expand Up @@ -550,7 +563,8 @@ def _clean_search_args( # noqa: C901
sortby: Optional[str] = None,
filter_query: Optional[str] = None,
filter_lang: Optional[str] = None,
q: Optional[List[str]] = None,
q: Optional[Union[str, List[str]]] = None,
**kwargs: Any,
) -> Dict[str, Any]:
"""Clean up search arguments to match format expected by pgstac"""
if filter_query:
Expand Down Expand Up @@ -596,7 +610,7 @@ def _clean_search_args( # noqa: C901
base_args["fields"] = {"include": includes, "exclude": excludes}

if q:
base_args["q"] = " OR ".join(q)
base_args["q"] = " OR ".join(q) if isinstance(q, list) else q

# Remove None values from dict
clean = {}
Expand Down
56 changes: 56 additions & 0 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
CollectionSearchExtension,
CollectionSearchFilterExtension,
FieldsExtension,
FreeTextAdvancedExtension,
FreeTextExtension,
ItemCollectionFilterExtension,
OffsetPaginationExtension,
Expand Down Expand Up @@ -402,3 +403,58 @@ async def default_client(default_app):
transport=ASGITransport(app=default_app), base_url="http://test"
) as c:
yield c


@pytest.fixture(scope="function")
async def app_advanced_freetext(database):
"""Default stac-fastapi-pgstac application without only the transaction extensions."""
api_settings = Settings(testing=True)

application_extensions = [
TransactionExtension(client=TransactionsClient(), settings=api_settings)
]

collection_extensions = [
FreeTextAdvancedExtension(),
OffsetPaginationExtension(),
]
collection_search_extension = CollectionSearchExtension.from_extensions(
collection_extensions
)
application_extensions.append(collection_search_extension)

app = StacApi(
settings=api_settings,
extensions=application_extensions,
client=CoreCrudClient(),
health_check=health_check,
collections_get_request_model=collection_search_extension.GET,
)

postgres_settings = PostgresSettings(
pguser=database.user,
pgpassword=database.password,
pghost=database.host,
pgport=database.port,
pgdatabase=database.dbname,
)
logger.info("Creating app Fixture")
time.time()
Copy link
Member

Choose a reason for hiding this comment

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

Is time used for anything?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤷

await connect_to_db(
app.app,
postgres_settings=postgres_settings,
add_write_connection_pool=True,
)
yield app.app
await close_db_connection(app.app)

logger.info("Closed Pools.")


@pytest.fixture(scope="function")
async def app_client_advanced_freetext(app_advanced_freetext):
logger.info("creating app_client")
async with AsyncClient(
transport=ASGITransport(app=app_advanced_freetext), base_url="http://test"
) as c:
yield c
65 changes: 65 additions & 0 deletions tests/resources/test_collection.py
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

There are no tests for the /search?q= and /collections/{col}/items?q=... cases?

Also, there could be POST /search with {"q": "advanced AND search"} or {"q": ["basic", "search"]}.

Copy link
Member

@gadomski gadomski Jul 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't believe this PR is meant to implement free-text item search, just correct some stuff around free-text collection search.

Copy link
Contributor

Choose a reason for hiding this comment

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

Function self._clean_search_args calls are updated with the **kwargs, so q will now trickle down within these calls as well (which is good), and should be considered (but could be in a separate PR though not to block this one).

Copy link
Member Author

Choose a reason for hiding this comment

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

q won't be in /search because we don't usually use the free-text extension for items

if you're passing a dict, pydantic should raise a validation https://github.com/stac-utils/stac-fastapi/blob/fa42985255fad0bab7dbe3aadbf1f74cb1635f3a/stac_fastapi/extensions/stac_fastapi/extensions/core/free_text/request.py#L37-L43

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't believe this PR is meant to implement free-text item search, just correct some stuff around free-text collection search.

Exactly, but it also enables free-text for items by using kwargs, as for other unknown extension people would want to implement. Now they would have to just pass a custom _clean_search_args to support any kind of input passed through kwargs

Copy link
Contributor

Choose a reason for hiding this comment

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

OK. Good point about the Pydantic model.

I'm not sure to understand the "q won't be in /search".
Isn't it available if the conformance is applied?
https://github.com/stac-utils/stac-fastapi/blob/fa42985255fad0bab7dbe3aadbf1f74cb1635f3a/stac_fastapi/extensions/stac_fastapi/extensions/core/free_text/free_text.py#L27-L40

I was able to activate it is my implementation (https://github.com/crim-ca/stac-app/pull/28/files). It should do the title/description/keywords free-text search across all collections' items, as if filter or query was used, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure to understand the "q won't be in /search".

I just mean that q parameter will ONLY be available if enabled at the application level.

As mentioned in #263 (comment) it works only for Advanced because we don't do str -> list -> str transformation but keep the values as str

Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,71 @@ async def test_collection_search_freetext(
assert resp.json()["collections"][0]["id"] == load_test2_collection.id

resp = await app_client.get(
"/collections",
params={"q": "temperature,calibrated"},
)
assert resp.json()["numberReturned"] == 2
assert resp.json()["numberMatched"] == 2
assert len(resp.json()["collections"]) == 2

resp = await app_client.get(
"/collections",
params={"q": "temperature,yo"},
Copy link
Member

Choose a reason for hiding this comment

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

)
assert resp.json()["numberReturned"] == 1
assert resp.json()["numberMatched"] == 1
assert len(resp.json()["collections"]) == 1
assert resp.json()["collections"][0]["id"] == load_test2_collection.id

resp = await app_client.get(
"/collections",
params={"q": "nosuchthing"},
)
assert len(resp.json()["collections"]) == 0


@requires_pgstac_0_9_2
@pytest.mark.asyncio
async def test_collection_search_freetext_advanced(
app_client_advanced_freetext, load_test_collection, load_test2_collection
):
# free-text
resp = await app_client_advanced_freetext.get(
"/collections",
params={"q": "temperature"},
)
assert resp.json()["numberReturned"] == 1
assert resp.json()["numberMatched"] == 1
assert len(resp.json()["collections"]) == 1
assert resp.json()["collections"][0]["id"] == load_test2_collection.id

resp = await app_client_advanced_freetext.get(
"/collections",
params={"q": "temperature,calibrated"},
)
assert resp.json()["numberReturned"] == 2
assert resp.json()["numberMatched"] == 2
assert len(resp.json()["collections"]) == 2

resp = await app_client_advanced_freetext.get(
"/collections",
params={"q": "temperature,yo"},
)
assert resp.json()["numberReturned"] == 1
assert resp.json()["numberMatched"] == 1
assert len(resp.json()["collections"]) == 1
assert resp.json()["collections"][0]["id"] == load_test2_collection.id

resp = await app_client_advanced_freetext.get(
"/collections",
params={"q": "temperature OR yo"},
)
assert resp.json()["numberReturned"] == 1
assert resp.json()["numberMatched"] == 1
assert len(resp.json()["collections"]) == 1
assert resp.json()["collections"][0]["id"] == load_test2_collection.id

resp = await app_client_advanced_freetext.get(
"/collections",
params={"q": "nosuchthing"},
)
Expand Down
Loading