-
Notifications
You must be signed in to change notification settings - Fork 4
Implemented API: 4.8 Make response of GET /collections and GET /collections/{id} STAC-Conform #201
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.
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
Changes from all commits
2d9a90a
ac1ffe2
255fe0b
b5e3963
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,12 +59,13 @@ describe('buildCollectionSearchQuery - aggregated fields', () => { | |
| expect(sql).toMatch(/WHERE cse\.collection_id = c\.id/); | ||
| }); | ||
|
|
||
| // Provider roles are converted to arrays in SQL via string_to_array | ||
| test('includes LATERAL JOIN for providers with roles', () => { | ||
| const { sql } = buildCollectionSearchQuery({ limit: 10, token: 0 }); | ||
|
|
||
| expect(sql).toMatch(/jsonb_agg\(jsonb_build_object\(/); | ||
| expect(sql).toMatch(/'name', p\.provider/); | ||
| expect(sql).toMatch(/'roles', cpr\.collection_provider_roles/); | ||
| expect(sql).toMatch(/'roles', string_to_array\(cpr\.collection_provider_roles, ','\)/); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a benefit, if we turn the providers into an array?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. STAC compliance: providers[*].roles is defined by the STAC specification as an array of strings. A comma-separated string would be incorrectly typed. |
||
| expect(sql).toMatch(/FROM collection_providers cpr/); | ||
| expect(sql).toMatch(/JOIN providers p ON p\.id = cpr\.provider_id/); | ||
| expect(sql).toMatch(/WHERE cpr\.collection_id = c\.id/); | ||
|
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,7 +114,7 @@ function buildCollectionSearchQuery(params) { | |
| const values = []; | ||
| let i = 1; | ||
|
|
||
| // Full-text search using weighted tsvector across title (weight A) and description (weight B). | ||
| // Full-text search using tsvector across title and description. | ||
| // | ||
| // Notes: | ||
| // - Currently only title and description are included in the weighted tsvector. | ||
|
|
@@ -199,6 +199,8 @@ function buildCollectionSearchQuery(params) { | |
| // | ||
| // LATERAL JOINs aggregate related data (keywords, extensions, providers, assets, summaries, | ||
| // and crawl timestamps) from normalized tables without duplicating collection rows. | ||
| // NOTE: Provider roles are stored as a comma-separated string and converted to array | ||
| // via string_to_array for STAC compliance. | ||
| // Each LEFT JOIN LATERAL subquery returns a single aggregated row per collection. | ||
| let sql = selectPart + ` | ||
| FROM collection c | ||
|
|
@@ -217,7 +219,7 @@ function buildCollectionSearchQuery(params) { | |
| LEFT JOIN LATERAL ( | ||
| SELECT jsonb_agg(jsonb_build_object( | ||
| 'name', p.provider, | ||
| 'roles', cpr.collection_provider_roles | ||
| 'roles', string_to_array(cpr.collection_provider_roles, ',') | ||
| ) ORDER BY p.provider) AS providers | ||
| FROM collection_providers cpr | ||
| JOIN providers p ON p.id = cpr.provider_id | ||
|
|
@@ -272,7 +274,11 @@ function buildCollectionSearchQuery(params) { | |
| // Note: sortby.field is validated against a whitelist in the calling code; only collection | ||
| // table columns are allowed for sorting (not aggregated fields like keywords/providers). | ||
| if (sortby) { | ||
| sql += ` ORDER BY c.${sortby.field} ${sortby.direction}`; | ||
| const sortField = sortby.field; | ||
| const dir = sortby.direction; | ||
| // Apply ASCII collation only for license to match deterministic tests | ||
| const collate = sortField === 'license' ? ' COLLATE "C"' : ''; | ||
| sql += ` ORDER BY c.${sortField}${collate} ${dir}, c.id ASC`; | ||
|
Comment on lines
+277
to
+281
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Explain me, why you are doing this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code sorts collections by a specific field (e.g. license or title). When sorting by license, an ASCII-based sort order is intentionally used. The reason is that different machines could otherwise produce different sort orders, depending on locale and database settings. By using COLLATE "C", the sorting is identical on all systems, ensuring that tests remain reproducible everywhere. If you think we don’t need it I can remove it. |
||
| } else if (q) { | ||
| sql += ` ORDER BY rank DESC, c.id ASC`; | ||
| } else { | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was already a I mean it's a design-choice. Im okay with this, but then we should apply your logic to literally every
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed most of it, but not everything, because the pool event gets called very often. |
Uh oh!
There was an error while loading. Please reload this page.