Skip to content

FC-879 - Collection default smartfunction not always triggering - #6

Merged
bplatz merged 1 commit into
masterfrom
fix/collection-default-sf
Oct 23, 2020
Merged

bplatz merged 1 commit into
masterfrom
fix/collection-default-sf

Conversation

@bplatz

@bplatz bplatz commented Oct 23, 2020

Copy link
Copy Markdown
Contributor

No description provided.

@bplatz
bplatz requested a review from a team October 23, 2020 12:42
@bplatz
bplatz merged commit bf2805a into master Oct 23, 2020
@bplatz
bplatz deleted the fix/collection-default-sf branch October 23, 2020 12:55
@aaj3f aaj3f mentioned this pull request Apr 30, 2026
5 tasks done
bplatz added a commit that referenced this pull request May 17, 2026
… diagnostics, nits

Selected items from the additional review feedback. Each call
explained.

**#1 — Retry-fragility comment on `take(opts.shapes)`.** No retry
exists on the staging path today, but `take()` moves the inline
SHACL JSON off the txn, so a future retry would silently skip
inline-shapes validation on the second attempt. Hard invariant
comment added at the move site; if retry is ever added, defer
the take until after `stage_txn` returns successfully.

**#3 — `UnsupportedFeature` errors no longer carry an empty
`ledger_id`.** `resolve_graph_ref` rejected `f:atT` /
`f:trustPolicy` / `f:rollbackGuard` *before* verifying that the
ref was actually cross-ledger, so a misrouted same-ledger ref
hitting this path would have produced a confusing error with
`ledger_id: ""`. Reordered: the same-ledger-ref guard runs first,
binding `raw_ledger_ref` once; the unsupported-feature checks
now carry the real ledger id in every diagnostic.

**#4 — `f:atT` rejection breadth: audited, no gap.** All five
`f:GraphRef`-shaped predicates already reject `f:atT` at their
same-ledger entry points:
- `policy_builder::resolve_policy_source_g_ids`
- `tx::resolve_shapes_source_g_ids` (SHACL)
- `ontology_imports::resolve_schema_bundle`
- `tx::resolve_constraint_source_g_ids_for`
- `view::fluree_ext::resolve_local_rules_source_g_id`

