fix #7102: missing "out of sync error"

which should be shown if one try to save pending editor changes to a file which was modified in background.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>

.gitignore

@@ -15,6 +15,7 @@ examples/*/src-gen
 examples/*/gen-webpack.config.js
 examples/*/.theia
 examples/*/.vscode
+examples/*/.test
 .browser_modules
 **/docs/api
 package-backup.json

examples/api-tests/src/saveable.spec.js

@@ -0,0 +1,126 @@
+/********************************************************************************
+ * Copyright (C) 2020 TypeFox and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+// @ts-check
+describe('Saveable', function () {
+
+    const { assert } = chai;
+
+    const { EditorManager } = require('@theia/editor/lib/browser/editor-manager');
+    const Uri = require('@theia/core/lib/common/uri');
+    const { Saveable } = require('@theia/core/lib/browser/saveable');
+    const { WorkspaceService } = require('@theia/workspace/lib/browser/workspace-service');
+    const { FileSystem, FileStat } = require('@theia/filesystem/lib/common/filesystem');
+    const { MonacoEditor } = require('@theia/monaco/lib/browser/monaco-editor');
+
+    /** @type {import('inversify').Container} */
+    const container = window['theia'].container;
+    const editorManager = container.get(EditorManager);
+    const workspaceService = container.get(WorkspaceService);
+    /** @type {import('@theia/filesystem/lib/common/filesystem').FileSystem} */
+    const fileSystem = container.get(FileSystem);
+
+    const rootUri = new Uri.default(workspaceService.tryGetRoots()[0].uri);
+    const fileUri = rootUri.resolve('.test/foo.txt');
+    /** @type {import('@theia/editor/lib/browser/editor-widget').EditorWidget} */
+    let widget;
+    /** @type {MonacoEditor} */
+    let editor;
+    /** @type {{ stat: FileStat, content: string }} */
+    let state;
+
+    before(async () => {
+        await Promise.all([
+            fileSystem.createFile(fileUri.toString(), { content: 'foo' }),
+            editorManager.closeAll()
+        ]);
+        widget = await editorManager.open(fileUri, { mode: 'reveal' });
+        editor = MonacoEditor.get(widget);
+    });
+
+    after(() => Promise.all([
+        fileSystem.delete(fileUri.parent.toString(), { moveToTrash: false }),
+        (widget.close(), widget = undefined),
+        editor = undefined,
+        state = undefined
+    ]));
+
+    const client = fileSystem.getClient();
+    const originalShouldOverwrite = client.shouldOverwrite;
+
+    afterEach(() => client.shouldOverwrite = originalShouldOverwrite);
+
+    it('normal save', async () => {
+        assert.isFalse(Saveable.isDirty(widget), 'should NOT be dirty before change');
+        editor.getControl().setValue('bar');
+        assert.isTrue(Saveable.isDirty(widget), 'should be dirty before save');
+        await Saveable.save(widget);
+        assert.isFalse(Saveable.isDirty(widget), 'should NOT be dirty after save');
+        assert.equal(editor.getControl().getValue(), 'bar', 'model should be updated');
+        state = await fileSystem.resolveContent(fileUri.toString());
+        assert.equal(state.content, 'bar', 'fs should be updated');
+    });
+
+    it('reject save', async () => {
+        let outOfSync = false;
+        client.shouldOverwrite = async () => {
+            outOfSync = true;
+            return false;
+        };
+        editor.getControl().setValue('FOO');
+        assert.isTrue(Saveable.isDirty(widget), 'should be dirty before save');
+        await fileSystem.touchFile(fileUri.toString());
+        await Saveable.save(widget);
+        assert.isTrue(outOfSync, 'file should be out of sync');
+        assert.isTrue(Saveable.isDirty(widget), 'should be dirty after rejected save');
+        assert.equal(editor.getControl().getValue(), 'FOO', 'model should be updated');
+        state = await fileSystem.resolveContent(fileUri.toString());
+        assert.equal(state.content, 'bar', 'fs should NOT be updated');
+    });
+
+    it('accept rejected save', async () => {
+        let outOfSync = false;
+        client.shouldOverwrite = async () => {
+            outOfSync = true;
+            return true;
+        };
+        assert.isTrue(Saveable.isDirty(widget), 'should be dirty before save');
+        await Saveable.save(widget);
+        assert.isTrue(outOfSync, 'file should be out of sync');
+        assert.isFalse(Saveable.isDirty(widget), 'should NOT be dirty after save');
+        assert.equal(editor.getControl().getValue(), 'FOO', 'model should be updated');
+        state = await fileSystem.resolveContent(fileUri.toString());
+        assert.equal(state.content, 'FOO', 'fs should be updated');
+    });
+
+    it('accept new save', async () => {
+        let outOfSync = false;
+        client.shouldOverwrite = async () => {
+            outOfSync = true;
+            return true;
+        };
+        editor.getControl().setValue('BAR');
+        assert.isTrue(Saveable.isDirty(widget), 'should be dirty before save');
+        await fileSystem.touchFile(fileUri.toString());
+        await Saveable.save(widget);
+        assert.isTrue(outOfSync, 'file should be out of sync');
+        assert.isFalse(Saveable.isDirty(widget), 'should NOT be dirty after save');
+        assert.equal(editor.getControl().getValue(), 'BAR', 'model should be updated');
+        state = await fileSystem.resolveContent(fileUri.toString());
+        assert.equal(state.content, 'BAR', 'fs should be updated');
+    });
+
+});

