fix(turtle): make skolem IRIs collision-free and deterministic

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: `<parent>/<slot>` for
+/// a single-valued slot, and `<parent>/<slot>/<member>` 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 `<parent>/0` and get merged into one
+/// node.
 fn identifier_node(
     map: &std::collections::HashMap<String, LinkMLInstance>,
     class: &ClassView,
     conv: &Converter,
     state: &mut State,
     parent: Option<&Node>,
-    index: Option<usize>,
+    slot_name: &str,
+    member: Option<&str>,
 ) -> (Node, Option<String>) {
     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<W: Write>(
                     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<W: Write>(
                                 };
                                 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<W: Write>(
                 }
             }
             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<W: Write>(
                                     conv,
                                     state,
                                     Some(subject),
-                                    Some(idx),
+                                    k,
+                                    Some(member_key.as_str()),
                                 );
                                 let triple = Triple {
                                     subject: subject.as_subject(),

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"

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:

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<String> {
+    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<String> = 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<String> {
+    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 <BASE + class_name>` triple.
+fn subjects_of_type(triples: &[String], class_name: &str) -> Vec<String> {
+    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<String> = BTreeSet::new();
+    found.extend(subjects_of_type(&triples, "Address"));
+    found.extend(subjects_of_type(&triples, "Contact"));
+
+    let expected: BTreeSet<String> = [
+        "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<String> = 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<String> = [
+        "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
+        );
+    }
+}