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 );