From 4864e5b51d8edb63a6afe8605bece318344ab298 Mon Sep 17 00:00:00 2001 From: Dmitry Bushev Date: Mon, 10 Jul 2023 17:50:14 +0100 Subject: [PATCH] Add fileModifiedOnDisk notification (#7239) part of #7178 Changelog: - add: `text/fileModifiedOnDisk` notification - update: during the auto-save, check if the file is modified on disk and send the notification. I.e. auto-save does not overwrite the file if it was changed on disk (but the save command does) - update: IDE handles the file-modified-on-disk notification and reloads the module from disk # Important Notes Currently, the auto-save (and the check that the file is modified on disk) is triggered only after the file was edited. The proper check (using the file-watcher service) will be added in the next PR https://github.com/enso-org/enso/assets/357683/ff91f3e6-2f7a-4c01-a745-98cb140e1964 --- .../src/language_server/types.rs | 13 + app/gui/src/model/project/synchronized.rs | 25 ++ .../protocol-language-server.md | 25 ++ .../filemanager/FileManager.scala | 56 +++- .../filemanager/FileManagerProtocol.scala | 38 ++- .../filemanager/FileSystemApi.scala | 6 +- .../json/JsonConnectionController.scala | 6 + .../protocol/json/JsonRpc.scala | 1 + .../file/WriteBinaryFileHandler.scala | 2 +- .../file/WriteTextualFileHandler.scala | 2 +- .../org/enso/languageserver/text/Buffer.scala | 63 ++-- .../text/CollaborativeBuffer.scala | 143 +++++++-- .../text/FileWithMetadata.scala | 24 ++ .../enso/languageserver/text/TextApi.scala | 9 + .../languageserver/text/TextProtocol.scala | 19 +- .../websocket/json/TextOperationsTest.scala | 294 ++++++++++++++++++ .../scala/org/enso/text/ContentVersion.scala | 7 +- 17 files changed, 657 insertions(+), 76 deletions(-) create mode 100644 engine/language-server/src/main/scala/org/enso/languageserver/text/FileWithMetadata.scala diff --git a/app/gui/controller/engine-protocol/src/language_server/types.rs b/app/gui/controller/engine-protocol/src/language_server/types.rs index 92058e0252..43496366db 100644 --- a/app/gui/controller/engine-protocol/src/language_server/types.rs +++ b/app/gui/controller/engine-protocol/src/language_server/types.rs @@ -126,6 +126,11 @@ pub enum Notification { #[serde(rename = "text/didChange")] TextDidChange(FileEditList), + /// This is a notification sent from the server to the clients to inform them that a file + /// was modified on disk by external editor. + #[serde(rename = "text/fileModifiedOnDisk")] + TextFileModifiedOnDisk(TextFileModifiedOnDisk), + /// Sent from the server to the client to inform about new information for certain expressions /// becoming available. This notification is superseded by executionContext/expressionUpdates. #[serde(rename = "executionContext/expressionValuesComputed")] @@ -367,6 +372,14 @@ pub struct TextAutoSave { pub path: Path, } +/// The `text/fileModifiedOnDisk` notification parameters. +#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Serialize, Deserialize)] +#[allow(missing_docs)] +pub struct TextFileModifiedOnDisk { + pub path: Path, +} + // ====================== diff --git a/app/gui/src/model/project/synchronized.rs b/app/gui/src/model/project/synchronized.rs index 0eba1280ea..9d1d6e8ef2 100644 --- a/app/gui/src/model/project/synchronized.rs +++ b/app/gui/src/model/project/synchronized.rs @@ -21,6 +21,7 @@ use engine_protocol::language_server::ContentRoot; use engine_protocol::language_server::ExpressionUpdates; use engine_protocol::language_server::FileEditList; use engine_protocol::language_server::MethodPointer; +use engine_protocol::language_server::TextFileModifiedOnDisk; use engine_protocol::project_manager; use engine_protocol::project_manager::MissingComponentAction; use engine_protocol::project_manager::ProjectName; @@ -211,6 +212,20 @@ async fn update_modules_on_file_change( } +/// Reload the file from disk when `text/fileModifiedOnDisk` notification received. +#[profile(Detail)] +async fn reload_module_on_file_change( + modified: TextFileModifiedOnDisk, + module_registry: Rc>, +) -> FallibleResult { + let module_path = module::Path::from_file_path(modified.path)?; + if let Some(module) = module_registry.get(&module_path).await? { + module.reopen_externally_changed_file().await?; + } + Ok(()) +} + + // ============= // === Model === @@ -543,6 +558,16 @@ impl Project { }); } } + Event::Notification(Notification::TextFileModifiedOnDisk(modified)) => { + if let Some(module_registry) = weak_module_registry.upgrade() { + executor::global::spawn(async move { + let status = reload_module_on_file_change(modified, module_registry); + if let Err(err) = status.await { + error!("Error while reloading module on file change: {err}"); + } + }); + } + } Event::Notification(Notification::ExpressionUpdates(updates)) => { let ExpressionUpdates { context_id, updates } = updates; let execution_update = ExecutionUpdate::ExpressionUpdates(updates); diff --git a/docs/language-server/protocol-language-server.md b/docs/language-server/protocol-language-server.md index aae8a835df..360501a696 100644 --- a/docs/language-server/protocol-language-server.md +++ b/docs/language-server/protocol-language-server.md @@ -117,6 +117,7 @@ transport formats, please look [here](./protocol-architecture). - [`text/applyExpressionValue`](#textapplyexpressionvalue) - [`text/didChange`](#textdidchange) - [`text/autoSave`](#textautosave) + - [`text/fileModifiedOnDisk`](#textfilemodifiedondisk) - [Workspace Operations](#workspace-operations) - [`workspace/projectInfo`](#workspaceprojectinfo) - [Monitoring](#monitoring) @@ -3150,6 +3151,30 @@ This notification must _only_ be sent for files that the client has open. null; ``` +### `text/fileModifiedOnDisk` + +This is a notification sent from the server to the clients to inform them that +the file was modified on disk by an external editor. + +- **Type:** Notification +- **Direction:** Server -> Client +- **Connection:** Protocol +- **Visibility:** Public + +#### Parameters + +```typescript +{ + path: Path; +} +``` + +#### Errors + +```typescript +null; +``` + ## Workspace Operations The language server also has a set of operations useful for managing the client diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileManager.scala b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileManager.scala index 6d1a65f40a..ab27b898e6 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileManager.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileManager.scala @@ -52,21 +52,47 @@ class FileManager( case FileManagerProtocol.WriteFile(path, content) => val result = for { - file <- resolvePath(path) - _ <- fs.write(file, content) - } yield () + root <- findContentRoot(path.rootId) + file = path.toFile(root.file) + _ <- fs.write(file, content) + attrs <- fs.info(file) + } yield FileAttributes.fromFileSystemAttributes(root.file, path, attrs) exec .execTimed(config.timeout, result) .map(FileManagerProtocol.WriteFileResult) .pipeTo(sender()) - () + + case FileManagerProtocol.WriteFileIfNotModified( + path, + lastRecordedModifiedTime, + content + ) => + val isModified = + for { + file <- resolvePath(path) + attrs <- fs.info(file) + } yield attrs.lastModifiedTime.isAfter(lastRecordedModifiedTime) + val write = + for { + root <- findContentRoot(path.rootId) + file = path.toFile(root.file) + _ <- fs.write(file, content) + attrs <- fs.info(file) + } yield FileAttributes.fromFileSystemAttributes(root.file, path, attrs) + val result = ZIO.unlessZIO(isModified)(write) + exec + .execTimed(config.timeout, result) + .map(FileManagerProtocol.WriteFileIfNotModifiedResult) + .pipeTo(sender()) case FileManagerProtocol.WriteBinaryFile(path, contents) => val result = for { - file <- resolvePath(path) - _ <- fs.writeBinary(file, contents) - } yield () + root <- findContentRoot(path.rootId) + file = path.toFile(root.file) + _ <- fs.writeBinary(file, contents) + attrs <- fs.info(file) + } yield FileAttributes.fromFileSystemAttributes(root.file, path, attrs) exec .execTimed(config.timeout, result) .map(FileManagerProtocol.WriteFileResult) @@ -112,6 +138,22 @@ class FileManager( .map(FileManagerProtocol.ReadBinaryFileResult) .pipeTo(sender()) + case FileManagerProtocol.ReadFileWithAttributes(path) => + val result = + for { + root <- findContentRoot(path.rootId) + file = path.toFile(root.file) + content <- fs.read(file) + attrs <- fs.info(file) + } yield ( + FileManagerProtocol.TextualFileContent(file, content), + FileAttributes.fromFileSystemAttributes(root.file, path, attrs) + ) + exec + .execTimed(config.timeout, result) + .map(FileManagerProtocol.ReadFileWithAttributesResult) + .pipeTo(sender()) + case FileManagerProtocol.CreateFile(FileSystemObject.File(name, path)) => val result = for { diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileManagerProtocol.scala b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileManagerProtocol.scala index e462b461e5..fde58293d9 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileManagerProtocol.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileManagerProtocol.scala @@ -1,6 +1,7 @@ package org.enso.languageserver.filemanager import java.io.File +import java.time.OffsetDateTime object FileManagerProtocol { @@ -42,7 +43,28 @@ object FileManagerProtocol { * * @param result either file system failure or unit representing success */ - case class WriteFileResult(result: Either[FileSystemFailure, Unit]) + case class WriteFileResult(result: Either[FileSystemFailure, FileAttributes]) + + /** Requests the Language Server write textual content to a file if it was not + * modified after `lastModifiedTime`. + * + * @param path a path to a file + * @param lastModifiedTime a last recorded modification time of the file + * @param content a textual content + */ + case class WriteFileIfNotModified( + path: Path, + lastModifiedTime: OffsetDateTime, + content: String + ) + + /** Signals file manipulation status. + * + * @param result either file system failure or unit representing success + */ + case class WriteFileIfNotModifiedResult( + result: Either[FileSystemFailure, Option[FileAttributes]] + ) /** Requests the Language Server read a file. * @@ -50,6 +72,12 @@ object FileManagerProtocol { */ case class ReadFile(path: Path) + /** Requests the Language Server read a file with a file attributes. + * + * @param path a path to a file + */ + case class ReadFileWithAttributes(path: Path) + /** Requests the Language Server to read a binary content of a file. * * @param path a path to a file @@ -72,6 +100,14 @@ object FileManagerProtocol { result: Either[FileSystemFailure, BinaryFileContent] ) + /** Returns a result of reading a file with attributes. + * + * @param result either file system failure or content of a file + */ + case class ReadFileWithAttributesResult( + result: Either[FileSystemFailure, (TextualFileContent, FileAttributes)] + ) + /** Requests the Language Server create a file system object. * * @param `object` a file system object diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystemApi.scala b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystemApi.scala index 06fef96b5a..17b1bae2ab 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystemApi.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystemApi.scala @@ -255,7 +255,7 @@ object FileSystemApi { * * @param creationTime creation time * @param lastAccessTime last access time - * @param lastModifiedtime last modified time + * @param lastModifiedTime last modified time * @param kind either [[DirectoryEntryTruncated]] or [[FileEntry]] or [[OtherEntry]] * @param byteSize size of entry in bytes */ @@ -273,7 +273,7 @@ object FileSystemApi { * * @param creationTime creation time * @param lastAccessTime last access time - * @param lastModifiedtime last modified time + * @param lastModifiedTime last modified time * @param kind a type of the file system object * @param byteSize size of an entry in bytes * @return file attributes @@ -296,7 +296,7 @@ object FileSystemApi { /** Creates [[Attributes]] from file system attributes * * @param path to the file system object - * @param attributes of a file system object + * @param basic attributes of a file system object * @return file attributes */ def fromBasicAttributes( diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/protocol/json/JsonConnectionController.scala b/engine/language-server/src/main/scala/org/enso/languageserver/protocol/json/JsonConnectionController.scala index b66c2b1023..2351e38c3a 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/protocol/json/JsonConnectionController.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/protocol/json/JsonConnectionController.scala @@ -304,6 +304,12 @@ class JsonConnectionController( case TextProtocol.FileAutoSaved(path) => webActor ! Notification(FileAutoSaved, FileAutoSaved.Params(path)) + case TextProtocol.FileModifiedOnDisk(path) => + webActor ! Notification( + FileModifiedOnDisk, + FileModifiedOnDisk.Params(path) + ) + case TextProtocol.FileEvent(path, event) => webActor ! Notification(EventFile, EventFile.Params(path, event)) diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/protocol/json/JsonRpc.scala b/engine/language-server/src/main/scala/org/enso/languageserver/protocol/json/JsonRpc.scala index 450766c8c7..feadedae3d 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/protocol/json/JsonRpc.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/protocol/json/JsonRpc.scala @@ -106,6 +106,7 @@ object JsonRpc { .registerNotification(GrantCapability) .registerNotification(TextDidChange) .registerNotification(FileAutoSaved) + .registerNotification(FileModifiedOnDisk) .registerNotification(EventFile) .registerNotification(ContentRootAdded) .registerNotification(ContentRootRemoved) diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/requesthandler/file/WriteBinaryFileHandler.scala b/engine/language-server/src/main/scala/org/enso/languageserver/requesthandler/file/WriteBinaryFileHandler.scala index a52cf02662..1ad66158a6 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/requesthandler/file/WriteBinaryFileHandler.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/requesthandler/file/WriteBinaryFileHandler.scala @@ -84,7 +84,7 @@ class WriteBinaryFileHandler( cancellable.cancel() context.stop(self) - case FileManagerProtocol.WriteFileResult(Right(())) => + case FileManagerProtocol.WriteFileResult(Right(_)) => val packet = SuccessReplyFactory.createPacket(requestId) replyTo ! packet cancellable.cancel() diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/requesthandler/file/WriteTextualFileHandler.scala b/engine/language-server/src/main/scala/org/enso/languageserver/requesthandler/file/WriteTextualFileHandler.scala index 772a83dd39..db5e3ae756 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/requesthandler/file/WriteTextualFileHandler.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/requesthandler/file/WriteTextualFileHandler.scala @@ -61,7 +61,7 @@ class WriteTextualFileHandler( cancellable.cancel() context.stop(self) - case FileManagerProtocol.WriteFileResult(Right(())) => + case FileManagerProtocol.WriteFileResult(Right(_)) => replyTo ! ResponseResult(WriteFile, id, Unused) cancellable.cancel() context.stop(self) diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/text/Buffer.scala b/engine/language-server/src/main/scala/org/enso/languageserver/text/Buffer.scala index 79be7adccb..e50eeb52a3 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/text/Buffer.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/text/Buffer.scala @@ -6,22 +6,23 @@ import org.enso.text.buffer.Rope import org.enso.text.editing.model.Position import org.enso.text.editing.model.Range +import java.time.OffsetDateTime + /** A buffer state representation. * - * @param file the file linked to the buffer. + * @param fileWithMetadata the file linked to the buffer. * @param contents the contents of the buffer. * @param inMemory determines if the buffer is in-memory * @param version the current version of the buffer contents. */ case class Buffer( - file: File, + fileWithMetadata: FileWithMetadata, contents: Rope, inMemory: Boolean, version: ContentVersion ) { - /** Returns a range covering the whole buffer. - */ + /** Returns a range covering the whole buffer. */ lazy val fullRange: Range = { val lines = contents.lines.length Range( @@ -29,30 +30,37 @@ case class Buffer( Position(lines - 1, contents.lines.drop(lines - 1).characters.length) ) } + + /** Create a buffer with new contents. + * + * @param newContents the new contents of the buffer + * @param inMemory determines if the buffer is in-memory + * @return the buffer with new provided contents + */ + def withContents( + newContents: Rope, + inMemory: Boolean = inMemory + )(implicit versionCalculator: ContentBasedVersioning): Buffer = + copy( + contents = newContents, + version = versionCalculator.evalVersion(newContents.toString), + inMemory = inMemory + ) + + /** Create a buffer with new last modified time. + * + * @param lastModifiedTime the last modified time of the underlying file + * @return the buffer with new last modified time + */ + def withLastModifiedTime(lastModifiedTime: OffsetDateTime): Buffer = + copy( + fileWithMetadata = + fileWithMetadata.copy(lastModifiedTime = Some(lastModifiedTime)) + ) } object Buffer { - /** Creates a new buffer with a freshly generated version. - * - * @param file the file linked to the buffer. - * @param contents the contents of this buffer. - * @param inMemory determines if the buffer is in-memory - * @param versionCalculator a digest calculator for content based versioning. - * @return a new buffer instance. - */ - def apply( - file: File, - contents: Rope, - inMemory: Boolean - )(implicit versionCalculator: ContentBasedVersioning): Buffer = - Buffer( - file, - contents, - inMemory, - versionCalculator.evalVersion(contents.toString) - ) - /** Creates a new buffer with a freshly generated version. * * @param file the file linked to the buffer. @@ -66,5 +74,10 @@ object Buffer { contents: String, inMemory: Boolean )(implicit versionCalculator: ContentBasedVersioning): Buffer = - Buffer(file, Rope(contents), inMemory) + Buffer( + FileWithMetadata(file), + Rope(contents), + inMemory, + versionCalculator.evalVersion(contents) + ) } diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/text/CollaborativeBuffer.scala b/engine/language-server/src/main/scala/org/enso/languageserver/text/CollaborativeBuffer.scala index 24f3056e6e..2e70b07eba 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/text/CollaborativeBuffer.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/text/CollaborativeBuffer.scala @@ -12,6 +12,7 @@ import org.enso.languageserver.event.{ JsonSessionTerminated } import org.enso.languageserver.filemanager.{ + FileAttributes, FileEventKind, FileManagerProtocol, FileNotFound, @@ -91,8 +92,34 @@ class CollaborativeBuffer( timeoutCancellable: Cancellable, inMemoryBuffer: Boolean ): Receive = { + case FileManagerProtocol.ReadFileWithAttributesResult( + Right((content, attrs)) + ) => + handleFileContent( + rpcSession, + replyTo, + content, + Some(attrs), + inMemoryBuffer, + Map.empty + ) + unstashAll() + timeoutCancellable.cancel() + + case FileManagerProtocol.ReadFileWithAttributesResult(Left(failure)) => + replyTo ! OpenFileResponse(Left(failure)) + timeoutCancellable.cancel() + stop(Map.empty) + case FileManagerProtocol.ReadTextualFileResult(Right(content)) => - handleFileContent(rpcSession, replyTo, content, inMemoryBuffer, Map.empty) + handleFileContent( + rpcSession, + replyTo, + content, + None, + inMemoryBuffer, + Map.empty + ) unstashAll() timeoutCancellable.cancel() @@ -249,7 +276,7 @@ class CollaborativeBuffer( if (buffer.inMemory) { fileManager ! FileManagerProtocol.OpenBuffer(path) } else { - fileManager ! FileManagerProtocol.ReadFile(path) + fileManager ! FileManagerProtocol.ReadFileWithAttributes(path) } val timeoutCancellable = context.system.scheduler .scheduleOnce(timingsConfig.requestTimeout, self, IOTimeout) @@ -276,9 +303,12 @@ class CollaborativeBuffer( clients: Map[ClientId, JsonSession], inMemoryBuffer: Boolean ): Receive = { - case FileManagerProtocol.ReadTextualFileResult(Right(file)) => + case FileManagerProtocol.ReadFileWithAttributesResult( + Right((file, attrs)) + ) => timeoutCancellable.cancel() val buffer = Buffer(file.path, file.content, inMemoryBuffer) + .withLastModifiedTime(attrs.lastModifiedTime) // Notify *all* clients about the new buffer // This also ensures that the client that requested the restore operation @@ -305,7 +335,7 @@ class CollaborativeBuffer( ) ) - case FileManagerProtocol.ReadTextualFileResult(Left(FileNotFound)) => + case FileManagerProtocol.ReadFileWithAttributesResult(Left(FileNotFound)) => clients.values.foreach { _.rpcController ! TextProtocol.FileEvent(path, FileEventKind.Removed) } @@ -313,7 +343,7 @@ class CollaborativeBuffer( timeoutCancellable.cancel() stop(Map.empty) - case FileManagerProtocol.ReadTextualFileResult(Left(err)) => + case FileManagerProtocol.ReadFileWithAttributesResult(Left(err)) => replyTo ! ReloadBufferFailed(path, "io failure: " + err.toString) timeoutCancellable.cancel() context.become( @@ -384,6 +414,20 @@ class CollaborativeBuffer( ) } + case FileManagerProtocol.WriteFileIfNotModifiedResult(Left(failure)) => + replyTo.foreach(_ ! SaveFailed(failure)) + unstashAll() + timeoutCancellable.cancel() + onClose match { + case Some(clientId) => + replyTo.foreach(_ ! FileClosed) + removeClient(buffer, clients, lockHolder, clientId, autoSave) + case None => + context.become( + collaborativeEditing(buffer, clients, lockHolder, autoSave) + ) + } + case Status.Failure(failure) => logger.error( s"Waiting on save operation to complete failed with: ${failure.getMessage}.", @@ -400,7 +444,7 @@ class CollaborativeBuffer( ) } - case FileManagerProtocol.WriteFileResult(Right(())) => + case FileManagerProtocol.WriteFileResult(Right(attrs)) => replyTo match { case Some(replyTo) => replyTo ! FileSaved case None => @@ -413,10 +457,47 @@ class CollaborativeBuffer( onClose match { case Some(clientId) => replyTo.foreach(_ ! FileClosed) - removeClient(buffer, clients, lockHolder, clientId, autoSave) + removeClient( + buffer.withLastModifiedTime(attrs.lastModifiedTime), + clients, + lockHolder, + clientId, + autoSave + ) case None => context.become( - collaborativeEditing(buffer, clients, lockHolder, autoSave) + collaborativeEditing( + buffer.withLastModifiedTime(attrs.lastModifiedTime), + clients, + lockHolder, + autoSave + ) + ) + } + + case FileManagerProtocol.WriteFileIfNotModifiedResult(Right(result)) => + replyTo match { + case Some(replyTo) => replyTo ! FileSaved + case None => + val message = result.fold[Any](FileModifiedOnDisk(bufferPath))(_ => + FileAutoSaved(bufferPath) + ) + clients.values.foreach { + _.rpcController ! message + } + } + unstashAll() + timeoutCancellable.cancel() + onClose match { + case Some(clientId) => + replyTo.foreach(_ ! FileClosed) + removeClient(buffer, clients, lockHolder, clientId, autoSave) + case None => + val newBuffer = result.fold(buffer)(attrs => + buffer.withLastModifiedTime(attrs.lastModifiedTime) + ) + context.become( + collaborativeEditing(newBuffer, clients, lockHolder, autoSave) ) } @@ -438,10 +519,17 @@ class CollaborativeBuffer( val hasLock = lockHolder.exists(_.clientId == clientId) if (hasLock) { if (clientVersion == buffer.version) { - fileManager ! FileManagerProtocol.WriteFile( - bufferPath, - buffer.contents.toString - ) + val saveMessage = + if (isAutoSave && buffer.fileWithMetadata.lastModifiedTime.nonEmpty) { + FileManagerProtocol.WriteFileIfNotModified( + bufferPath, + buffer.fileWithMetadata.lastModifiedTime.get, + buffer.contents.toString + ) + } else { + FileManagerProtocol.WriteFile(bufferPath, buffer.contents.toString) + } + fileManager ! saveMessage currentAutoSaves.get(clientId).foreach(_._2.cancel()) val timeoutCancellable = context.system.scheduler @@ -518,7 +606,7 @@ class CollaborativeBuffer( subscribers foreach { _.rpcController ! TextDidChange(List(change)) } runtimeConnector ! Api.Request( Api.SetExpressionValueNotification( - buffer.file, + buffer.fileWithMetadata.file, change.edits, expressionId, expressionValue @@ -551,7 +639,11 @@ class CollaborativeBuffer( val subscribers = clients.filterNot(_._1 == clientId).values subscribers foreach { _.rpcController ! TextDidChange(List(change)) } runtimeConnector ! Api.Request( - Api.EditFileNotification(buffer.file, change.edits, execute) + Api.EditFileNotification( + buffer.fileWithMetadata.file, + change.edits, + execute + ) ) val newAutoSave: Map[ClientId, (ContentVersion, Cancellable)] = upsertAutoSaveTimer( @@ -617,14 +709,7 @@ class CollaborativeBuffer( EditorOps .applyEdits(buffer.contents, edits) .leftMap(toEditFailure) - .map(rope => - Buffer( - buffer.file, - rope, - buffer.inMemory, - versionCalculator.evalVersion(rope.toString) - ) - ) + .map(rope => buffer.withContents(rope)) } private val toEditFailure: TextEditValidationFailure => ApplyEditFailure = { @@ -644,7 +729,7 @@ class CollaborativeBuffer( rpcSession: JsonSession, path: Path ): Unit = { - fileManager ! FileManagerProtocol.ReadFile(path) + fileManager ! FileManagerProtocol.ReadFileWithAttributes(path) val timeoutCancellable = context.system.scheduler .scheduleOnce(timingsConfig.requestTimeout, self, IOTimeout) context.become( @@ -678,11 +763,15 @@ class CollaborativeBuffer( rpcSession: JsonSession, originalSender: ActorRef, file: FileManagerProtocol.TextualFileContent, + attributes: Option[FileAttributes], inMemoryBuffer: Boolean, autoSave: Map[ClientId, (ContentVersion, Cancellable)] ): Unit = { - val buffer = Buffer(file.path, file.content, inMemoryBuffer) - val cap = CapabilityRegistration(CanEdit(bufferPath)) + val initialBuffer = Buffer(file.path, file.content, inMemoryBuffer) + val buffer = attributes.fold(initialBuffer)(attrs => + initialBuffer.withLastModifiedTime(attrs.lastModifiedTime) + ) + val cap = CapabilityRegistration(CanEdit(bufferPath)) originalSender ! OpenFileResponse( Right(OpenFileResult(buffer, Some(cap))) ) @@ -737,7 +826,9 @@ class CollaborativeBuffer( val newClientMap = clients - clientId if (newClientMap.isEmpty) { - runtimeConnector ! Api.Request(Api.CloseFileNotification(buffer.file)) + runtimeConnector ! Api.Request( + Api.CloseFileNotification(buffer.fileWithMetadata.file) + ) stop(autoSave) } else { context.become( diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/text/FileWithMetadata.scala b/engine/language-server/src/main/scala/org/enso/languageserver/text/FileWithMetadata.scala new file mode 100644 index 0000000000..8ca9c909db --- /dev/null +++ b/engine/language-server/src/main/scala/org/enso/languageserver/text/FileWithMetadata.scala @@ -0,0 +1,24 @@ +package org.enso.languageserver.text + +import java.io.File +import java.time.OffsetDateTime + +/** A file with extra info. + * + * @param file the underlying file + * @param lastModifiedTime the last known modified time on disk + */ +case class FileWithMetadata( + file: File, + lastModifiedTime: Option[OffsetDateTime] +) +object FileWithMetadata { + + /** Create a file with metadata. + * + * @param file the underlying file + * @return a new instance of [[FileWithMetadata]] + */ + def apply(file: File): FileWithMetadata = + FileWithMetadata(file, None) +} diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/text/TextApi.scala b/engine/language-server/src/main/scala/org/enso/languageserver/text/TextApi.scala index 8af5ebdaad..fb12827795 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/text/TextApi.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/text/TextApi.scala @@ -107,6 +107,15 @@ object TextApi { } } + case object FileModifiedOnDisk extends Method("text/fileModifiedOnDisk") { + case class Params(path: Path) + implicit + val hasParams: HasParams.Aux[this.type, FileModifiedOnDisk.Params] = + new HasParams[this.type] { + type Params = FileModifiedOnDisk.Params + } + } + case object SaveFile extends Method("text/save") { case class Params(path: Path, currentVersion: Version) implicit val hasParams: HasParams.Aux[this.type, SaveFile.Params] = diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/text/TextProtocol.scala b/engine/language-server/src/main/scala/org/enso/languageserver/text/TextProtocol.scala index 668f3927f6..4681ca22a6 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/text/TextProtocol.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/text/TextProtocol.scala @@ -136,6 +136,13 @@ object TextProtocol { */ case class FileAutoSaved(path: Path) + /** A notification sent by the Language Server, notifying a client that the + * file was modified on disk. + * + * @param path path to the file + */ + case class FileModifiedOnDisk(path: Path) + /** A notification sent by the Language Server, notifying a client about * a file event after reloading the buffer to sync with file system * @@ -155,16 +162,13 @@ object TextProtocol { currentVersion: TextApi.Version ) - /** Signals the result of saving a file. - */ + /** Signals the result of saving a file. */ sealed trait SaveFileResult - /** Signals that saving a file was executed successfully. - */ + /** Signals that saving a file was executed successfully. */ case object FileSaved extends SaveFileResult - /** Signals that the client doesn't hold write lock to the buffer. - */ + /** Signals that the client doesn't hold write lock to the buffer. */ case object SaveDenied extends SaveFileResult /** Signals that version provided by a client doesn't match to the version @@ -184,4 +188,7 @@ object TextProtocol { */ case class SaveFailed(fsFailure: FileSystemFailure) extends SaveFileResult + /** Signals that the file is modified on disk. */ + case object SaveFailedFileModifiedOnDisk extends SaveFileResult + } diff --git a/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/TextOperationsTest.scala b/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/TextOperationsTest.scala index 50c35949ba..fb5f2e4da0 100644 --- a/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/TextOperationsTest.scala +++ b/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/TextOperationsTest.scala @@ -6,6 +6,10 @@ import org.enso.languageserver.event.{BufferClosed, JsonSessionTerminated} import org.enso.languageserver.filemanager.Path import org.enso.languageserver.session.JsonSession import org.enso.testkit.FlakySpec + +import java.nio.charset.StandardCharsets +import java.nio.file.Files + import scala.concurrent.duration._ class TextOperationsTest extends BaseServerTest with FlakySpec { @@ -2354,6 +2358,162 @@ class TextOperationsTest extends BaseServerTest with FlakySpec { """) } + "persist changes when the file was changed to disk" in { + val client = getInitialisedWsClient() + client.send(json""" + { "jsonrpc": "2.0", + "method": "file/write", + "id": 0, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo1.txt" ] + }, + "contents": "123456789" + } + } + """) + client.expectJson(json""" + { "jsonrpc": "2.0", + "id": 0, + "result": null + } + """) + client.send(json""" + { "jsonrpc": "2.0", + "method": "text/openFile", + "id": 1, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo1.txt" ] + } + } + } + """) + client.expectJson(json""" + { + "jsonrpc" : "2.0", + "id" : 1, + "result" : { + "writeCapability" : { + "method" : "text/canEdit", + "registerOptions" : { + "path" : { + "rootId" : $testContentRootId, + "segments" : [ + "foo1.txt" + ] + } + } + }, + "content" : "123456789", + "currentVersion" : "5795c3d628fd638c9835a4c79a55809f265068c88729a1a3fcdf8522" + } + } + """) + + client.send(json""" + { "jsonrpc": "2.0", + "method": "text/applyEdit", + "id": 2, + "params": { + "edit": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo1.txt" ] + }, + "oldVersion": "5795c3d628fd638c9835a4c79a55809f265068c88729a1a3fcdf8522", + "newVersion": "ebe55342f9c8b86857402797dd723fb4a2174e0b56d6ace0a6929ec3", + "edits": [ + { + "range": { + "start": { "line": 0, "character": 0 }, + "end": { "line": 0, "character": 0 } + }, + "text": "bar" + }, + { + "range": { + "start": { "line": 0, "character": 12 }, + "end": { "line": 0, "character": 12 } + }, + "text": "foo" + } + ] + } + } + } + """) + client.expectJson(json""" + { "jsonrpc": "2.0", + "id": 2, + "result": null + } + """) + + // Change file on disk + val fooTxt = testContentRoot.file.toPath.resolve("foo1.txt") + Files.write(fooTxt, "abcdef".getBytes(StandardCharsets.UTF_8)) + + client.send(json""" + { "jsonrpc": "2.0", + "method": "file/read", + "id": 3, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo1.txt" ] + } + } + } + """) + client.expectJson(json""" + { "jsonrpc": "2.0", + "id": 3, + "result": { "contents": "abcdef" } + } + """) + + client.send(json""" + { "jsonrpc": "2.0", + "method": "text/save", + "id": 3, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo1.txt" ] + }, + "currentVersion": "ebe55342f9c8b86857402797dd723fb4a2174e0b56d6ace0a6929ec3" + } + } + """) + client.expectJson(json""" + { "jsonrpc": "2.0", + "id": 3, + "result": null + } + """) + client.send(json""" + { "jsonrpc": "2.0", + "method": "file/read", + "id": 4, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo1.txt" ] + } + } + } + """) + client.expectJson(json""" + { "jsonrpc": "2.0", + "id": 4, + "result": { "contents": "bar123456789foo" } + } + """) + } + } "auto-save" must { @@ -2745,6 +2905,140 @@ class TextOperationsTest extends BaseServerTest with FlakySpec { } + "not persist changes when the file is changed on disk" in { + this.timingsConfig.withAutoSave(2.seconds) + + val client = getInitialisedWsClient() + client.send(json""" + { "jsonrpc": "2.0", + "method": "file/write", + "id": 0, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo.txt" ] + }, + "contents": "123456789" + } + } + """) + client.expectJson(json""" + { "jsonrpc": "2.0", + "id": 0, + "result": null + } + """) + client.send(json""" + { "jsonrpc": "2.0", + "method": "text/openFile", + "id": 1, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo.txt" ] + } + } + } + """) + client.expectJson(json""" + { + "jsonrpc" : "2.0", + "id" : 1, + "result" : { + "writeCapability" : { + "method" : "text/canEdit", + "registerOptions" : { + "path" : { + "rootId" : $testContentRootId, + "segments" : [ + "foo.txt" + ] + } + } + }, + "content" : "123456789", + "currentVersion" : "5795c3d628fd638c9835a4c79a55809f265068c88729a1a3fcdf8522" + } + } + """) + + // Change file on disk + Thread.sleep(1.seconds.toMillis) + val fooTxt = testContentRoot.file.toPath.resolve("foo.txt") + Files.write(fooTxt, "abcdef".getBytes(StandardCharsets.UTF_8)) + + client.send(json""" + { "jsonrpc": "2.0", + "method": "text/applyEdit", + "id": 2, + "params": { + "edit": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo.txt" ] + }, + "oldVersion": "5795c3d628fd638c9835a4c79a55809f265068c88729a1a3fcdf8522", + "newVersion": "ebe55342f9c8b86857402797dd723fb4a2174e0b56d6ace0a6929ec3", + "edits": [ + { + "range": { + "start": { "line": 0, "character": 0 }, + "end": { "line": 0, "character": 0 } + }, + "text": "bar" + }, + { + "range": { + "start": { "line": 0, "character": 12 }, + "end": { "line": 0, "character": 12 } + }, + "text": "foo" + } + ] + } + } + } + """) + client.expectJson(json""" + { "jsonrpc": "2.0", + "id": 2, + "result": null + } + """) + + Thread.sleep(8.seconds.toMillis) + // No explicit file save + client.expectJson(json""" + { "jsonrpc": "2.0", + "method":"text/fileModifiedOnDisk", + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo.txt" ] + } + } + } + """) + client.send(json""" + { "jsonrpc": "2.0", + "method": "file/read", + "id": 3, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo.txt" ] + } + } + } + """) + client.expectJson(json""" + { "jsonrpc": "2.0", + "id": 3, + "result": { "contents": "abcdef" } + } + """) + } + } } diff --git a/lib/scala/text-buffer/src/main/scala/org/enso/text/ContentVersion.scala b/lib/scala/text-buffer/src/main/scala/org/enso/text/ContentVersion.scala index 17f033de3b..c360567c41 100644 --- a/lib/scala/text-buffer/src/main/scala/org/enso/text/ContentVersion.scala +++ b/lib/scala/text-buffer/src/main/scala/org/enso/text/ContentVersion.scala @@ -3,12 +3,7 @@ package org.enso.text import org.bouncycastle.util.encoders.Hex /** Version of the text contents. */ -case class ContentVersion(toHexString: String) { - - /** Calculate digest. */ - def toDigest: Array[Byte] = - Hex.decode(toHexString) -} +case class ContentVersion(toHexString: String) object ContentVersion {