packages/core/src/common/messaging/proxy-factory.ts

@@ -29,6 +29,7 @@ export type JsonRpcServer<Client> = Disposable & {
      * to handle JSON-RPC messages from the remote server.
      */
     setClient(client: Client | undefined): void;
+    getClient?(): Client | undefined;
 };
 
 export interface JsonRpcConnectionEventEmitter {
@@ -220,6 +221,9 @@ export class JsonRpcProxyFactory<T extends object> implements ProxyHandler<T> {
                 this.target = client;
             };
         }
+        if (p === 'getClient') {
+            return () => this.target;
+        }
         if (p === 'onDidOpenConnection') {
             return this.onDidOpenConnectionEmitter.event;
         }

packages/core/src/common/resource.ts

@@ -24,19 +24,64 @@ import { MaybePromise } from './types';
 import { CancellationToken } from './cancellation';
 import { ApplicationError } from './application-error';
 
+export interface ResourceVersion {
+}
+
+export interface ResourceReadOptions {
+    encoding?: string
+}
+
+export interface ResourceSaveOptions {
+    encoding?: string,
+    overwriteEncoding?: string,
+    version?: ResourceVersion
+}
+
 export interface Resource extends Disposable {
     readonly uri: URI;
-    readContents(options?: { encoding?: string }): Promise<string>;
-    saveContents?(content: string, options?: { encoding?: string }): Promise<void>;
-    saveContentChanges?(changes: TextDocumentContentChangeEvent[], options?: { encoding?: string }): Promise<void>;
+    /**
+     * Latest read version of this resource.
+     *
+     * Optional if a resource does not support versioning, check with `in` operator`.
+     * Undefined if a resource did not read content yet.
+     */
+    readonly version?: ResourceVersion | undefined;
+    /**
+     * Reads latest content of this resouce.
+     *
+     * If a resource supports versioning it updates version to latest.
+     *
+     * @throws `ResourceError.NotFound` if a resource not found
+     */
+    readContents(options?: ResourceReadOptions): Promise<string>;
+    /**
+     * Rewrites the complete content for this resource.
+     * If a resource does not exist it will be created.
+     *
+     * If a resource supports versioning clients can pass some version
+     * to check against it, if it is not provided latest version is used.
+     *
+     * @throws `ResourceError.OutOfSync` if latest resource version is out of sync with the given
+     */
+    saveContents?(content: string, options?: ResourceSaveOptions): Promise<void>;
+    /**
+     * Applies incemental content changes to this resource.
+     *
+     * If a resource supports versioning clients can pass some version
+     * to check against it, if it is not provided latest version is used.
+     *
+     * @throws `ResourceError.NotFound` if a resource not found or was not read yet
+     * @throws `ResourceError.OutOfSync` if latest resource version is out of sync with the given
+     */
+    saveContentChanges?(changes: TextDocumentContentChangeEvent[], options?: ResourceSaveOptions): Promise<void>;
     readonly onDidChangeContents?: Event<void>;
     guessEncoding?(): Promise<string | undefined>
 }
 export namespace Resource {
     export interface SaveContext {
         content: string
         changes?: TextDocumentContentChangeEvent[]
-        options?: { encoding?: string, overwriteEncoding?: string }
+        options?: ResourceSaveOptions
     }
     export async function save(resource: Resource, context: SaveContext, token?: CancellationToken): Promise<void> {
         if (!resource.saveContents) {
@@ -56,11 +101,16 @@ export namespace Resource {
         }
         try {
             await resource.saveContentChanges(context.changes, context.options);
+            return true;
         } catch (e) {
-            console.error(e);
+            if (ResourceError.OutOfSync.is(e)) {
+                throw e;
+            }
+            if (!ResourceError.NotFound.is(e)) {
+                console.error(`Failed to apply incremenal changes to '${resource.uri.toString()}':`, e);
+            }
             return false;
         }
-        return true;
     }
     export function shouldSaveContent({ content, changes }: SaveContext): boolean {
         if (!changes) {
@@ -80,6 +130,7 @@ export namespace Resource {
 
 export namespace ResourceError {
     export const NotFound = ApplicationError.declare(-40000, (raw: ApplicationError.Literal<{ uri: URI }>) => raw);
+    export const OutOfSync = ApplicationError.declare(-40001, (raw: ApplicationError.Literal<{ uri: URI }>) => raw);
 }
 
 export const ResourceResolver = Symbol('ResourceResolver');

packages/filesystem/src/browser/file-resource.ts

@@ -16,20 +16,28 @@
 
 import { injectable, inject } from 'inversify';
 import { TextDocumentContentChangeEvent } from 'vscode-languageserver-protocol';
-import {
-    Resource, ResourceResolver, Emitter, Event, DisposableCollection, ResourceError
-} from '@theia/core';
+import { Resource, ResourceVersion, ResourceResolver, ResourceError, ResourceSaveOptions } from '@theia/core/lib/common/resource';
+import { DisposableCollection } from '@theia/core/lib/common/disposable';
+import { Emitter, Event } from '@theia/core/lib/common/event';
 import URI from '@theia/core/lib/common/uri';
 import { FileSystem, FileStat, FileSystemError } from '../common/filesystem';
 import { FileSystemWatcher, FileChangeEvent } from './filesystem-watcher';
 
+export interface FileResourceVersion extends ResourceVersion {
+    readonly stat: FileStat
+}
+
 export class FileResource implements Resource {
 
     protected readonly toDispose = new DisposableCollection();
     protected readonly onDidChangeContentsEmitter = new Emitter<void>();
     readonly onDidChangeContents: Event<void> = this.onDidChangeContentsEmitter.event;
 
-    protected stat: FileStat | undefined;
+    protected _version: FileResourceVersion | undefined;
+    get version(): FileResourceVersion | undefined {
+        return this._version;
+    }
+
     protected uriString: string;
 
     constructor(
@@ -46,7 +54,6 @@ export class FileResource implements Resource {
         if (stat && stat.isDirectory) {
             throw new Error('The given uri is a directory: ' + this.uriString);
         }
-        this.stat = stat;
 
         this.toDispose.push(this.fileSystemWatcher.onFilesChanged(event => {
             if (FileChangeEvent.isAffected(event, this.uri)) {
@@ -67,11 +74,11 @@ export class FileResource implements Resource {
     async readContents(options?: { encoding?: string }): Promise<string> {
         try {
             const { stat, content } = await this.fileSystem.resolveContent(this.uriString, options);
-            this.stat = stat;
+            this._version = { stat };
             return content;
         } catch (e) {
             if (FileSystemError.FileNotFound.is(e)) {
-                this.stat = undefined;
+                this._version = undefined;
                 throw ResourceError.NotFound({
                     ...e.toJson(),
                     data: {
@@ -83,37 +90,65 @@ export class FileResource implements Resource {
         }
     }
 
-    async saveContents(content: string, options?: { encoding?: string, overwriteEncoding?: string }): Promise<void> {
-        if (options && options.overwriteEncoding) {
-            this.stat = await this.doSaveContents(content, { encoding: options.overwriteEncoding });
-        } else {
-            this.stat = await this.doSaveContents(content, options);
+    async saveContents(content: string, options?: ResourceSaveOptions): Promise<void> {
+        try {
+            let resolvedOptions = options;
+            if (options && options.overwriteEncoding) {
+                resolvedOptions = {
+                    ...options,
+                    encoding: options.overwriteEncoding
+                };
+                delete resolvedOptions.overwriteEncoding;
+            }
+            const stat = await this.doSaveContents(content, resolvedOptions);
+            this._version = { stat };
+        } catch (e) {
+            if (FileSystemError.FileIsOutOfSync.is(e)) {
+                throw ResourceError.OutOfSync({ ...e.toJson(), data: { uri: this.uri } });
+            }
+            throw e;
         }
     }
-    protected async doSaveContents(content: string, options?: { encoding?: string }): Promise<FileStat> {
-        const stat = await this.getFileStat();
+    protected async doSaveContents(content: string, options?: { encoding?: string, version?: ResourceVersion }): Promise<FileStat> {
+        const version = this.version || this._version;
+        const stat = version && version.stat || await this.getFileStat();
         if (stat) {
-            return this.fileSystem.setContent(stat, content, options);
+            try {
+                return await this.fileSystem.setContent(stat, content, options);
+            } catch (e) {
+                if (!FileSystemError.FileNotFound.is(e)) {
+                    throw e;
+                }
+            }
         }
         return this.fileSystem.createFile(this.uriString, { content, ...options });
     }
 
-    async saveContentChanges(changes: TextDocumentContentChangeEvent[], options?: { encoding?: string, overwriteEncoding?: string }): Promise<void> {
-        if (!this.stat) {
-            throw new Error(this.uriString + ' has not been read yet');
+    async saveContentChanges(changes: TextDocumentContentChangeEvent[], options?: ResourceSaveOptions): Promise<void> {
+        const version = options && options.version && this._version;
+        if (!version) {
+            throw ResourceError.NotFound({ message: 'has not been read yet', data: { uri: this.uri } });
+        }
+        try {
+            const stat = await this.fileSystem.updateContent(version.stat, changes, options);
+            this._version = { stat };
+        } catch (e) {
+            if (FileSystemError.FileNotFound.is(e)) {
+                throw ResourceError.NotFound({ ...e.toJson(), data: { uri: this.uri } });
+            }
+            if (FileSystemError.FileIsOutOfSync.is(e)) {
+                throw ResourceError.OutOfSync({ ...e.toJson(), data: { uri: this.uri } });
+            }
+            throw e;
         }
-        this.stat = await this.fileSystem.updateContent(this.stat, changes, options);
     }
 
     async guessEncoding(): Promise<string | undefined> {
-        if (!this.stat) {
-            return undefined;
-        }
         return this.fileSystem.guessEncoding(this.uriString);
     }
 
     protected async sync(): Promise<void> {
-        if (await this.isInSync(this.stat)) {
+        if (await this.isInSync(this.version && this.version.stat)) {
             return;
         }
         this.onDidChangeContentsEmitter.fire(undefined);

packages/filesystem/src/node/node-filesystem.ts

@@ -117,7 +117,7 @@ export class FileSystemNode implements FileSystem {
         if (stat.isDirectory) {
             throw FileSystemError.FileIsDirectory(file.uri, 'Cannot set the content.');
         }
-        if (!this.checkInSync(file, stat)) {
+        if (!(await this.isInSync(file, stat))) {
             throw this.createOutOfSyncError(file, stat);
         }
         if (contentChanges.length === 0 && !(options && options.overwriteEncoding)) {