Skip to content

Commit f6e2615

Browse files
committed
Distinguish Document path from name and parse URIs robustly
Previously the Document `path` property returned the URI basename, making it identical to a name and mislabeled. Split them: - `uri` -> full document URI (e.g. file:///app/models/user.rb) - `path` -> file-system path (e.g. /app/models/user.rb) - `name` -> base file name (e.g. user.rb) Add `Document::file_path` / `Document::file_name`, which decode the URI via the `url` crate (already a dependency) so percent-encoding and platform paths (including Windows drive paths) are handled correctly instead of naively splitting on '/'. `require_path` now reuses `file_path` instead of re-parsing the URI. Non-file:// URIs (the synthetic built-in document) fall back to the raw URI. Clarify that `prop` is the property name read off a node, and advertise the new `name` property in the schema.
1 parent 15a5462 commit f6e2615

4 files changed

Lines changed: 58 additions & 14 deletions

File tree

rust/rubydex/src/model/document.rs

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,28 @@ impl Document {
8484
self.diagnostics.push(diagnostic);
8585
}
8686

87+
/// The file-system path of this document, decoded from its URI.
88+
///
89+
/// Returns `None` when the URI is not a `file://` URL (e.g. the synthetic built-in document) or
90+
/// cannot be converted to a path. Uses `Url` so percent-encoding and platform-specific paths
91+
/// (including Windows drive paths) are handled correctly.
92+
#[must_use]
93+
pub fn file_path(&self) -> Option<PathBuf> {
94+
let url = Url::parse(&self.uri).ok()?;
95+
if url.scheme() != "file" {
96+
return None;
97+
}
98+
url.to_file_path().ok()
99+
}
100+
101+
/// The base file name of this document (the last path segment), decoded from its URI.
102+
///
103+
/// Returns `None` when the URI has no file path (see [`Document::file_path`]).
104+
#[must_use]
105+
pub fn file_name(&self) -> Option<String> {
106+
Some(self.file_path()?.file_name()?.to_string_lossy().into_owned())
107+
}
108+
87109
/// Computes the require path for this document given load paths.
88110
///
89111
/// Returns `None` if:
@@ -97,12 +119,7 @@ impl Document {
97119
/// Panics if load path entries exceed u16.
98120
#[must_use]
99121
pub fn require_path(&self, load_paths: &[PathBuf]) -> Option<(String, u16)> {
100-
let url = Url::parse(&self.uri).ok()?;
101-
if url.scheme() != "file" {
102-
return None;
103-
}
104-
105-
let file_path = url.to_file_path().ok()?;
122+
let file_path = self.file_path()?;
106123
if file_path.extension().is_none_or(|ext| ext != "rb") {
107124
return None;
108125
}

rust/rubydex/src/query/cypher/schema.rs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -255,13 +255,13 @@ pub fn node_name(graph: &Graph, node: NodeRef) -> String {
255255
.and_then(|decl_id| graph.declarations().get(decl_id))
256256
.map_or_else(String::new, |declaration| declaration.name().to_string()),
257257
NodeRef::Document(id) => graph.documents().get(&id).map_or_else(String::new, |document| {
258-
let uri = document.uri();
259-
uri.rsplit('/').next().unwrap_or(uri).to_string()
258+
document.file_name().unwrap_or_else(|| document.uri().to_string())
260259
}),
261260
}
262261
}
263262

264-
/// Resolves a node property to a value. Unknown properties yield `NULL`.
263+
/// Resolves a node property to a value, where `prop` is the property name read off the node (the
264+
/// `x` in `RETURN n.x` / `WHERE n.x = ...`). Unknown properties yield `NULL`.
265265
#[must_use]
266266
pub fn property(graph: &Graph, node: NodeRef, prop: &str) -> CypherValue {
267267
match prop {
@@ -319,12 +319,18 @@ fn document_property(graph: &Graph, id: UriId, prop: &str) -> CypherValue {
319319
return CypherValue::Null;
320320
};
321321

322+
// Non-`file://` URIs (the synthetic built-in document) have no file path, so `path`/`name` fall
323+
// back to the raw URI.
322324
match prop {
325+
// Full document URI, e.g. `file:///app/models/user.rb`.
323326
"uri" => CypherValue::Str(document.uri().to_string()),
324-
"path" | "name" => {
325-
let uri = document.uri();
326-
CypherValue::Str(uri.rsplit('/').next().unwrap_or(uri).to_string())
327-
}
327+
// File-system path, e.g. `/app/models/user.rb`.
328+
"path" => CypherValue::Str(document.file_path().map_or_else(
329+
|| document.uri().to_string(),
330+
|path| path.to_string_lossy().into_owned(),
331+
)),
332+
// Base file name, e.g. `user.rb`.
333+
"name" => CypherValue::Str(document.file_name().unwrap_or_else(|| document.uri().to_string())),
328334
_ => CypherValue::Null,
329335
}
330336
}

rust/rubydex/src/query/cypher/schema_info.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,12 @@ const PROPERTIES: &[PropInfo] = &[
218218
PropInfo {
219219
node_type: "Document",
220220
property: "path",
221-
description: "Basename of the document URI",
221+
description: "File system path of the document",
222+
},
223+
PropInfo {
224+
node_type: "Document",
225+
property: "name",
226+
description: "Base file name of the document",
222227
},
223228
];
224229

rust/rubydex/src/query/cypher/tests.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,3 +188,19 @@ fn unknown_relationship_type_errors() {
188188
let parsed = parse("MATCH (a)-[:BOGUS]->(b) RETURN a").unwrap();
189189
assert!(execute(&graph, &parsed).is_err());
190190
}
191+
192+
#[test]
193+
fn document_uri_path_and_name_are_distinct() {
194+
let graph = fixture_graph();
195+
let result = run(
196+
&graph,
197+
"MATCH (d:Document) WHERE d.uri = 'file:///zoo.rb' RETURN d.uri, d.path, d.name",
198+
);
199+
assert_eq!(
200+
result.columns,
201+
vec!["d.uri".to_string(), "d.path".to_string(), "d.name".to_string()]
202+
);
203+
assert_eq!(column_strings(&result, 0), vec!["file:///zoo.rb".to_string()]);
204+
assert_eq!(column_strings(&result, 1), vec!["/zoo.rb".to_string()]);
205+
assert_eq!(column_strings(&result, 2), vec!["zoo.rb".to_string()]);
206+
}

0 commit comments

Comments
 (0)