feat: Use relative paths for local file assets (#1027)

package-lock.json

@@ -11916,6 +11916,12 @@
                 }
             }
         },
+        "mock-fs": {
+            "version": "4.13.0",
+            "resolved": "https://registry.npmjs.org/mock-fs/-/mock-fs-4.13.0.tgz",
+            "integrity": "sha512-DD0vOdofJdoaRNtnWcrXe6RQbpHkPPmtqGq14uRX0F8ZKJ5nv89CVTYl/BZdppDxBDaV0hl75htg3abpEWlPZA==",
+            "dev": true
+        },
         "moo": {
             "version": "0.4.3",
             "resolved": "https://registry.npmjs.org/moo/-/moo-0.4.3.tgz",
@@ -12097,7 +12103,7 @@
             "dependencies": {
                 "semver": {
                     "version": "5.3.0",
-                    "resolved": "http://registry.npmjs.org/semver/-/semver-5.3.0.tgz",
+                    "resolved": "https://registry.npmjs.org/semver/-/semver-5.3.0.tgz",
                     "integrity": "sha1-myzl094C0XxgEq0yaqa00M9U+U8=",
                     "dev": true
                 }
@@ -12234,7 +12240,7 @@
                 },
                 "chalk": {
                     "version": "1.1.3",
-                    "resolved": "http://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz",
+                    "resolved": "https://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz",
                     "integrity": "sha1-qBFcVeSnAv5NFQq9OHKCKn4J/Jg=",
                     "dev": true,
                     "requires": {
@@ -17293,7 +17299,7 @@
             "dependencies": {
                 "source-map": {
                     "version": "0.4.4",
-                    "resolved": "http://registry.npmjs.org/source-map/-/source-map-0.4.4.tgz",
+                    "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.4.4.tgz",
                     "integrity": "sha1-66T12pwNyZneaAMti092FzZSA2s=",
                     "dev": true,
                     "requires": {

package.json

@@ -119,6 +119,7 @@
         "foreman": "^3.0.1",
         "jest-enzyme": "^7.0.1",
         "jquery": "^3.3.1",
+        "mock-fs": "^4.13.0",
         "node-sass": "^4.14.1",
         "popper.js": "^1.14.6",
         "redux-immutable-state-invariant": "^2.1.0",

src/common/mockFactory.ts

@@ -398,6 +398,7 @@ export default class MockFactory {
     public static createLocalFileSystemOptions(): ILocalFileSystemProxyOptions {
         return {
             folderPath: "C:\\projects\\vott\\project",
+            relativePath: false,
         };
     }
 

src/electron/providers/storage/localFileSystem.test.ts

@@ -1,7 +1,8 @@
 import fs from "fs";
-import path from "path";
+import path, { relative } from "path";
 import shortid from "shortid";
 import LocalFileSystem from "./localFileSystem";
+import mockFs from "mock-fs";
 
 jest.mock("electron", () => ({
     dialog: {
@@ -9,14 +10,36 @@ jest.mock("electron", () => ({
     },
 }));
 import { dialog } from "electron";
+import { AssetService } from "../../../services/assetService";
 
 describe("LocalFileSystem Storage Provider", () => {
     let localFileSystem: LocalFileSystem = null;
+    const sourcePath = path.join("path", "to", "my", "source");
 
     beforeEach(() => {
         localFileSystem = new LocalFileSystem(null);
     });
 
+    beforeAll(() => {
+        mockFs({
+            path: {
+                to: {
+                    my: {
+                        source: {
+                            "file1.jpg": "contents",
+                            "file2.jpg": "contents",
+                            "file3.jpg": "contents",
+                        },
+                    },
+                },
+            },
+        });
+    });
+
+    afterAll(() => {
+        mockFs.restore();
+    });
+
     it("writes, reads and deletes a file as text", async () => {
         const filePath = path.join(process.cwd(), "test-output", `${shortid.generate()}.json`);
         const contents = {
@@ -89,4 +112,35 @@ describe("LocalFileSystem Storage Provider", () => {
     it("deleting file that doesn't exist resolves successfully", async () => {
         await expect(localFileSystem.deleteFile("/path/to/fake/file.txt")).resolves.not.toBeNull();
     });
+
+    it("getAssets uses an absolute path when relative not specified", async () => {
+        AssetService.createAssetFromFilePath = jest.fn(() => []);
+        await localFileSystem.getAssets(sourcePath);
+        const calls: any[] = (AssetService.createAssetFromFilePath as any).mock.calls;
+        expect(calls).toHaveLength(3);
+        calls.forEach((call, index) => {
+            const absolutePath = path.join(sourcePath, `file${index + 1}.jpg`);
+            expect(call).toEqual([
+                absolutePath,
+                undefined,
+                absolutePath,
+            ]);
+        });
+    });
+
+    it("getAssets uses a path relative to the source connection when specified", async () => {
+        AssetService.createAssetFromFilePath = jest.fn(() => []);
+        await localFileSystem.getAssets(sourcePath, true);
+        const calls: any[] = (AssetService.createAssetFromFilePath as any).mock.calls;
+        expect(calls).toHaveLength(3);
+        calls.forEach((call, index) => {
+            const relativePath = `file${index + 1}.jpg`;
+            const absolutePath = path.join(sourcePath, relativePath);
+            expect(call).toEqual([
+                absolutePath,
+                undefined,
+                relativePath,
+            ]);
+        });
+    });
 });

src/electron/providers/storage/localFileSystem.ts

@@ -3,9 +3,10 @@ import fs from "fs";
 import path from "path";
 import rimraf from "rimraf";
 import { IStorageProvider } from "../../../providers/storage/storageProviderFactory";
-import { IAsset, AssetType, StorageType } from "../../../models/applicationState";
+import { IAsset, AssetType, StorageType, IConnection } from "../../../models/applicationState";
 import { AssetService } from "../../../services/assetService";
 import { strings } from "../../../common/strings";
+import { ILocalFileSystemProxyOptions } from "../../../providers/storage/localFileSystemProxy";
 
 export default class LocalFileSystem implements IStorageProvider {
     public storageType: StorageType.Local;
@@ -136,9 +137,12 @@ export default class LocalFileSystem implements IStorageProvider {
         });
     }
 
-    public async getAssets(folderPath?: string): Promise<IAsset[]> {
+    public async getAssets(sourceConnectionFolderPath?: string, relativePath: boolean = false): Promise<IAsset[]> {
-        return (await this.listFiles(path.normalize(folderPath)))
+        return (await this.listFiles(path.normalize(sourceConnectionFolderPath)))
-            .map((filePath) => AssetService.createAssetFromFilePath(filePath))
+            .map((filePath) => AssetService.createAssetFromFilePath(
+                filePath,
+                undefined,
+                relativePath ? path.relative(sourceConnectionFolderPath, filePath) : filePath))
             .filter((asset) => asset.type !== AssetType.Unknown);
     }
 

src/providers/activeLearning/electronProxyHandler.test.ts

@@ -18,7 +18,7 @@ describe("Load default model from filesystem with TF io.IOHandler", () => {
             return Promise.resolve([]);
         });
 
-        const handler = new ElectronProxyHandler("folder");
+        const handler = new ElectronProxyHandler("folder", false);
         try {
             const model = await tf.loadGraphModel(handler);
         } catch (_) {

src/providers/activeLearning/electronProxyHandler.ts

@@ -4,8 +4,8 @@ import { LocalFileSystemProxy, ILocalFileSystemProxyOptions } from "../../provid
 export class ElectronProxyHandler implements tfc.io.IOHandler {
     protected readonly provider: LocalFileSystemProxy;
 
-    constructor(folderPath: string) {
+    constructor(folderPath: string, relativePath: boolean) {
-        const options: ILocalFileSystemProxyOptions = { folderPath };
+        const options: ILocalFileSystemProxyOptions = { folderPath, relativePath };
         this.provider = new LocalFileSystemProxy(options);
     }
 

src/providers/activeLearning/objectDetection.ts

@@ -53,7 +53,7 @@ export class ObjectDetection {
                 const response = await axios.get(modelFolderPath + "/classes.json");
                 this.jsonClasses = JSON.parse(JSON.stringify(response.data));
             } else {
-                const handler = new ElectronProxyHandler(modelFolderPath);
+                const handler = new ElectronProxyHandler(modelFolderPath, false);
                 this.model = await tf.loadGraphModel(handler);
                 this.jsonClasses = await handler.loadClasses();
             }

src/providers/storage/assetProviderFactory.test.ts

@@ -25,7 +25,7 @@ class TestAssetProvider implements IAssetProvider {
     public initialize(): Promise<void> {
         throw new Error("Method not implemented");
     }
-    public getAssets(containerName?: string): Promise<IAsset[]> {
+    public getAssets(): Promise<IAsset[]> {
         throw new Error("Method not implemented.");
     }
 }

src/providers/storage/assetProviderFactory.ts

@@ -10,6 +10,7 @@ import getHostProcess, { HostProcessType } from "../../common/hostProcess";
 export interface IAssetProvider {
     initialize?(): Promise<void>;
     getAssets(containerName?: string): Promise<IAsset[]>;
+    addDefaultPropsToNewConnection?(connection: IConnection): IConnection;
 }
 
 /**

src/providers/storage/azureBlobStorage.ts

@@ -191,8 +191,8 @@ export class AzureBlobStorage implements IStorageProvider {
      * @param containerName - Container from which to retrieve assets. Defaults to
      * container specified in Azure Cloud Storage options
      */
-    public async getAssets(containerName?: string): Promise<IAsset[]> {
+    public async getAssets(): Promise<IAsset[]> {
-        containerName = (containerName) ? containerName : this.options.containerName;
+        const { containerName } = this.options;
         const files = await this.listFiles(containerName);
         const result: IAsset[] = [];
         for (const file of files) {

src/providers/storage/localFileSystemProxy.test.ts

@@ -2,6 +2,7 @@ import { IpcRendererProxy } from "../../common/ipcRendererProxy";
 import { LocalFileSystemProxy, ILocalFileSystemProxyOptions } from "./localFileSystemProxy";
 import { StorageProviderFactory } from "./storageProviderFactory";
 import registerProviders from "../../registerProviders";
+import MockFactory from "../../common/mockFactory";
 
 describe("LocalFileSystem Proxy Storage Provider", () => {
     it("Provider is registered with the StorageProviderFactory", () => {
@@ -19,6 +20,7 @@ describe("LocalFileSystem Proxy Storage Provider", () => {
         let provider: LocalFileSystemProxy = null;
         const options: ILocalFileSystemProxyOptions = {
             folderPath: "/test",
+            relativePath: false,
         };
 
         beforeEach(() => {
@@ -122,5 +124,40 @@ describe("LocalFileSystem Proxy Storage Provider", () => {
             expect(IpcRendererProxy.send).toBeCalledWith("LocalFileSystem:listContainers", [expectedContainerPath]);
             expect(actualFolders).toEqual(expectedFolders);
         });
+
+        it("sends relative path argument according to options", async () => {
+            const sendFunction = jest.fn();
+            IpcRendererProxy.send = sendFunction;
+            await provider.getAssets();
+            const { folderPath, relativePath } = options;
+            expect(IpcRendererProxy.send).toBeCalledWith("LocalFileSystem:getAssets", [folderPath, relativePath]);
+            sendFunction.mockReset();
+
+            const newFolderPath = "myFolder";
+            const newRelativePath = true;
+
+            const relativeProvider = new LocalFileSystemProxy({
+                folderPath: newFolderPath,
+                relativePath: newRelativePath,
+            });
+            await relativeProvider.getAssets();
+            expect(IpcRendererProxy.send).toBeCalledWith("LocalFileSystem:getAssets", [newFolderPath, newRelativePath]);
+        });
+
+        it("adds default props to a new connection", () => {
+            const connection = MockFactory.createTestConnection();
+            delete connection.providerOptions["relativePath"];
+            expect(connection).not.toHaveProperty("providerOptions.relativePath");
+            delete connection.id;
+            expect(provider.addDefaultPropsToNewConnection(connection))
+                .toHaveProperty("providerOptions.relativePath", true);
+        });
+
+        it("does not add default props to existing connection", () => {
+            const connection = MockFactory.createTestConnection();
+            delete connection.providerOptions["relativePath"];
+            expect(provider.addDefaultPropsToNewConnection(connection))
+                .not.toHaveProperty("providerOptions.relativePath");
+        });
     });
 });

src/providers/storage/localFileSystemProxy.ts

@@ -1,7 +1,7 @@
 import { IpcRendererProxy } from "../../common/ipcRendererProxy";
 import { IStorageProvider } from "./storageProviderFactory";
 import { IAssetProvider } from "./assetProviderFactory";
-import { IAsset, StorageType } from "../../models/applicationState";
+import { IAsset, IConnection, StorageType } from "../../models/applicationState";
 
 const PROXY_NAME = "LocalFileSystem";
 
@@ -11,6 +11,7 @@ const PROXY_NAME = "LocalFileSystem";
  */
 export interface ILocalFileSystemProxyOptions {
     folderPath: string;
+    relativePath: boolean;
 }
 
 /**
@@ -26,6 +27,7 @@ export class LocalFileSystemProxy implements IStorageProvider, IAssetProvider {
         if (!this.options) {
             this.options = {
                 folderPath: null,
+                relativePath: false,
             };
         }
     }
@@ -125,8 +127,26 @@ export class LocalFileSystemProxy implements IStorageProvider, IAssetProvider {
      * Retrieve assets from directory
      * @param folderName - Directory containing assets
      */
-    public getAssets(folderName?: string): Promise<IAsset[]> {
+    public getAssets(): Promise<IAsset[]> {
-        const folderPath = [this.options.folderPath, folderName].join("/");
+        const { folderPath, relativePath } = this.options;
-        return IpcRendererProxy.send(`${PROXY_NAME}:getAssets`, [folderPath]);
+        return IpcRendererProxy.send(`${PROXY_NAME}:getAssets`, [folderPath, relativePath]);
+    }
+
+    /**
+     * Adds default properties to new connections
+     *
+     * Currently adds `relativePath: true` to the providerOptions. Pre-existing connections
+     * will only use absolute path
+     *
+     * @param connection Connection
+     */
+    public addDefaultPropsToNewConnection(connection: IConnection): IConnection {
+        return connection.id ? connection : {
+            ...connection,
+            providerOptions: {
+                ...connection.providerOptions,
+                relativePath: true,
+            } as any,
+        };
     }
 }

src/providers/storage/storageProviderFactory.test.ts

@@ -51,7 +51,7 @@ class TestStorageProvider implements IStorageProvider {
     public deleteContainer(folderPath: string): Promise<void> {
         throw new Error("Method not implemented.");
     }
-    public getAssets(containerName?: string): Promise<IAsset[]> {
+    public getAssets(): Promise<IAsset[]> {
         throw new Error("Method not implemented.");
     }
 }

src/react/components/pages/connections/connectionsPage.tsx

@@ -11,6 +11,7 @@ import ConnectionForm from "./connectionForm";
 import ConnectionItem from "./connectionItem";
 import "./connectionsPage.scss";
 import { toast } from "react-toastify";
+import { AssetProviderFactory } from "../../../../providers/storage/assetProviderFactory";
 
 /**
  * Properties for Connection Page
@@ -134,12 +135,20 @@ export default class ConnectionPage extends React.Component<IConnectionPageProps
     }
 
     private onFormSubmit = async (connection: IConnection) => {
+        connection = this.addDefaultPropsIfNewConnection(connection);
         await this.props.actions.saveConnection(connection);
         toast.success(interpolate(strings.connections.messages.saveSuccess, { connection }));
 
         this.props.history.goBack();
     }
 
+    private addDefaultPropsIfNewConnection(connection: IConnection): IConnection {
+        const assetProvider = AssetProviderFactory.createFromConnection(connection);
+        return !connection.id && assetProvider.addDefaultPropsToNewConnection
+            ? assetProvider.addDefaultPropsToNewConnection(connection)
+            : connection;
+    }
+
     private onFormCancel() {
         this.props.history.goBack();
     }

src/services/assetService.test.ts

@@ -9,6 +9,7 @@ import HtmlFileReader from "../common/htmlFileReader";
 import { encodeFileURI } from "../common/utils";
 import _ from "lodash";
 import registerMixins from "../registerMixins";
+import MD5 from "md5.js";
 
 describe("Asset Service", () => {
     describe("Static Methods", () => {
@@ -24,6 +25,22 @@ describe("Asset Service", () => {
             expect(asset.format).toEqual("jpg");
         });
 
+        it("creates an asset using file path as identifier", () => {
+            const path = "c:/dir1/dir2/asset1.jpg";
+            const asset = AssetService.createAssetFromFilePath(path);
+            const expectedIdenfifier = `file:${path}`;
+            const expectedId = new MD5().update(expectedIdenfifier).digest("hex");
+            expect(asset.id).toEqual(expectedId);
+        });
+
+        it("creates an asset using passed in identifier", () => {
+            const path = "C:\\dir1\\dir2\\asset1.jpg";
+            const identifier = "asset1.jpg";
+            const asset = AssetService.createAssetFromFilePath(path, undefined, identifier);
+            const expectedId = new MD5().update(identifier).digest("hex");
+            expect(asset.id).toEqual(expectedId);
+        });
+
         it("creates an asset from an encoded file", () => {
             const path = "C:\\dir1\\dir2\\asset%201.jpg";
             const asset = AssetService.createAssetFromFilePath(path);

src/services/assetService.ts

@@ -23,13 +23,15 @@ export class AssetService {
 
     /**
      * Create IAsset from filePath
-     * @param filePath - filepath of asset
+     * @param assetFilePath - filepath of asset
-     * @param fileName - name of asset
+     * @param assetFileName - name of asset
      */
-    public static createAssetFromFilePath(filePath: string, fileName?: string): IAsset {
+    public static createAssetFromFilePath(
-        Guard.empty(filePath);
+            assetFilePath: string,
-
+            assetFileName?: string,
-        const normalizedPath = filePath.toLowerCase();
+            assetIdentifier?: string): IAsset {
+        Guard.empty(assetFilePath);
+        const normalizedPath = assetFilePath.toLowerCase();
 
         // If the path is not already prefixed with a protocol
         // then assume it comes from the local file system
@@ -37,17 +39,18 @@ export class AssetService {
             !normalizedPath.startsWith("https://") &&
             !normalizedPath.startsWith("file:")) {
             // First replace \ character with / the do the standard url encoding then encode unsupported characters
-            filePath = encodeFileURI(filePath, true);
+            assetFilePath = encodeFileURI(assetFilePath, true);
         }
+        assetIdentifier = assetIdentifier || assetFilePath;
 
-        const md5Hash = new MD5().update(filePath).digest("hex");
+        const md5Hash = new MD5().update(assetIdentifier).digest("hex");
-        const pathParts = filePath.split(/[\\\/]/);
+        const pathParts = assetFilePath.split(/[\\\/]/);
         // Example filename: video.mp4#t=5
         // fileNameParts[0] = "video"
         // fileNameParts[1] = "mp4"
         // fileNameParts[2] = "t=5"
-        fileName = fileName || pathParts[pathParts.length - 1];
+        assetFileName = assetFileName || pathParts[pathParts.length - 1];
-        const fileNameParts = fileName.split(".");
+        const fileNameParts = assetFileName.split(".");
         const extensionParts = fileNameParts[fileNameParts.length - 1].split(/[\?#]/);
         const assetFormat = extensionParts[0];
 
@@ -58,8 +61,8 @@ export class AssetService {
             format: assetFormat,
             state: AssetState.NotVisited,
             type: assetType,
-            name: fileName,
+            name: assetFileName,
-            path: filePath,
+            path: assetFilePath,
             size: null,
         };
     }