From efd36e8317363490242d5600b600683566f0c388 Mon Sep 17 00:00:00 2001 From: Frank Dekervel Date: Tue, 25 Aug 2026 11:39:09 +0200 Subject: [PATCH] fix(turtle): make skolem IRIs collision-free and deterministic Skolem IRIs were built from the child's key or its position alone, with the slot it hangs off left out of the path. Two keyless objects under different slots of the same parent therefore got the same IRI and were merged into a single node: sk:r1 sk:homes <.../r1/0> . <.../r1/0> sk:street "home street" . sk:r1 sk:works <.../r1/0> . # same subject <.../r1/0> sk:street "work street" . Objects with neither a key nor a position fell back to a `gen{N}` global counter walked in `HashMap` iteration order, so the same input produced different IRIs on different runs. Inlined mappings were worse still: their members were numbered by enumeration order over a `HashMap`, discarding the mapping key that already identified them. Put the slot in the path and drop the counter. A skolem IRI is now a pure function of the path walked to reach the node: / single-valued slot // keyed member (mapping key or key slot) // keyless list member Path segments keep the RFC 3986 unreserved set intact rather than escaping every non-alphanumeric, so snake_case slot names and keys stay readable; characters that cannot appear in a segment are still percent-encoded. Not addressed: write_turtle's top-level `Mapping` arm numbers members the same way, but `LinkMLInstance::Mapping` carries a mandatory slot and no loader produces one at the document root, so the path is unreachable and left untested rather than changed blind. Co-Authored-By: Claude Opus 5 (1M context) --- src/runtime/src/turtle.rs | 68 +++++--- src/runtime/tests/data/skolem_data.yaml | 14 ++ src/runtime/tests/data/skolem_schema.yaml | 48 ++++++ src/runtime/tests/turtle_skolem.rs | 190 ++++++++++++++++++++++ src/tools/tests/cli.rs | 7 +- 5 files changed, 303 insertions(+), 24 deletions(-) create mode 100644 src/runtime/tests/data/skolem_data.yaml create mode 100644 src/runtime/tests/data/skolem_schema.yaml create mode 100644 src/runtime/tests/turtle_skolem.rs diff --git a/src/runtime/src/turtle.rs b/src/runtime/src/turtle.rs index 9f7df19..57dddaf 100644 --- a/src/runtime/src/turtle.rs +++ b/src/runtime/src/turtle.rs @@ -83,8 +83,18 @@ fn literal_value(v: &JsonValue) -> String { } } +/// Characters that may appear literally in one segment of a skolem IRI path: +/// the RFC 3986 unreserved set (`ALPHA / DIGIT / "-" / "." / "_" / "~"`). +/// Everything else — `/` and space above all — is percent-encoded, so a key +/// value can never introduce a path segment of its own. +const PATH_SEGMENT: &percent_encoding::AsciiSet = &NON_ALPHANUMERIC + .remove(b'-') + .remove(b'.') + .remove(b'_') + .remove(b'~'); + fn encode_path_part(s: &str) -> String { - utf8_percent_encode(s, NON_ALPHANUMERIC).to_string() + utf8_percent_encode(s, PATH_SEGMENT).to_string() } fn slot_predicate_iri(slot: &SlotView, conv: &Converter) -> String { @@ -205,13 +215,26 @@ fn try_lang_tag_collapse( .map(Term::Literal) } +/// Pick the subject node for `map`, an object reached through `slot_name` on +/// `parent`. +/// +/// An object carrying an identifier names itself. Otherwise, in skolem mode the +/// IRI is built from the path walked to reach the object: `/` for +/// a single-valued slot, and `//` for a member of a +/// multivalued one, where `member` is the object's own key if it has one and +/// the caller-supplied discriminator (a mapping key, or a list index) otherwise. +/// +/// The slot has to be in the path: two keyless objects under different slots of +/// the same parent otherwise both land on `/0` and get merged into one +/// node. fn identifier_node( map: &std::collections::HashMap, class: &ClassView, conv: &Converter, state: &mut State, parent: Option<&Node>, - index: Option, + slot_name: &str, + member: Option<&str>, ) -> (Node, Option) { if let Some(id_slot) = class.identifier_slot() { if let Some(LinkMLInstance::Scalar { value, .. }) = map.get(&id_slot.name) { @@ -225,24 +248,22 @@ fn identifier_node( } if state.skolem { if let Some(p) = parent { - let part_opt = class.key_or_identifier_slot().and_then(|ks| { - map.get(&ks.name).and_then(|v| match v { - LinkMLInstance::Scalar { value, .. } => { - if let JsonValue::String(s) = value { - Some(encode_path_part(s)) - } else { + let key_part = class + .key_or_identifier_slot() + .and_then(|ks| { + map.get(&ks.name).and_then(|v| match v { + LinkMLInstance::Scalar { value, .. } => { Some(encode_path_part(&literal_value(value))) } - } - _ => None, + _ => None, + }) }) - }); - let part = part_opt - .or_else(|| index.map(|i| i.to_string())) - .unwrap_or_else(|| { - state.counter += 1; - format!("gen{}", state.counter) - }); + .or_else(|| member.map(encode_path_part)); + let slot_part = encode_path_part(slot_name); + let part = match key_part { + Some(k) => format!("{}/{}", slot_part, k), + None => slot_part, + }; let node = state.child_subject(p, &part); return (node, None); } @@ -341,7 +362,7 @@ fn serialize_map( formatter.serialize_triple(triple.as_ref())?; } else { let (obj, child_id) = - identifier_node(values, class_ref, conv, state, Some(subject), None); + identifier_node(values, class_ref, conv, state, Some(subject), k, None); let triple = Triple { subject: subject.as_subject(), predicate: predicate.clone(), @@ -386,13 +407,17 @@ fn serialize_map( }; formatter.serialize_triple(triple.as_ref())?; } else { + // Position in the list is the discriminator for + // members that carry no key of their own. + let position = idx.to_string(); let (obj, child_id) = identifier_node( mv, class_ref, conv, state, Some(subject), - Some(idx), + k, + Some(position.as_str()), ); let triple = Triple { subject: subject.as_subject(), @@ -418,7 +443,7 @@ fn serialize_map( } } LinkMLInstance::Mapping { values, .. } => { - for (idx, item) in values.values().enumerate() { + for (member_key, item) in values.iter() { match item { LinkMLInstance::Scalar { value: v, slot, .. } => { let triple = Triple { @@ -449,7 +474,8 @@ fn serialize_map( conv, state, Some(subject), - Some(idx), + k, + Some(member_key.as_str()), ); let triple = Triple { subject: subject.as_subject(), diff --git a/src/runtime/tests/data/skolem_data.yaml b/src/runtime/tests/data/skolem_data.yaml new file mode 100644 index 0000000..8023a7a --- /dev/null +++ b/src/runtime/tests/data/skolem_data.yaml @@ -0,0 +1,14 @@ +id: sktest:p1 +home_addresses: + - street: home street +work_addresses: + - street: work street +primary_contact: + phone: "555-0001" +backup_contact: + phone: "555-0002" +accounts: + savings_2024: + balance: "100" + "odd label/with slash": + balance: "200" diff --git a/src/runtime/tests/data/skolem_schema.yaml b/src/runtime/tests/data/skolem_schema.yaml new file mode 100644 index 0000000..68c228c --- /dev/null +++ b/src/runtime/tests/data/skolem_schema.yaml @@ -0,0 +1,48 @@ +id: https://example.com/skolem-test +name: skolem_test +prefixes: + sktest: https://example.com/skolem-test/ + linkml: https://w3id.org/linkml/ +default_prefix: sktest +default_range: string +imports: +- linkml:types +classes: + Person: + tree_root: true + attributes: + id: + identifier: true + # Two multivalued slots over the same keyless class: the list index alone + # is not enough to tell their members apart. + home_addresses: + range: Address + multivalued: true + inlined_as_list: true + work_addresses: + range: Address + multivalued: true + inlined_as_list: true + # Two single-valued slots over the same keyless class: neither a key nor + # an index is available for either one. + primary_contact: + range: Contact + backup_contact: + range: Contact + # A keyed collection: the key, not the index, names the member. The key + # value here holds characters that are not legal in a path segment. + accounts: + range: Account + multivalued: true + inlined: true + Address: + attributes: + street: + Contact: + attributes: + phone: + Account: + attributes: + label: + key: true + balance: diff --git a/src/runtime/tests/turtle_skolem.rs b/src/runtime/tests/turtle_skolem.rs new file mode 100644 index 0000000..bae132b --- /dev/null +++ b/src/runtime/tests/turtle_skolem.rs @@ -0,0 +1,190 @@ +#![cfg(feature = "ttl")] + +//! Skolem IRIs must be a deterministic, collision-free function of the path +//! taken through the instance tree to reach a node. + +use linkml_runtime::{ + load_yaml_file, + turtle::{write_ntriples, TurtleOptions}, +}; +use linkml_schemaview::identifier::{converter_from_schemas, Identifier}; +use linkml_schemaview::io::from_yaml; +use linkml_schemaview::schemaview::SchemaView; +use std::collections::BTreeSet; +use std::path::{Path, PathBuf}; + +const BASE: &str = "https://example.com/skolem-test/"; + +fn data_path(name: &str) -> PathBuf { + let mut p = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + p.push("tests"); + p.push("data"); + p.push(name); + p +} + +/// Load the skolem fixture and serialise it to N-Triples with skolem IRIs on. +/// +/// Everything is rebuilt from scratch on each call so that repeated calls +/// exercise fresh `HashMap`s — the instance maps are hashed with a per-instance +/// `RandomState`, so a serialiser whose output depends on map iteration order +/// gives a different answer on each call. +fn serialize_skolem() -> Vec { + let schema = from_yaml(Path::new(&data_path("skolem_schema.yaml"))).unwrap(); + let types_schema = from_yaml(Path::new(&data_path("types.yaml"))).unwrap(); + let mut sv = SchemaView::new(); + sv.add_schema(schema.clone()).unwrap(); + sv.add_schema_with_import_ref( + types_schema.clone(), + Some((schema.id.clone(), "linkml:types".to_string())), + ) + .unwrap(); + let conv = converter_from_schemas([&schema, &types_schema]); + let class = sv + .get_class(&Identifier::new("Person"), &conv) + .unwrap() + .unwrap(); + let v = load_yaml_file( + Path::new(&data_path("skolem_data.yaml")), + &sv, + &class, + &conv, + ) + .unwrap() + .into_instance() + .unwrap(); + let mut buf = Vec::new(); + write_ntriples( + &v, + &sv, + &schema, + &conv, + &mut buf, + TurtleOptions { skolem: true }, + ) + .unwrap(); + let nt = String::from_utf8(buf).unwrap(); + let mut lines: Vec = nt.lines().map(|l| l.trim().to_string()).collect(); + lines.retain(|l| !l.is_empty()); + lines.sort(); + lines +} + +/// Subject IRI of every triple whose object is the given literal. +fn subjects_with_literal(triples: &[String], literal: &str) -> Vec { + let needle = format!("\"{}\"", literal); + triples + .iter() + .filter(|l| l.contains(&needle)) + .filter_map(|l| l.split('>').next()) + .map(|s| s.trim_start_matches('<').to_string()) + .collect() +} + +/// Subject IRI of every `rdf:type ` triple. +fn subjects_of_type(triples: &[String], class_name: &str) -> Vec { + let needle = format!("<{}{}> .", BASE, class_name); + triples + .iter() + .filter(|l| l.contains("22-rdf-syntax-ns#type") && l.ends_with(&needle)) + .filter_map(|l| l.split('>').next()) + .map(|s| s.trim_start_matches('<').to_string()) + .collect() +} + +#[test] +fn skolem_iris_do_not_collide_across_slots() { + let triples = serialize_skolem(); + + // One Address hangs off home_addresses and one off work_addresses. Both are + // keyless and both sit at index 0 of their own list, so an IRI built from + // the index alone gives them the same name and merges them into one node. + let addresses = subjects_of_type(&triples, "Address"); + let distinct: BTreeSet<&String> = addresses.iter().collect(); + assert_eq!( + distinct.len(), + 2, + "the two addresses share a skolem IRI: {:?}\n{}", + addresses, + triples.join("\n") + ); + + // The same for the two single-valued Contact slots. + let contacts = subjects_of_type(&triples, "Contact"); + let distinct: BTreeSet<&String> = contacts.iter().collect(); + assert_eq!( + distinct.len(), + 2, + "the two contacts share a skolem IRI: {:?}\n{}", + contacts, + triples.join("\n") + ); + + // A collision is visible from the data side too: the merged subject ends up + // carrying both street values. + let home = subjects_with_literal(&triples, "home street"); + let work = subjects_with_literal(&triples, "work street"); + assert_eq!(home.len(), 1, "{}", triples.join("\n")); + assert_eq!(work.len(), 1, "{}", triples.join("\n")); + assert_ne!( + home[0], + work[0], + "home and work address collapsed onto one subject\n{}", + triples.join("\n") + ); +} + +#[test] +fn skolem_iris_encode_the_slot_they_hang_off() { + let triples = serialize_skolem(); + let mut found: BTreeSet = BTreeSet::new(); + found.extend(subjects_of_type(&triples, "Address")); + found.extend(subjects_of_type(&triples, "Contact")); + + let expected: BTreeSet = [ + "p1/home_addresses/0", + "p1/work_addresses/0", + "p1/primary_contact", + "p1/backup_contact", + ] + .iter() + .map(|s| format!("{}{}", BASE, s)) + .collect(); + + assert_eq!(found, expected, "\n{}", triples.join("\n")); +} + +#[test] +fn skolem_iris_are_keyed_by_key_not_index() { + let triples = serialize_skolem(); + let found: BTreeSet = subjects_of_type(&triples, "Account").into_iter().collect(); + + // Slot names and key values are snake_case far more often than not, and + // `_` is unreserved in RFC 3986 — it must survive into the path. Characters + // that genuinely cannot appear in a path segment still get escaped. + let expected: BTreeSet = [ + "p1/accounts/savings_2024", + "p1/accounts/odd%20label%2Fwith%20slash", + ] + .iter() + .map(|s| format!("{}{}", BASE, s)) + .collect(); + + assert_eq!(found, expected, "\n{}", triples.join("\n")); +} + +#[test] +fn skolem_iris_are_stable_across_serializations() { + // A counter walked in hash-map order hands the same node a different IRI on + // each run: the set of IRIs stays {gen1, gen2} but which contact gets which + // flips, so the triple set as a whole changes. A path-derived IRI does not. + let first = serialize_skolem(); + for i in 1..8 { + assert_eq!( + serialize_skolem(), + first, + "skolem IRIs changed on repetition {}", + i + ); + } +} diff --git a/src/tools/tests/cli.rs b/src/tools/tests/cli.rs index f39a23d..68c59b6 100644 --- a/src/tools/tests/cli.rs +++ b/src/tools/tests/cli.rs @@ -23,10 +23,11 @@ fn skolem_flag_creates_named_nodes() { cmd.assert().success(); let ttl = std::fs::read_to_string(&out_path).unwrap(); - // Skolem IRIs with slashes in the local name are kept as full IRIs - // because "poly:root/gen1" is invalid Turtle (slash in local name). + // The keyless child of the `obj` slot is named after the slot it hangs off. + // Skolem IRIs with slashes in the local name are kept as full IRIs because + // "poly:root/obj" is invalid Turtle (slash in local name). assert!( - ttl.contains(""), + ttl.contains(""), "Expected full IRI for skolem node with slash in local name. Got:\n{}", ttl );