From fe471314ecc4b2d6c2b70630d23fcd4f44cdb745 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Olczak?= Date: Wed, 26 Feb 2020 18:03:14 +0100 Subject: [PATCH] File Reads for the Language Server (#559) File Reads for the Language Server --- doc/design/engine/engine-services.md | 52 ++++++++++++-- .../languageserver/ClientController.scala | 71 ++++++++++++------ .../enso/languageserver/LanguageServer.scala | 16 +++++ .../filemanager/FileManagerApi.scala | 21 +++++- .../filemanager/FileManagerProtocol.scala | 14 ++++ .../filemanager/FileSystem.scala | 43 +++++++---- .../filemanager/FileSystemApi.scala | 10 ++- .../filemanager/FileSystemFailure.scala | 5 ++ .../filemanager/FileSystemFailureMapper.scala | 8 +++ .../languageserver/WebSocketServerTest.scala | 72 ++++++++++++++++++- .../filemanager/FileSystemSpec.scala | 20 ++++++ 11 files changed, 288 insertions(+), 44 deletions(-) diff --git a/doc/design/engine/engine-services.md b/doc/design/engine/engine-services.md index aecf5b7358..13f97a075b 100644 --- a/doc/design/engine/engine-services.md +++ b/doc/design/engine/engine-services.md @@ -1111,9 +1111,9 @@ null ##### Errors -- **FileSystemError(errorCode=1000)** This error signals generic file system errors. -- **ContentRootNotFoundError(errorCode=1001)** The error informs that the requested content root cannot be found. -- **AccessDeniedError(errorCode=1002)** It signals that a user doesn't have access to a resource. +- [`FileSystemError`](#filesystemerror) to signal a generic, unrecoverable file-system error. +- [`ContentRootNotFoundError`](#contentrootnotfounderror) to signal that the requested content root cannot be found. +- [`AccessDeniedError`](#accessdeniederror) to signal that a user doesn't have access to a resource. #### `file/read` This requests that the file manager component reads the contents of a specified @@ -1142,7 +1142,11 @@ return the contents from the in-memory buffer rather than the file on disk. ``` ##### Errors -TBC + +- [`FileSystemError`](#filesystemerror) to signal a generic, unrecoverable file-system error. +- [`ContentRootNotFoundError`](#contentrootnotfounderror) to signal that the requested content root cannot be found. +- [`AccessDeniedError`](#accessdeniederror) to signal that a user doesn't have access to a resource. +- [`FileNotFound`](#filenotfound) informs that file cannot be found. #### `file/create` This request asks the file manager to create the specified file system object. @@ -1692,3 +1696,43 @@ TBC ### Errors - Language Server The language server component also has its own set of errors. This section is not a complete specification and will be updated as new errors are added. + +##### `FileSystemError` +This error signals generic file system errors. + +```typescript +"error" : { + "code" : 1000, + "message" : "File '/foo/bar' exists but is a directory" +} +``` + +##### `ContentRootNotFoundError` +The error informs that the requested content root cannot be found. + +```typescript +"error" : { + "code" : 1001, + "message" : "Content root not found" +} +``` + +##### `AccessDeniedError` +It signals that a user doesn't have access to a resource. + +```typescript +"error" : { + "code" : 1002, + "message" : "Access denied" +} +``` + +##### `FileNotFound` +It signals that requested file doesn't exist. + +```typescript +"error" : { + "code" : 1003, + "message" : "File not found" +} +``` diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/ClientController.scala b/engine/language-server/src/main/scala/org/enso/languageserver/ClientController.scala index 7ba6ea8d1f..5d6e98d8d4 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/ClientController.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/ClientController.scala @@ -7,15 +7,10 @@ import akka.pattern.ask import akka.util.Timeout import org.enso.languageserver.ClientApi._ import org.enso.languageserver.data.{CapabilityRegistration, Client} -import org.enso.languageserver.filemanager.FileManagerApi.{ - FileSystemError, - FileWrite, - FileWriteParams -} +import org.enso.languageserver.filemanager.FileManagerApi.{FileRead, _} import org.enso.languageserver.filemanager.FileManagerProtocol.FileWriteResult import org.enso.languageserver.filemanager.{ FileManagerProtocol, - FileSystemFailure, FileSystemFailureMapper } import org.enso.languageserver.jsonrpc.Errors.ServiceError @@ -69,6 +64,7 @@ object ClientApi { .registerRequest(AcquireCapability) .registerRequest(ReleaseCapability) .registerRequest(FileWrite) + .registerRequest(FileRead) .registerNotification(ForceReleaseCapability) .registerNotification(GrantCapability) @@ -123,22 +119,57 @@ class ClientController( server ! LanguageProtocol.ReleaseCapability(clientId, params.id) sender ! ResponseResult(ReleaseCapability, id, Unused) - case Request(FileWrite, id, params: FileWriteParams) => - (server ? FileManagerProtocol.FileWrite(params.path, params.content)) - .onComplete { - case Success(FileWriteResult(Right(()))) => - webActor ! ResponseResult(FileWrite, id, Unused) + case Request(FileWrite, id, params: FileWrite.Params) => + writeFile(webActor, id, params) - case Success(FileWriteResult(Left(failure))) => - webActor ! ResponseError( - Some(id), - FileSystemFailureMapper.mapFailure(failure) - ) + case Request(FileRead, id, params: FileRead.Params) => + readFile(webActor, id, params) - case Failure(th) => - log.error("An exception occurred during writing to a file", th) - webActor ! ResponseError(Some(id), ServiceError) - } + } + + private def readFile( + webActor: ActorRef, + id: Id, + params: FileRead.Params + ): Unit = { + (server ? FileManagerProtocol.FileRead(params.path)).onComplete { + case Success( + FileManagerProtocol.FileReadResult(Right(content: String)) + ) => + webActor ! ResponseResult(FileRead, id, FileRead.Result(content)) + + case Success(FileManagerProtocol.FileReadResult(Left(failure))) => + webActor ! ResponseError( + Some(id), + FileSystemFailureMapper.mapFailure(failure) + ) + + case Failure(th) => + log.error("An exception occurred during reading a file", th) + webActor ! ResponseError(Some(id), ServiceError) + } + } + + private def writeFile( + webActor: ActorRef, + id: Id, + params: FileWrite.Params + ): Unit = { + (server ? FileManagerProtocol.FileWrite(params.path, params.contents)) + .onComplete { + case Success(FileWriteResult(Right(()))) => + webActor ! ResponseResult(FileWrite, id, Unused) + + case Success(FileWriteResult(Left(failure))) => + webActor ! ResponseError( + Some(id), + FileSystemFailureMapper.mapFailure(failure) + ) + + case Failure(th) => + log.error("An exception occurred during writing to a file", th) + webActor ! ResponseError(Some(id), ServiceError) + } } } diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/LanguageServer.scala b/engine/language-server/src/main/scala/org/enso/languageserver/LanguageServer.scala index 58e49433ba..eac22b861d 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/LanguageServer.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/LanguageServer.scala @@ -6,6 +6,8 @@ import akka.actor.{Actor, ActorLogging, ActorRef, Stash} import cats.effect.IO import org.enso.languageserver.data._ import org.enso.languageserver.filemanager.FileManagerProtocol.{ + FileRead, + FileReadResult, FileWrite, FileWriteResult } @@ -134,5 +136,19 @@ class LanguageServer(config: Config, fs: FileSystemApi[IO]) } yield () sender ! FileWriteResult(result) + + case FileRead(path) => + val result = + for { + rootPath <- config.findContentRoot(path.rootId) + content <- fs.read(path.toFile(rootPath)).unsafeRunSync() + } yield content + + sender ! FileReadResult(result) } + /* Note [Usage of unsafe methods] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + It invokes side-effecting function, all exceptions are caught and + explicitly returned as left side of disjunction. + */ } diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileManagerApi.scala b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileManagerApi.scala index 17dd191158..aa8340dfe3 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileManagerApi.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileManagerApi.scala @@ -16,15 +16,30 @@ import org.enso.languageserver.jsonrpc.{ object FileManagerApi { case object FileWrite extends Method("file/write") { + + case class Params(path: Path, contents: String) + implicit val hasParams = new HasParams[this.type] { - type Params = FileWriteParams + type Params = FileWrite.Params } implicit val hasResult = new HasResult[this.type] { type Result = Unused.type } } - case class FileWriteParams(path: Path, content: String) + case object FileRead extends Method("file/read") { + + case class Params(path: Path) + + case class Result(contents: String) + + implicit val hasParams = new HasParams[this.type] { + type Params = FileRead.Params + } + implicit val hasResult = new HasResult[this.type] { + type Result = FileRead.Result + } + } case class FileSystemError(override val message: String) extends Error(1000, message) @@ -34,4 +49,6 @@ object FileManagerApi { case object AccessDeniedError extends Error(1002, "Access denied") + case object FileNotFoundError extends Error(1003, "File not found") + } 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 4dc89f8539..f70146dbba 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 @@ -17,4 +17,18 @@ object FileManagerProtocol { */ case class FileWriteResult(result: Either[FileSystemFailure, Unit]) + /** + * Requests the Language Server read a file. + * + * @param path a path to a file + */ + case class FileRead(path: Path) + + /** + * Returns a result of reading a file. + * + * @param result either file system failure or content of a file + */ + case class FileReadResult(result: Either[FileSystemFailure, String]) + } diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystem.scala b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystem.scala index fa6acb4fe0..1c91cfca01 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystem.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystem.scala @@ -1,6 +1,6 @@ package org.enso.languageserver.filemanager -import java.io.{File, IOException} +import java.io.{File, FileNotFoundException, IOException} import java.nio.file._ import cats.effect.Sync @@ -25,20 +25,33 @@ class FileSystem[F[_]: Sync] extends FileSystemApi[F] { file: File, content: String ): F[Either[FileSystemFailure, Unit]] = - Sync[F].delay { writeStringToFile(file, content) } + Sync[F].delay { + Either + .catchOnly[IOException] { + FileUtils.write(file, content, "UTF-8") + } + .leftMap(errorHandling) + } - private def writeStringToFile( - file: File, - content: String - ): Either[FileSystemFailure, Unit] = - Either - .catchOnly[IOException]( - FileUtils.write(file, content, "UTF-8") - ) - .leftMap { - case _: AccessDeniedException => AccessDenied - case ex => GenericFileSystemFailure(ex.getMessage) - } - .map(_ => ()) + /** + * Reads the contents of a textual file. + * + * @param file path to the file + * @return either [[FileSystemFailure]] or the content of a file as a String + */ + override def read(file: File): F[Either[FileSystemFailure, String]] = + Sync[F].delay { + Either + .catchOnly[IOException] { + FileUtils.readFileToString(file, "UTF-8") + } + .leftMap(errorHandling) + } + + private val errorHandling: IOException => FileSystemFailure = { + case _: FileNotFoundException => FileNotFound + case _: AccessDeniedException => AccessDenied + case ex => GenericFileSystemFailure(ex.getMessage) + } } 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 1d884aa41f..51b4650667 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 @@ -14,11 +14,19 @@ trait FileSystemApi[F[_]] { * * @param file path to the file * @param content a textual content of the file - * @return either FileSystemFailure or Unit + * @return either [[FileSystemFailure]] or Unit */ def write( file: File, content: String ): F[Either[FileSystemFailure, Unit]] + /** + * Reads the contents of a textual file. + * + * @param file path to the file + * @return either [[FileSystemFailure]] or the content of a file as a String + */ + def read(file: File): F[Either[FileSystemFailure, String]] + } diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystemFailure.scala b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystemFailure.scala index eeaecc2a01..1506ba14f0 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystemFailure.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystemFailure.scala @@ -15,6 +15,11 @@ case object ContentRootNotFound extends FileSystemFailure */ case object AccessDenied extends FileSystemFailure +/** + * Signals that the file cannot be found. + */ +case object FileNotFound extends FileSystemFailure + /** * Signals file system specific errors. * diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystemFailureMapper.scala b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystemFailureMapper.scala index 7384d5a2eb..abbaeb01be 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystemFailureMapper.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/filemanager/FileSystemFailureMapper.scala @@ -3,16 +3,24 @@ package org.enso.languageserver.filemanager import org.enso.languageserver.filemanager.FileManagerApi.{ AccessDeniedError, ContentRootNotFoundError, + FileNotFoundError, FileSystemError } import org.enso.languageserver.jsonrpc.Error object FileSystemFailureMapper { + /** + * Maps [[FileSystemFailure]] into JSON RPC error. + * + * @param fileSystemFailure file system specific failure + * @return JSON RPC error + */ def mapFailure(fileSystemFailure: FileSystemFailure): Error = fileSystemFailure match { case ContentRootNotFound => ContentRootNotFoundError case AccessDenied => AccessDeniedError + case FileNotFound => FileNotFoundError case GenericFileSystemFailure(reason) => FileSystemError(reason) } diff --git a/engine/language-server/src/test/scala/org/enso/languageserver/WebSocketServerTest.scala b/engine/language-server/src/test/scala/org/enso/languageserver/WebSocketServerTest.scala index 20536de508..c45e33d9d2 100644 --- a/engine/language-server/src/test/scala/org/enso/languageserver/WebSocketServerTest.scala +++ b/engine/language-server/src/test/scala/org/enso/languageserver/WebSocketServerTest.scala @@ -240,7 +240,7 @@ class WebSocketServerTest "rootId": $testContentRootId, "segments": [ "foo", "bar", "baz.txt" ] }, - "content": "123456789" + "contents": "123456789" } } """) @@ -267,7 +267,7 @@ class WebSocketServerTest "rootId": ${UUID.randomUUID()}, "segments": [ "foo", "bar", "baz.txt" ] }, - "content": "123456789" + "contents": "123456789" } } """) @@ -283,6 +283,74 @@ class WebSocketServerTest client.expectNoMessage() } + "read a file content" in { + val client = new WsTestClient(address) + + client.send(json""" + { "jsonrpc": "2.0", + "method": "file/write", + "id": 4, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo.txt" ] + }, + "contents": "123456789" + } + } + """) + client.expectJson(json""" + { "jsonrpc": "2.0", + "id": 4, + "result": null + } + """) + client.send(json""" + { "jsonrpc": "2.0", + "method": "file/read", + "id": 5, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo.txt" ] + } + } + } + """) + client.expectJson(json""" + { "jsonrpc": "2.0", + "id": 5, + "result": { "contents": "123456789" } + } + """) + } + + "return FileNotFoundError if a file doesn't exist" in { + val client = new WsTestClient(address) + + client.send(json""" + { "jsonrpc": "2.0", + "method": "file/read", + "id": 6, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "bar.txt" ] + } + } + } + """) + client.expectJson(json""" + { "jsonrpc": "2.0", + "id": 6, + "error" : { + "code" : 1003, + "message" : "File not found" + } + } + """) + } + } class WsTestClient(address: String) { diff --git a/engine/language-server/src/test/scala/org/enso/languageserver/filemanager/FileSystemSpec.scala b/engine/language-server/src/test/scala/org/enso/languageserver/filemanager/FileSystemSpec.scala index 03ebc778df..416608324c 100644 --- a/engine/language-server/src/test/scala/org/enso/languageserver/filemanager/FileSystemSpec.scala +++ b/engine/language-server/src/test/scala/org/enso/languageserver/filemanager/FileSystemSpec.scala @@ -48,6 +48,26 @@ class FileSystemSpec extends AnyFlatSpec with Matchers { readTxtFile(path) shouldBe content } + it should "return FileNotFound failure if the file doesn't exist" in new TestCtx { + //given + val path = Paths.get(testDirPath.toString, "foo.txt") + //when + val result = objectUnderTest.read(path.toFile).unsafeRunSync() + //then + result shouldBe Left(FileNotFound) + } + + it should "read a file content" in new TestCtx { + //given + val path = Paths.get(testDirPath.toString, "foo.txt") + val content = "123456789" + objectUnderTest.write(path.toFile, content).unsafeRunSync() + //when + val result = objectUnderTest.read(path.toFile).unsafeRunSync() + //then + result shouldBe Right(content) + } + def readTxtFile(path: Path): String = { val buffer = Source.fromFile(path.toFile) val content = buffer.getLines().mkString