diff --git a/src/backend/documents.rs b/src/backend/documents.rs index 56836fa37..3fdf4e179 100644 --- a/src/backend/documents.rs +++ b/src/backend/documents.rs @@ -268,6 +268,8 @@ impl Backend { if let Some(content) = self.get_file_content(&uri) { self.update_resource_symbol_index(&uri, &content); } else { + // A buffer never saved leaves no file behind. + self.note_drupal_config_document(&uri, None); self.clear_file_maps(&uri); } } else if let Some(path) = self.workspace_index_path(&uri) { diff --git a/src/drupal/config.rs b/src/drupal/config.rs index 05ac79106..db7924556 100644 --- a/src/drupal/config.rs +++ b/src/drupal/config.rs @@ -11,16 +11,18 @@ //! configuration object under its name, with a `type` of `config_object` or //! of a type built on it, next to reusable data types and the configuration //! entities' wildcard entries (`node.type.*`). It is read when a name is -//! first asked for, and kept until a schema file or the list of default -//! configuration files changes. +//! first asked for. Each schema file is kept as read until its text +//! changes, and only a changed file is read again. use std::collections::HashMap; use std::path::{Path, PathBuf}; -use std::sync::Arc; +use std::sync::{Arc, OnceLock}; -use tower_lsp_server::gen_lsp_types::{Location, Range}; +use tower_lsp_server::gen_lsp_types::{FileChangeType, Location, Range}; +use super::config_keys::{YamlNode, yaml_tree}; use crate::Backend; +use crate::symbol_map::SymbolMap; /// The schema type every configuration object is built on. const CONFIG_OBJECT: &str = "config_object"; @@ -127,7 +129,7 @@ pub(crate) fn schema_lookup_names(name: &str) -> impl Iterator { /// One top-level entry of a schema file. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct SchemaEntry { - pub uri: String, + pub uri: Arc, /// The byte range of the entry's key. pub start: u32, pub end: u32, @@ -136,6 +138,47 @@ pub(crate) struct SchemaEntry { pub label: Option, } +/// One configuration schema file as last read: its top-level entries, the +/// hash of the text they were read from, and the file as a tree once a +/// configuration key is asked for. +#[derive(Debug)] +pub(crate) struct SchemaFile { + uri: Arc, + hash: u64, + entries: Vec<(Arc, Arc)>, + tree: OnceLock>>, +} + +impl SchemaFile { + fn read(uri: Arc, content: &str) -> Self { + let entries = schema_entries(&uri, content) + .into_iter() + .map(|(key, entry)| (Arc::from(key), Arc::new(entry))) + .collect(); + Self { + uri, + hash: SymbolMap::hash_source(content), + entries, + tree: OnceLock::new(), + } + } + + /// The file as a tree, read with `read` the first time. A tree is + /// kept only when read from the text the entries were: its nodes are + /// found by where their entries start. + fn tree(&self, read: impl FnOnce(&str) -> Option>) -> Option>> { + if let Some(tree) = self.tree.get() { + return Some(Arc::clone(tree)); + } + let content = read(&self.uri)?; + let tree = Arc::new(yaml_tree(&content)); + if SymbolMap::hash_source(&content) == self.hash { + let _ = self.tree.set(Arc::clone(&tree)); + } + Some(tree) + } +} + /// The configuration names of a site, and where each is declared. #[derive(Debug, Default)] pub(crate) struct ConfigIndex { @@ -144,10 +187,9 @@ pub(crate) struct ConfigIndex { /// The default configuration files of each name. defaults: HashMap>, /// Each top-level schema entry, by its key, the first one read kept. - schema: HashMap, - /// The schema files read for their keys, by URI (see - /// [`super::config_keys`]). - pub(super) trees: parking_lot::Mutex>>>, + schema: HashMap, Arc>, + /// The schema files it was read from, by URI. + files: HashMap, Arc>, /// The test modules' files that were open when it was read, which /// count only while they are. open_test_files: Vec<(PathBuf, bool)>, @@ -156,14 +198,27 @@ pub(crate) struct ConfigIndex { impl ConfigIndex { /// The index of the default configuration files `defaults` and the /// schema files `schemas`, each with its URI and content. + #[cfg(test)] pub(crate) fn build<'a>( defaults: &[PathBuf], schemas: impl IntoIterator, ) -> Self { + let files = schemas + .into_iter() + .map(|(uri, content)| Arc::new(SchemaFile::read(Arc::from(uri), content))) + .collect(); + Self::combine(defaults, files) + } + + /// The index of the default configuration files `defaults` and the + /// schema files `files`, in that order. + fn combine(defaults: &[PathBuf], files: Vec>) -> Self { let mut index = Self::default(); - for (uri, content) in schemas { - for entry in schema_entries(uri, content) { - index.schema.entry(entry.0).or_insert(entry.1); + for file in &files { + for (key, entry) in &file.entries { + if !index.schema.contains_key(key) { + index.schema.insert(Arc::clone(key), Arc::clone(entry)); + } } } for path in defaults { @@ -181,15 +236,29 @@ impl ConfigIndex { .filter(|(key, entry)| { is_config_name(key) && index.derives_from_config_object(entry.ty.as_deref()) }) - .map(|(key, _)| key.clone()) + .map(|(key, _)| key.to_string()) .chain(index.defaults.keys().cloned()) .collect(); names.sort_unstable(); names.dedup(); index.names = names; + index.files = files + .into_iter() + .map(|file| (Arc::clone(&file.uri), file)) + .collect(); index } + /// The schema file at `uri` as a tree, read with `read` the first time + /// it is asked for. + pub(crate) fn tree( + &self, + uri: &str, + read: impl FnOnce(&str) -> Option>, + ) -> Option>> { + self.files.get(uri)?.tree(read) + } + /// Every configuration name, sorted. pub(crate) fn names(&self) -> &[String] { &self.names @@ -197,7 +266,7 @@ impl ConfigIndex { /// The schema entry under the key `key` itself. pub(crate) fn schema_named(&self, key: &str) -> Option<&SchemaEntry> { - self.schema.get(key) + self.schema.get(key).map(Arc::as_ref) } /// The default configuration files that ship `name`. @@ -209,8 +278,8 @@ impl ConfigIndex { /// its own, or the first wildcard entry that matches it. pub(crate) fn schema_entry(&self, name: &str) -> Option<(String, &SchemaEntry)> { schema_lookup_names(name).find_map(|key| { - let entry = self.schema.get(&key)?; - Some((key, entry)) + let entry = self.schema.get(key.as_str())?; + Some((key, entry.as_ref())) }) } @@ -242,7 +311,7 @@ fn is_config_name(key: &str) -> bool { } /// The top-level entries of a schema file, each with its key. -fn schema_entries(uri: &str, content: &str) -> Vec<(String, SchemaEntry)> { +fn schema_entries(uri: &Arc, content: &str) -> Vec<(String, SchemaEntry)> { let mut entries: Vec<(String, SchemaEntry)> = Vec::new(); // The indentation of the current entry's settings, once seen. let mut child_indent: Option = None; @@ -263,7 +332,7 @@ fn schema_entries(uri: &str, content: &str) -> Vec<(String, SchemaEntry)> { entries.push(( key.to_string(), SchemaEntry { - uri: uri.to_string(), + uri: Arc::clone(uri), start, end: start + key.len() as u32, ty: None, @@ -309,56 +378,284 @@ pub(super) fn yaml_key_end(text: &str) -> Option { None } -impl Backend { - /// Remember the default configuration files and schema files the - /// Drupal walk found. - pub(crate) fn note_drupal_config_files(&self, defaults: Vec, schemas: Vec) { - self.drupal.default_config_files.write().extend(defaults); - self.drupal.config_schema_files.write().extend(schemas); - *self.drupal.config_index.write() = None; +/// What the configuration index is read from: the default configuration +/// files and schema files the walk found, each schema file as last read, +/// and the index built from them. A change to one schema file reads that +/// file again and no other. +#[derive(Default)] +pub(crate) struct ConfigSources { + inner: parking_lot::RwLock, +} + +#[derive(Default)] +struct Sources { + defaults: Vec, + /// Each schema file with the URI it is read by. + schemas: Vec<(PathBuf, Arc)>, + /// The schema files read, by URI, less the ones changed since. + files: HashMap, Arc>, + index: Option>, + /// Moves on every change to the files or their lists, so that a build + /// that overlapped one stores no index. + generation: u64, + /// The generation each schema file last changed in, so that a build + /// that overlapped a change keeps the other files it read. + changed_in: HashMap, u64>, + /// The generation the files were last all forgotten in. + cleared_in: u64, +} + +impl Sources { + fn changed(&mut self) { + self.index = None; + self.generation += 1; } - /// The configuration names of the site, and where each is declared, - /// read on first use. - pub(crate) fn drupal_config_index(&self) -> Arc { - if let Some(index) = self.drupal.config_index.read().as_ref() { - return Arc::clone(index); + /// Forget the schema file at `uri` as read. + fn evict(&mut self, uri: &str) { + self.changed(); + self.files.remove(uri); + match self.changed_in.get_mut(uri) { + Some(generation) => *generation = self.generation, + None => { + self.changed_in.insert(Arc::from(uri), self.generation); + } } - let mut defaults = self.drupal.default_config_files.read().clone(); - let mut schema_files = self.drupal.config_schema_files.read().clone(); + } + + fn is_listed(&self, uri: &str) -> bool { + self.schemas.iter().any(|(_, listed)| &**listed == uri) + } +} + +/// A default configuration file or schema file created, changed, or +/// deleted on disk. +pub(crate) struct ConfigFileEvent { + pub path: PathBuf, + pub is_schema: bool, + pub kind: FileChangeType, + /// Whether it is open, when its buffer is read rather than the disk. + pub is_open: bool, +} + +impl ConfigSources { + /// Add the default configuration files and schema files the walk found. + pub(crate) fn add_files(&self, defaults: Vec, schemas: Vec) { + let schemas: Vec<(PathBuf, Arc)> = schemas + .into_iter() + .map(|path| { + let uri = Arc::from(crate::util::path_to_uri(&path)); + (path, uri) + }) + .collect(); + let mut sources = self.inner.write(); + sources.defaults.extend(defaults); + sources.schemas.extend(schemas); + sources.changed(); + } + + /// Forget every file. + pub(crate) fn clear(&self) { + let mut sources = self.inner.write(); + sources.defaults.clear(); + sources.schemas.clear(); + sources.files.clear(); + sources.changed_in.clear(); + sources.changed(); + sources.cleared_in = sources.generation; + } + + /// The configuration index, built on first use from the files found and + /// the open test modules' files `open_test_files` gives. Only the + /// schema files not read yet, or changed since, are read with `read`. + pub(crate) fn index( + &self, + read: impl Fn(&str) -> Option>, + open_test_files: impl FnOnce() -> Vec<(PathBuf, bool)>, + ) -> Arc { + // Read before the open files are, so that one opening meanwhile + // keeps this build from being stored. + let generation = { + let sources = self.inner.read(); + if let Some(index) = &sources.index { + return Arc::clone(index); + } + sources.generation + }; // A test module's files count while they are open, as its other // declaration files do. - let open_test_files = self.open_drupal_test_config_files(); - for (path, is_schema) in open_test_files.iter().cloned() { - let files = if is_schema { - &mut schema_files - } else { - &mut defaults - }; - if !files.contains(&path) { - files.push(path); + let open_test_files = open_test_files(); + let (defaults, schemas) = { + let sources = self.inner.read(); + if let Some(index) = &sources.index { + return Arc::clone(index); } - } - let schemas: Vec<(String, Arc)> = schema_files - .iter() - .filter_map(|path| { - let uri = crate::util::path_to_uri(path); - let content = self.get_file_content_arc(&uri)?; - Some((uri, content)) + let mut defaults = sources.defaults.clone(); + let mut schemas: Vec<(Arc, Option>)> = sources + .schemas + .iter() + .map(|(_, uri)| (Arc::clone(uri), sources.files.get(uri).cloned())) + .collect(); + for (path, is_schema) in &open_test_files { + if !*is_schema { + if !defaults.contains(path) { + defaults.push(path.clone()); + } + } else if !sources.schemas.iter().any(|(listed, _)| listed == path) { + let uri: Arc = Arc::from(crate::util::path_to_uri(path)); + let read = sources.files.get(&uri).cloned(); + schemas.push((uri, read)); + } + } + (defaults, schemas) + }; + let mut read_now = Vec::new(); + let files = schemas + .into_iter() + .filter_map(|(uri, known)| match known { + Some(file) => Some(file), + None => { + let content = read(&uri)?; + let file = Arc::new(SchemaFile::read(uri, &content)); + read_now.push(Arc::clone(&file)); + Some(file) + } }) .collect(); - let mut index = ConfigIndex::build( - &defaults, - schemas - .iter() - .map(|(uri, content)| (uri.as_str(), content.as_str())), - ); + let mut index = ConfigIndex::combine(&defaults, files); index.open_test_files = open_test_files; let index = Arc::new(index); - *self.drupal.config_index.write() = Some(Arc::clone(&index)); + let mut sources = self.inner.write(); + if sources.generation == generation { + if let Some(stored) = &sources.index { + return Arc::clone(stored); + } + sources.files = index.files.clone(); + sources.index = Some(Arc::clone(&index)); + } else if sources.cleared_in <= generation { + // Keep the files read that did not change meanwhile. + for file in read_now { + if sources + .changed_in + .get(&file.uri) + .is_none_or(|changed| *changed <= generation) + { + sources.files.entry(Arc::clone(&file.uri)).or_insert(file); + } + } + } index } + /// Follow a document event for the default configuration or schema + /// file at `uri`, spelled as the walk spells its path, which now holds + /// `content`, or is gone (`None`): a schema file is read again when its + /// text changed, and the index is read again when a test module's file + /// opened or closed, which `open_test_files` tells. + pub(crate) fn record_document( + &self, + uri: &str, + content: Option<&str>, + open_test_files: impl FnOnce() -> Vec<(PathBuf, bool)>, + ) { + if uri.contains("/config/schema/") { + let stale = { + let sources = self.inner.read(); + match sources.files.get(uri) { + Some(file) => { + content.is_none_or(|content| SymbolMap::hash_source(content) != file.hash) + } + // A file not read yet matters when a build may be + // reading it: a listed one, or a test module's, which + // is read while it is open. + None => uri.contains("/tests/") || sources.is_listed(uri), + } + }; + if stale { + self.inner.write().evict(uri); + return; + } + } + // Only a test module's file opening or closing changes which are + // open. + if !uri.contains("/tests/") { + return; + } + let Some(index) = self.inner.read().index.clone() else { + // A build under way may have seen the files open before. + self.inner.write().changed(); + return; + }; + if index.open_test_files != open_test_files() { + self.inner.write().changed(); + } + } + + /// Follow default configuration files and schema files created, + /// changed, or deleted on disk. A default configuration file gives its + /// name by its path alone, so only its creation or deletion counts. A + /// schema file keeps its place in the list unless its last event in the + /// batch deletes it, which decides the entry kept for a key two files + /// declare, and an open one follows its buffer, not the disk. + pub(crate) fn record_watched_changes( + &self, + changes: impl IntoIterator, + ) { + let mut defaults: Vec = Vec::new(); + let mut schemas: Vec = Vec::new(); + for change in changes { + if !change.is_schema { + if change.kind != FileChangeType::Changed { + defaults.push(change); + } + } else if let Some(last) = schemas.iter_mut().find(|last| last.path == change.path) { + *last = change; + } else { + schemas.push(change); + } + } + if defaults.is_empty() && schemas.is_empty() { + return; + } + let mut sources = self.inner.write(); + for change in defaults { + sources.defaults.retain(|known| *known != change.path); + if change.kind != FileChangeType::Deleted { + sources.defaults.push(change.path); + } + sources.changed(); + } + for change in schemas { + let uri = crate::util::path_to_uri(&change.path); + let listed = sources.is_listed(&uri); + if change.kind == FileChangeType::Deleted { + sources.schemas.retain(|(known, _)| *known != change.path); + } else if !listed { + sources.schemas.push((change.path, Arc::from(uri.as_str()))); + } else if change.kind == FileChangeType::Changed && change.is_open { + continue; + } + sources.evict(&uri); + } + } +} + +impl Backend { + /// Remember the default configuration files and schema files the + /// Drupal walk found. + pub(crate) fn note_drupal_config_files(&self, defaults: Vec, schemas: Vec) { + self.drupal.config.add_files(defaults, schemas); + } + + /// The configuration names of the site, and where each is declared, + /// read on first use. + pub(crate) fn drupal_config_index(&self) -> Arc { + self.drupal.config.index( + |uri| self.get_file_content_arc(uri), + || self.open_drupal_test_config_files(), + ) + } + /// The open default configuration and schema files of test modules, /// each with whether it is a schema file, sorted. fn open_drupal_test_config_files(&self) -> Vec<(PathBuf, bool)> { @@ -445,55 +742,43 @@ impl Backend { } /// Keep the configuration files in step with files created, changed, - /// or deleted on disk: a default configuration file created or deleted - /// adds or removes its name, and any change to a schema file is read - /// again. + /// or deleted on disk. pub(crate) fn note_drupal_config_changes( &self, changes: &[tower_lsp_server::gen_lsp_types::FileEvent], ) { - use tower_lsp_server::gen_lsp_types::FileChangeType; let roots = self.drupal.web_roots.read().clone(); - let mut changed = false; - for change in changes { - let path_str = change.uri.path(); - if !path_str.ends_with(".yml") || !path_str.contains("/config/") { - continue; - } - let Some(path) = crate::util::url_to_path(&change.uri) else { - continue; - }; - let Some(root) = roots.iter().find(|root| path.starts_with(root)) else { - continue; - }; - let list = if is_default_config_file(&path, root) { - if change.kind == FileChangeType::Changed { - continue; + self.drupal + .config + .record_watched_changes(changes.iter().filter_map(|change| { + let path_str = change.uri.path(); + if !path_str.ends_with(".yml") || !path_str.contains("/config/") { + return None; } - &self.drupal.default_config_files - } else if is_schema_file(&path, root) { - &self.drupal.config_schema_files - } else { - continue; - }; - let mut files = list.write(); - files.retain(|known| *known != path); - if change.kind != FileChangeType::Deleted { - files.push(path); - } - changed = true; - } - if changed { - *self.drupal.config_index.write() = None; - } + let path = crate::util::url_to_path(&change.uri)?; + let root = roots.iter().find(|root| path.starts_with(root))?; + let is_schema = if is_default_config_file(&path, root) { + false + } else if is_schema_file(&path, root) { + true + } else { + return None; + }; + Some(ConfigFileEvent { + path, + is_schema, + kind: change.kind, + is_open: self.open_files.read().contains_key(change.uri.as_str()), + }) + })); } - /// Read the configuration again the next time a name is asked for when - /// a document change can change it: a schema file is read from its - /// buffer while it is open, and a test module's files count while they - /// are. A default configuration file gives its name by its path - /// alone, so editing one changes nothing. - pub(crate) fn note_drupal_config_document(&self, uri: &str) { + /// Follow a document change that can change the configuration: a + /// schema file is read from its buffer while it is open, and a test + /// module's files count while they are. A default configuration file + /// gives its name by its path alone, so editing one changes nothing. + /// `content` is `None` once the file is gone. + pub(crate) fn note_drupal_config_document(&self, uri: &str, content: Option<&str>) { if !self.is_drupal_project() || !uri.ends_with(".yml") || !["/config/install/", "/config/optional/", "/config/schema/"] @@ -502,14 +787,29 @@ impl Backend { { return; } - let stale = match self.drupal.config_index.read().as_ref() { - None => false, - Some(_) if uri.contains("/config/schema/") => true, - Some(index) => index.open_test_files != self.open_drupal_test_config_files(), + let spelled; + let uri = if uri.contains("/config/schema/") + && let Some(walked) = self.spell_as_walked(uri) + { + spelled = walked; + spelled.as_str() + } else { + uri }; - if stale { - *self.drupal.config_index.write() = None; - } + self.drupal + .config + .record_document(uri, content, || self.open_drupal_test_config_files()); + } + + /// `uri` spelled the way the walk spells its file's path: through the + /// directory links it followed, as watched-file events are spelled. + fn spell_as_walked(&self, uri: &str) -> Option { + let path = crate::util::uri_to_path(uri)?; + let path = self + .followed_links() + .to_link_spelling(&path) + .unwrap_or(path); + Some(crate::util::path_to_uri(&path)) } } @@ -541,7 +841,7 @@ mod tests { #[test] fn a_key_holding_a_colon_is_read_whole() { - let entries = schema_entries("u", SCHEMA); + let entries = schema_entries(&Arc::from("u"), SCHEMA); assert!(entries.iter().any(|(key, _)| key == "acme.menu_block:*")); let (_, settings) = entries .iter() @@ -604,6 +904,311 @@ mod tests { assert!(index.schema_entry("other.thing").is_none()); } + /// Schema files by URI, as a reader that counts how often each is read. + #[derive(Default)] + struct Disk { + texts: parking_lot::Mutex>>, + reads: parking_lot::Mutex>, + } + + impl Disk { + fn write(&self, uri: &str, text: &str) { + self.texts + .lock() + .insert(uri.to_string(), Arc::new(text.to_string())); + } + + fn read(&self, uri: &str) -> Option> { + *self.reads.lock().entry(uri.to_string()).or_default() += 1; + self.texts.lock().get(uri).cloned() + } + + fn reads(&self, uri: &str) -> usize { + self.reads.lock().get(uri).copied().unwrap_or_default() + } + } + + /// The schema file of the extension `name`, and its URI. + fn schema_file(name: &str) -> (PathBuf, String) { + let path = std::env::temp_dir() + .join("phpantom-config-sources") + .join(name) + .join("config/schema") + .join(format!("{name}.schema.yml")); + let uri = crate::util::path_to_uri(&path); + (path, uri) + } + + /// A watched-file event for the schema file of the extension `name`. + fn on_disk(name: &str, kind: FileChangeType, is_open: bool) -> ConfigFileEvent { + ConfigFileEvent { + path: schema_file(name).0, + is_schema: true, + kind, + is_open, + } + } + + const ACME: &str = "acme.settings:\n type: config_object\n label: 'Acme settings'\n mapping:\n notify:\n type: string\nacme.shared:\n type: config_object\n label: 'From acme'\n"; + const EXAMPLE: &str = "example.settings:\n type: config_object\n mapping:\n enabled:\n type: boolean\nacme.shared:\n type: config_object\n label: 'From example'\n"; + + /// Two schema files, `acme` before `example`, read once. + fn two_files() -> (ConfigSources, Disk, String, String) { + let (acme, acme_uri) = schema_file("acme"); + let (example, example_uri) = schema_file("example"); + let disk = Disk::default(); + disk.write(&acme_uri, ACME); + disk.write(&example_uri, EXAMPLE); + let sources = ConfigSources::default(); + sources.add_files(Vec::new(), vec![acme, example]); + (sources, disk, acme_uri, example_uri) + } + + #[test] + fn an_edit_reads_only_the_edited_schema_file_again() { + let (sources, disk, acme, example) = two_files(); + let read = |uri: &str| disk.read(uri); + let index = sources.index(read, Vec::new); + let tree = index.tree(&example, read).expect("the tree"); + assert_eq!((disk.reads(&acme), disk.reads(&example)), (1, 2)); + + let edited = "acme.settings:\n type: config_object\n label: 'Acme edited'\nacme.added:\n type: config_object\nacme.shared:\n type: config_object\n label: 'Still acme'\n"; + disk.write(&acme, edited); + sources.record_document(&acme, Some(edited), Vec::new); + let after = sources.index(read, Vec::new); + assert!(!Arc::ptr_eq(&index, &after)); + assert_eq!((disk.reads(&acme), disk.reads(&example)), (2, 2)); + assert!(Arc::ptr_eq( + &tree, + &after.tree(&example, read).expect("the tree") + )); + assert_eq!(disk.reads(&example), 2); + + assert_eq!( + after.names(), + [ + "acme.added", + "acme.settings", + "acme.shared", + "example.settings" + ] + ); + let label = |key: &str| { + after + .schema_named(key) + .and_then(|entry| entry.label.clone()) + }; + assert_eq!(label("acme.settings").as_deref(), Some("Acme edited")); + assert_eq!(label("acme.shared").as_deref(), Some("Still acme")); + let fresh = ConfigIndex::build(&[], [(acme.as_str(), edited), (example.as_str(), EXAMPLE)]); + assert_eq!(after.names(), fresh.names()); + assert_eq!(after.schema, fresh.schema); + } + + #[test] + fn an_event_with_the_text_read_keeps_the_index() { + let (sources, disk, acme, _) = two_files(); + let read = |uri: &str| disk.read(uri); + let index = sources.index(read, Vec::new); + sources.record_document(&acme, Some(ACME), Vec::new); + assert!(Arc::ptr_eq(&index, &sources.index(read, Vec::new))); + assert_eq!(disk.reads(&acme), 1); + } + + /// An event is judged by the text it carries, which the buffer read + /// may not hold yet. + #[test] + fn an_event_is_judged_by_its_own_text() { + let (sources, disk, acme, _) = two_files(); + let read = |uri: &str| disk.read(uri); + let index = sources.index(read, Vec::new); + disk.write(&acme, "acme.other:\n type: config_object\n"); + sources.record_document(&acme, Some(ACME), Vec::new); + assert!(Arc::ptr_eq(&index, &sources.index(read, Vec::new))); + + disk.write(&acme, ACME); + sources.record_document( + &acme, + Some("acme.other:\n type: config_object\n"), + Vec::new, + ); + assert!(!Arc::ptr_eq(&index, &sources.index(read, Vec::new))); + assert_eq!(disk.reads(&acme), 2); + } + + /// A key two files declare keeps the entry of the first file listed + /// after either changes on disk. + #[test] + fn a_schema_file_changed_on_disk_keeps_its_place() { + let (sources, disk, acme, example) = two_files(); + let read = |uri: &str| disk.read(uri); + sources.index(read, Vec::new); + sources.record_watched_changes([on_disk("acme", FileChangeType::Changed, false)]); + let index = sources.index(read, Vec::new); + assert_eq!((disk.reads(&acme), disk.reads(&example)), (2, 1)); + let shared = index.schema_named("acme.shared").expect("the entry"); + assert_eq!(&*shared.uri, acme); + assert_eq!(shared.label.as_deref(), Some("From acme")); + } + + /// An editor that saves by deleting a file and writing it again, and a + /// checkout, report both in one batch. + #[test] + fn a_schema_file_deleted_and_created_at_once_keeps_its_place() { + let (sources, disk, acme, _) = two_files(); + let read = |uri: &str| disk.read(uri); + sources.index(read, Vec::new); + sources.record_watched_changes([ + on_disk("acme", FileChangeType::Deleted, false), + on_disk("acme", FileChangeType::Created, false), + ]); + let index = sources.index(read, Vec::new); + assert_eq!(disk.reads(&acme), 2); + let shared = index.schema_named("acme.shared").expect("the entry"); + assert_eq!(&*shared.uri, acme); + + sources.record_watched_changes([ + on_disk("acme", FileChangeType::Created, false), + on_disk("acme", FileChangeType::Deleted, false), + ]); + let index = sources.index(read, Vec::new); + assert!(index.schema_named("acme.settings").is_none()); + } + + /// An open schema file is read from its buffer, which a change on disk + /// leaves as it is. + #[test] + fn a_change_on_disk_to_an_open_schema_file_keeps_the_index() { + let (sources, disk, acme, _) = two_files(); + let read = |uri: &str| disk.read(uri); + let index = sources.index(read, Vec::new); + sources.record_watched_changes([on_disk("acme", FileChangeType::Changed, true)]); + assert!(Arc::ptr_eq(&index, &sources.index(read, Vec::new))); + assert_eq!(disk.reads(&acme), 1); + } + + /// A schema file of an extension Drupal does not load is no source. + #[test] + fn an_edit_to_a_schema_file_not_listed_keeps_the_index() { + let (sources, disk, _, _) = two_files(); + let read = |uri: &str| disk.read(uri); + let index = sources.index(read, Vec::new); + let (_, other) = schema_file("other"); + sources.record_document(&other, Some("other.settings: {}\n"), Vec::new); + assert!(Arc::ptr_eq(&index, &sources.index(read, Vec::new))); + } + + /// A build that overlaps an edit returns what it read, and keeps only + /// the files the edit did not change. + #[test] + fn a_build_overlapping_an_edit_is_not_kept() { + let (sources, disk, acme, example) = two_files(); + let edited = "acme.settings:\n type: config_object\n label: 'Acme edited'\n"; + let edit_once = std::sync::Once::new(); + let read = |uri: &str| { + if uri == example { + edit_once.call_once(|| { + disk.write(&acme, edited); + sources.record_document(&acme, Some(edited), Vec::new); + }); + } + disk.read(uri) + }; + let overlapped = sources.index(read, Vec::new); + let label = |index: &ConfigIndex| { + index + .schema_named("acme.settings") + .and_then(|entry| entry.label.clone()) + }; + assert_eq!(label(&overlapped).as_deref(), Some("Acme settings")); + + let index = sources.index(read, Vec::new); + assert!(!Arc::ptr_eq(&overlapped, &index)); + assert_eq!(label(&index).as_deref(), Some("Acme edited")); + assert_eq!((disk.reads(&acme), disk.reads(&example)), (2, 1)); + assert!(Arc::ptr_eq(&index, &sources.index(read, Vec::new))); + } + + /// A test module's file that opens while a build asks which are open + /// counts from the next build on. + #[test] + fn a_build_overlapping_a_test_file_opening_is_not_kept() { + let (sources, disk, _, _) = two_files(); + let read = |uri: &str| disk.read(uri); + let path = std::env::temp_dir().join( + "phpantom-config-sources/acme/tests/modules/acme_test/config/install/acme_test.settings.yml", + ); + let uri = crate::util::path_to_uri(&path); + let opened = parking_lot::Mutex::new(Vec::new()); + let open_test_files = || { + let before = opened.lock().clone(); + if before.is_empty() { + opened.lock().push((path.clone(), false)); + sources.record_document(&uri, Some("a: b\n"), || opened.lock().clone()); + } + before + }; + let overlapped = sources.index(read, open_test_files); + assert!( + !overlapped + .names() + .contains(&"acme_test.settings".to_string()) + ); + let index = sources.index(read, open_test_files); + assert!(index.names().contains(&"acme_test.settings".to_string())); + } + + /// A test module's open schema file, edited while a build reads it, + /// is read again by the next build. + #[test] + fn a_test_schema_file_edited_while_read_is_read_again() { + let (sources, disk, _, _) = two_files(); + let path = std::env::temp_dir().join( + "phpantom-config-sources/acme/tests/modules/acme_test/config/schema/acme_test.schema.yml", + ); + let uri = crate::util::path_to_uri(&path); + disk.write(&uri, "acme_test.first:\n type: config_object\n"); + let edited = "acme_test.edited:\n type: config_object\n"; + let edit_once = std::sync::Once::new(); + let read = |read_uri: &str| { + let text = disk.read(read_uri); + if read_uri == uri { + edit_once.call_once(|| { + disk.write(&uri, edited); + sources.record_document(&uri, Some(edited), Vec::new); + }); + } + text + }; + let open_test_files = || vec![(path.clone(), true)]; + let overlapped = sources.index(read, open_test_files); + assert!(overlapped.names().contains(&"acme_test.first".to_string())); + let index = sources.index(read, open_test_files); + assert!(index.names().contains(&"acme_test.edited".to_string())); + assert!(!index.names().contains(&"acme_test.first".to_string())); + } + + /// A tree is found by where the entries start, so one read from newer + /// text than they were is used once and not kept. + #[test] + fn a_tree_of_newer_text_than_the_entries_is_not_kept() { + let (sources, disk, acme, _) = two_files(); + let read = |uri: &str| disk.read(uri); + let index = sources.index(read, Vec::new); + disk.write(&acme, "acme.other:\n type: config_object\n"); + let newer = index.tree(&acme, read).expect("the tree"); + assert_eq!(newer[0].key, "acme.other"); + + disk.write(&acme, ACME); + let tree = index.tree(&acme, read).expect("the tree"); + assert_eq!(tree[0].key, "acme.settings"); + assert!(Arc::ptr_eq( + &tree, + &index.tree(&acme, read).expect("the tree") + )); + assert_eq!(disk.reads(&acme), 3); + } + #[test] fn default_and_schema_files_are_found_by_their_folders() { let root = Path::new("/site/web"); diff --git a/src/drupal/config_keys.rs b/src/drupal/config_keys.rs index ce54f3365..db80edb0b 100644 --- a/src/drupal/config_keys.rs +++ b/src/drupal/config_keys.rs @@ -5,11 +5,9 @@ //! //! A key's span is keyed by the configuration name as well as the key //! (see [`config_key`]): `enabled` only means something under its object. -//! The schema files are read when a key is first asked for, and kept with -//! the configuration index (see [`super::config::ConfigIndex`]), which a -//! changed schema file drops. - -use std::sync::Arc; +//! A schema file is read as a tree when a key is first asked for, and kept +//! with the file's entries in the configuration index (see +//! [`super::config::ConfigIndex`]) until its text changes. use tower_lsp_server::gen_lsp_types::{Location, Range}; @@ -131,25 +129,18 @@ fn is_named_type(ty: &str) -> bool { !matches!(ty, "mapping" | "sequence") && !ty.contains(['[', '%']) } -impl Backend { - /// The parsed schema file at `uri`, read once per configuration index. - fn schema_tree( - &self, - index: &super::config::ConfigIndex, - uri: &str, - ) -> Option>> { - if let Some(tree) = index.trees.lock().get(uri) { - return Some(Arc::clone(tree)); - } - let content = self.get_file_content_arc(uri)?; - let tree = Arc::new(yaml_tree(&content)); - index - .trees - .lock() - .insert(uri.to_string(), Arc::clone(&tree)); - Some(tree) - } +/// The top-level node of `tree` the schema entry `entry`, under `key`, was +/// read from: none when the tree was read from other text. +fn entry_node<'a>( + tree: &'a [YamlNode], + key: &str, + entry: &super::config::SchemaEntry, +) -> Option<&'a YamlNode> { + tree.iter() + .find(|node| node.start == entry.start as usize && node.key == key) +} +impl Backend { /// The top-level node of the schema entry `key`. fn schema_node( &self, @@ -157,12 +148,9 @@ impl Backend { key: &str, ) -> Option<(String, YamlNode)> { let entry = index.schema_named(key)?; - let tree = self.schema_tree(index, &entry.uri)?; - let node = tree - .iter() - .find(|node| node.start == entry.start as usize)? - .clone(); - Some((entry.uri.clone(), node)) + let tree = index.tree(&entry.uri, |uri| self.get_file_content_arc(uri))?; + let node = entry_node(&tree, key, entry)?.clone(); + Some((entry.uri.to_string(), node)) } /// The keys `node`, written in the file at `uri`, maps: its own, then @@ -327,6 +315,25 @@ mod tests { assert_eq!(tree[0].setting("label"), Some("Acme")); } + /// A tree read from newer text than the entries may hold another key + /// where the entry's key was. + #[test] + fn an_entry_is_found_only_under_its_own_key() { + let tree = yaml_tree("acme.other:\n type: config_object\n"); + let entry = super::super::config::SchemaEntry { + uri: std::sync::Arc::from("file:///acme.schema.yml"), + start: 0, + end: 13, + ty: Some("config_object".to_string()), + label: None, + }; + assert!(entry_node(&tree, "acme.settings", &entry).is_none()); + assert_eq!( + entry_node(&tree, "acme.other", &entry).map(|node| node.key.as_str()), + Some("acme.other") + ); + } + #[test] fn a_key_names_its_object() { let key = config_key("acme.settings", "notifications.email"); diff --git a/src/drupal/mod.rs b/src/drupal/mod.rs index 8462b429f..b87a9638b 100644 --- a/src/drupal/mod.rs +++ b/src/drupal/mod.rs @@ -52,14 +52,9 @@ pub(crate) struct DrupalState { /// The services each service provider's `alter()` gives another class, /// keyed by the provider's URI; see [`service_providers`]. service_replacements: RwLock>>, - /// The default configuration files of every extension, which are - /// never parsed: their names are configuration names. - default_config_files: RwLock>, - /// The configuration schema files, read when a configuration name is - /// first asked for. - config_schema_files: RwLock>, - /// The configuration names read from both, until one of them changes. - config_index: RwLock>>, + /// The default configuration files and configuration schema files of + /// every extension, and the configuration names read from them. + config: config::ConfigSources, } /// A kind of file Drupal reads declarations from. @@ -322,9 +317,7 @@ impl Backend { self.drupal.hook_php_files.write().clear(); *self.drupal.extensions.write() = hooks::ExtensionIndex::default(); self.drupal.service_replacements.write().clear(); - self.drupal.default_config_files.write().clear(); - self.drupal.config_schema_files.write().clear(); - *self.drupal.config_index.write() = None; + self.drupal.config.clear(); } /// Remember the declaration files the Drupal walk found: the YAML for diff --git a/src/drupal/tests.rs b/src/drupal/tests.rs index 4aae445e5..dcd7a3e6d 100644 --- a/src/drupal/tests.rs +++ b/src/drupal/tests.rs @@ -79,16 +79,16 @@ fn only_what_changes_the_names_drops_the_configuration_index() { let schema = "web/modules/custom/acme/config/schema/acme.schema.yml"; let fixture = "web/core/modules/system/tests/modules/config_test/config/install/config_test.system.yml"; - backend.note_drupal_config_files(vec![default.clone()], Vec::new()); + backend.note_drupal_config_files(vec![default.clone()], vec![dir.path().join(schema)]); let first = backend.drupal_config_index(); - backend.note_drupal_config_document(&crate::util::path_to_uri(&default)); + backend.note_drupal_config_document(&crate::util::path_to_uri(&default), Some("")); assert!(std::sync::Arc::ptr_eq( &first, &backend.drupal_config_index() )); - backend.note_drupal_config_document(&uri(dir.path(), schema)); + backend.note_drupal_config_document(&uri(dir.path(), schema), Some("acme.settings: {}\n")); let second = backend.drupal_config_index(); assert!(!std::sync::Arc::ptr_eq(&first, &second)); @@ -97,19 +97,19 @@ fn only_what_changes_the_names_drops_the_configuration_index() { fixture_uri.clone(), std::sync::Arc::new("a: b\n".to_string()), ); - backend.note_drupal_config_document(&fixture_uri); + backend.note_drupal_config_document(&fixture_uri, Some("a: b\n")); let opened = backend.drupal_config_index(); assert!(!std::sync::Arc::ptr_eq(&second, &opened)); assert!(opened.names().contains(&"config_test.system".to_string())); // Typing in it changes no name. - backend.note_drupal_config_document(&fixture_uri); + backend.note_drupal_config_document(&fixture_uri, Some("a: b\n")); assert!(std::sync::Arc::ptr_eq( &opened, &backend.drupal_config_index() )); backend.open_files.write().remove(&fixture_uri); - backend.note_drupal_config_document(&fixture_uri); + backend.note_drupal_config_document(&fixture_uri, Some("a: b\n")); assert!( !backend .drupal_config_index() diff --git a/src/resource_navigation.rs b/src/resource_navigation.rs index b92f89ced..41619367c 100644 --- a/src/resource_navigation.rs +++ b/src/resource_navigation.rs @@ -163,7 +163,7 @@ impl Backend { /// one. pub(crate) fn update_resource_symbol_index(&self, uri: &str, content: &str) { self.note_twig_source(uri, content); - self.note_drupal_config_document(uri); + self.note_drupal_config_document(uri, Some(content)); if self.is_resource_uri(uri) { self.publish_resource_symbol_index(uri, content); } else if self.symbol_map_for(uri).is_some() { diff --git a/tests/integration/drupal_config.rs b/tests/integration/drupal_config.rs index 8fdcb7736..c3aed1ad0 100644 --- a/tests/integration/drupal_config.rs +++ b/tests/integration/drupal_config.rs @@ -394,6 +394,42 @@ async fn an_open_test_module_offers_its_names() { ); } +/// A test module's configuration file never saved stops counting once it +/// is closed. +#[tokio::test] +async fn a_closed_unsaved_test_module_file_offers_nothing() { + let controller = CONTROLLER.replace("system.site", ""); + let (backend, dir) = site_backend(&[(CONTROLLER_PATH, controller.as_str())]).await; + let php = uri_for(&dir, CONTROLLER_PATH); + let position = offset_position(&controller, controller.find("''").unwrap() + 1); + let offered = || async { + response_labels( + completion_response(&backend, &php, position.line, position.character, None).await, + ) + }; + let unsaved = uri_for( + &dir, + "web/core/modules/system/tests/modules/config_test/config/install/config_test.unsaved.yml", + ); + crate::common::open_document(&backend, &unsaved, "yaml", "foo: bar\n").await; + let labels = offered().await; + assert!( + labels.contains(&"config_test.unsaved".to_string()), + "{labels:?}" + ); + + backend + .did_close(DidCloseTextDocumentParams { + text_document: TextDocumentIdentifier { uri: unsaved }, + }) + .await; + let labels = offered().await; + assert!( + !labels.contains(&"config_test.unsaved".to_string()), + "{labels:?}" + ); +} + /// A class that uses core's `ConfigFormBaseTrait` without being a form, /// as a list builder may, reads a name in its `config()` too. #[tokio::test]