Plus the cross-ledger `resolver` (now with non-empty ledger_id
per #3). Audit-only change; no code modification beyond the doc
note.

**#2 — Cross-ledger policy target IRI loss is now logged.**
`wire_to_restrictions` previously `filter_map`'d unresolvable
target IRIs and `for_classes` IRIs silently. A cross-ledger
policy could quietly narrow its scope on D if M referenced IRIs
D had never registered. Two `tracing::warn!` calls now surface
both cases with `restriction_id`, requested count, and resolved
count — operators can spot the silent narrowing in logs.

Kept the restriction even when targets become empty (rather than
dropping the whole restriction): same-ledger via
`load_policy_restriction` also produces empty target sets for
the same IRI-not-in-snapshot scenario, and dropping the
restriction would diverge cross-ledger from same-ledger
semantics. The warn log is the parity-preserving signal.
`fluree-db-policy` gains a `tracing` dependency for this.

**Nits taken:**
- `TAtUnavailable` doc-commented as Phase 3 reserved (unreachable
  today because `UnsupportedFeature { feature: "f:atT" }` fires
  first).
- Empty `uniqueProperties: []` over HTTP is now documented as an
  intentional "no inline constraints" treatment, not a silent
  bug.
- Cache read/write race in `resolver` documented as benign:
  losers materialize the same value structurally; single-flight
  is a future optimization, not a correctness need.
- `f:AccessPolicy` IRI promoted from local `const` /
  duplicated-literal to `fluree_vocab::policy_iris::ACCESS_POLICY`.
  Two call sites updated; nothing else used the local consts.

**Nits skipped per "don't make changes that don't seem needed":**
- HTTP-level negative test for malformed `opts.shapes` /
  `uniqueProperties` (#5) — unit-level coverage exists; HTTP
  harness setup cost not justified.
- Same-ledger `rules_source_g_id` end-to-end test (#6) — already
  covered by `it_rules_source::rules_source_in_named_graph_is_honored`,
  which runs an actual query against the named-graph rule.
- DynamoDB hard-drop failure handling (#7) — storage layer,
  unrelated to this PR.
- Duplicate pattern match in `shapes_materializer` —
  cosmetic.
- `snapshot.ledger_id` redundancy in inline-shapes / inline-
  ontology `txn_id` — cosmetic.
- `is_last_live_branch` WARN log — unrelated path.
mwatts pushed a commit to mwatts/fluree-db that referenced this pull request Jun 12, 2026
Two pieces that together let a multi-query envelope actually execute
against a single resolved snapshot moment with bounded parallelism.

Snapshot resolution (fluree-db-api/src/query/multi_snapshot.rs):

resolve_envelope_snapshot walks the distinct ledger set and produces an
EnvelopeSnapshot { as_of, ledgers: HashMap<String, i64> }. AsOf::T pins
the single ledger directly (multi-ledger with integer asOf was already
rejected at validation). AsOf::Iso resolves per-ledger via
time_resolve::datetime_to_t against each ledger's current snapshot,
mirroring the rounding rule in load_graph_db_at so sub-millisecond ISO
precision doesn't push us off-by-one before the intended commit. Absent
asOf captures server-now once via Utc::now() and pins each ledger to its
current t; the wall-clock moment is echoed back so the response is
reproducible. Per-ledger work is sequential in v1 — the connection-level
ledger cache (LedgerManager::get_or_load) coalesces repeated loads, so
the bounded fan-out (max_distinct_ledgers) is acceptable.

apply_snapshot_to_jsonld rewrites a sub-query body in place: string from
becomes object {@id, t}; object from gets t inserted next to its @id/id;
array entries get the same treatment recursively. Bare ledger ids are
matched against the snapshot map with temporal suffix and named-graph
fragment stripped, but the original identifier is preserved on the
written-back @id so fragment-selected named graphs still resolve.

apply_snapshot_to_sparql uses the SPARQL parser's IRI spans to do
precise byte-level splicing of FROM <iri> / FROM NAMED <iri> clauses,
appending @t:N to each ledger present in the snapshot map. Replacements
are sorted and applied right-to-left so earlier spans aren't shifted by
later edits. Span-based rewrite means IRIs inside comments or string
literals are never touched. When parsing fails the original string is
returned so the downstream parser produces the real error. IRIs that
already carry a temporal suffix are skipped defensively — validation
would have rejected the envelope before reaching here, but we don't
double-pin.

Dispatcher (fluree-db-server/src/routes/multi_dispatch.rs):

dispatch_multi_query owns a JoinSet for sub-query tasks and a Semaphore
sized to DispatchConfig::max_concurrency. Each task acquires its permit
before computing its effective timeout: min(opts.timeoutMs, remaining
envelope budget) measured at permit-acquisition time, not envelope-entry
— a sub-query that waited 30s on a 60s envelope gets <=30s regardless of
its own opts.timeoutMs. The envelope deadline is a select! arm against
sleep_until; when it fires JoinSet::abort_all() cancels in-flight tasks,
remaining slots are drained, and any task that didn't complete is
marked AliasOutcomeKind::Timeout with the envelope budget. Sub-query
errors land in ::Error with a coarse classification — task fluree#5/fluree#6 will
refine the code surface when assembling the HTTP response.

Per-task execution does the merge work: @context and opts merge via the
multi.rs helpers with sub-query winning; the merged context is injected
into JSON-LD bodies (or used to derive SPARQL PREFIX/BASE directives for
SPARQL sub-queries). After merge, apply_snapshot_to_jsonld /
apply_snapshot_to_sparql pin the per-ledger t and the result is passed
to run_jsonld_subquery / run_sparql_subquery — the connection-scoped
helpers from the earlier extraction. TrackingOptions is built once per
sub-query from the merged opts (track_time/fuel/policy + max_fuel) and
threaded through to the SPARQL builder; the JSON-LD path picks up
tracking from has_tracking_opts on the merged body.

Each sub-query gets its own sub_query span (alias, language,
effective_timeout_ms, result_status) as a child of whatever envelope
span the caller establishes. Span tagging happens both at task-start
(effective_timeout_ms once known) and at task-end (result_status).

Visibility: fluree-db-api's  is promoted to pub so the
server can reach query::multi and query::multi_snapshot; siblings
inside that module that were already pub mod stay accessible. The
server's mod query is promoted to pub(crate) so the new multi_dispatch
sibling can reuse the existing run_jsonld_subquery / SubqueryOutput
without duplicating their bodies.

12 snapshot unit tests cover JSON-LD application (string/object/array
from, named-graph fragment preservation, ledger not in map) and SPARQL
application (FROM and FROM NAMED rewrite, IRI inside string literal
untouched, unparseable returns original, ledger not in map, existing
suffix not overwritten). 6 dispatcher unit tests cover DispatchConfig
clamping to server bounds and the per-sub-query timeout fallback
ordering. Existing 47 server integration tests and 506 api lib tests
pass.
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