Skip to content

Feat: gsmel 1255, apply the join - #185

Open
cmangeat wants to merge 11 commits into
mainfrom
feat/GSMEL-1255
Open

cmangeat wants to merge 11 commits into
mainfrom
feat/GSMEL-1255

Conversation

@cmangeat

@cmangeat cmangeat commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@cmangeat cmangeat changed the title Feat/gsmel 1255 Feat: gsmel 1255, apply the join Oct 8, 2026
@cmangeat
cmangeat marked this pull request as ready for review October 8, 2026 14:42
cmangeat and others added 11 commits October 13, 2026 14:49
The params-then-XCom fallback for staging_table_name was duplicated
across all five ingestion steps (file/url/ftp/database/api). Extracted
into _resolve_staging_table_name().

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The encrypted_credentials decryption logic was duplicated across
url/ftp/api_ingest_step, differing only in the log label. Extracted
into _resolve_auth_credentials().

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…on_select

Build default column configs from the staging table for passthrough, and
filter out excluded/missing columns before the loop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ding X/Y point

Filter the geometry column out before the loop when the point is built from
X/Y, and use if/else instead of continue in the column loop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… data db

Add an optional JoinConfig to IntegrityTransformation: the staging table is
LEFT JOINed (keys compared as text) with a table of the same database, and the
requested columns are added, unrenamed, after the staging ones. An empty column
list means no join.

The join is validated before the column loop: unknown keys/columns and names
clashing with the staging output (after rename), the X/Y point or the reserved
id_datafeeder raise a ValueError. A joined geom can replace an excluded staging
geometry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…adata

get_staging_metadata now reads integrity_transformation through
IntegrityTransformation.model_validate and takes force_projection from it,
instead of picking raw dict keys. _resolve_columns only resolves columns
from the already-parsed ColumnConfig list.

force_projection is now returned even when no columns were saved. A
malformed saved config still logs a warning and falls back to the live
staging schema.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e fallback

The title is now a single or-chain: integrity title, source file name,
source name (DATABASE table or API layer), then "". The import-type
conditions live in _source_name instead of two reassigning if blocks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
StagingMetadata gets an optional join (JoinConfig): PUT
/ingestion/staging/{id}/metadata stores it in the IntegrityTransformation,
GET returns it. The right-hand table is referenced by schema and name, as
listed by /ingestion/integrity-links/joinable-tables.

The PUT replaces the whole transformation, so saving without join clears
a previously saved one. The join is only checked for shape here; table,
key and column checks still happen at preview/process time.

Regenerate the frontend API client.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@tonio tonio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, works fine. A few things I think need fixing before merge:

1. The join target is not authorized on write
PUT /staging/{id}/metadata stores join.table_schema / join.table_name as sent. Only the identifier syntax is checked (in reflect_join_table). joinable-tables filters by visibility, but nothing forces the
client to pick from it: a crafted PUT can join any table of the data DB (another org's final table, other users' staging tables…), and the values are then readable through GET /preview.
Suggestion: store the integrity_link_id of the reference dataset in JoinConfig instead of schema + table, and resolve it server side with load_authorized_integrity_link (on PUT, preview and final
processing). It also keeps the join valid if the final table is renamed. The frontend already has the id, so the change there is small.

2. Metadata access is enough to copy data
joinable-tables uses visibility_condition (METADATA rules only) and joinable-columns requires METADATA_READ. But the join copies the reference data into the new dataset, so a user with only metadata read
access on B can get B's data into their own dataset. The required rule should probably be DATA READ, checked on the list, the columns and the save. Same check as point 1 once the join is resolved by id.

3. Map hidden when a joined geom replaces an excluded staging geom
In get_staging_preview, geom_excluded_in_config forces is_geographic = False / geojson = None as soon as the staging geom is excluded, even if the join adds a geom. That's the case documented in
build_transformation_select ("Its geom can replace an excluded staging geometry"). Something like:

geom_excluded_in_config = (
    ... existing condition ...
    and not (config.join and DEFAULT_GEOMETRY_COLUMN in config.join.columns)
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants