fix(temp-storage): move workingCache to temporary storage (fixes #1136)…

… (#1727) * fix(temp-storage): move workingCache to temporary storage * fix(temp-storage): remove old workingCache directory on setup * fix(content-storage): rename workingCache to contentStorage * fix(temp-storage): also remove deprecated tmp storage in userData * test(lint): make linter happy * test(format): fix format * refactor(setup): changed use of fs-extra Co-authored-by: Sebastian Rettig <serettig@posteo.de>
Lumieducation · Aug 2, 2021 · 274dd65 · 274dd65
1 parent 4619d09
commit 274dd65
Show file tree

Hide file tree

Showing 9 changed files with 63 additions and 27 deletions.
diff --git a/server/src/IServerConfig.ts b/server/src/IServerConfig.ts
@@ -1,8 +1,8 @@
 export default interface IServerConfig {
     cache: string;
     configFile: string;
+    contentStoragePath: string;
     librariesPath: string;
     settingsFile: string;
     temporaryStoragePath: string;
-    workingCachePath: string;
 }
diff --git a/server/src/boot/app.ts b/server/src/boot/app.ts
@@ -61,7 +61,7 @@ export default async (
     const h5pEditor: H5P.H5PEditor = await createH5PEditor(
         config,
         options?.libraryDir ?? serverConfig.librariesPath, // the path on the local disc where libraries should be stored)
-        serverConfig.workingCachePath, // the path on the local disc where content is stored. Only used / necessary if you use the local filesystem content storage class.
+        serverConfig.contentStoragePath, // the path on the local disc where content is stored. Only used / necessary if you use the local filesystem content storage class.
         serverConfig.temporaryStoragePath, // the path on the local disc where temporary files (uploads) should be stored. Only used / necessary if you use the local filesystem temporary storage class.
         (key, language) => translationFunction(key, { lng: language }),
         {

diff --git a/server/src/boot/setup.ts b/server/src/boot/setup.ts
@@ -1,20 +1,37 @@
 import * as Sentry from '@sentry/electron';
 import fsExtra from 'fs-extra';
 import { app } from 'electron';
-
+import path from 'path';
 import IServerConfig from '../IServerConfig';
 import { fsImplementations, H5PConfig } from '@lumieducation/h5p-server';
 import defaultSettings from './defaultSettings';
-import defaultRun from './defaultRun';
 
 import settingsCache from '../settingsCache';
 
 export default async function setup(
     serverConfig: IServerConfig
 ): Promise<void> {
     try {
+        // If the workingCache (prior 0.8.0) still exists in userData remove it. -> https://github.com/Lumieducation/Lumi/pull/1727
+        const deprecatedContentStoragePath = path.join(
+            process.env.USERDATA || app.getPath('userData'),
+            'workingCache'
+        );
+
+        const deprecatedTemporaryStoragePath = path.join(
+            process.env.USERDATA || app.getPath('userData'),
+            'tmp'
+        );
+
+        if (await fsExtra.pathExists(deprecatedContentStoragePath)) {
+            fsExtra.remove(deprecatedContentStoragePath); // deliberately without await to not block the setup if it takes long.
+        }
+
+        if (await fsExtra.pathExists(deprecatedTemporaryStoragePath)) {
+            fsExtra.remove(deprecatedTemporaryStoragePath); // deliberately without await to not block the setup if it takes long.
+        }
+
         // Make sure required directories exist
-        await fsExtra.mkdirp(serverConfig.workingCachePath);
         await fsExtra.mkdirp(serverConfig.librariesPath);
         await fsExtra.mkdirp(serverConfig.temporaryStoragePath);
 

diff --git a/server/src/main.ts b/server/src/main.ts
@@ -13,13 +13,16 @@ import serverConfigFactory from './serverConfig';
 import matomo from './matomo';
 import { machineId } from 'node-machine-id';
 import i18next from 'i18next';
+import fsExtra from 'fs-extra';
 
 import settingsCache from './settingsCache';
 import electronState from './electronState';
 import DelayedEmitter from './helpers/DelayedEmitter';
 
 const app = electron.app;
 let websocket: SocketIO.Server;
+const tmpDir = process.env.TEMPDATA || path.join(app.getPath('temp'), 'lumi');
+
 /**
  * The DelayedEmitter queues websocket events until the websocket is connected.
  * We need it as the browser window and the client must be initialized before
@@ -110,7 +113,8 @@ if (!gotSingleInstanceLock) {
     app.quit();
 } else {
     const serverConfig = serverConfigFactory(
-        process.env.USERDATA || app.getPath('userData')
+        process.env.USERDATA || app.getPath('userData'),
+        tmpDir
     );
     Sentry.init({
         dsn: 'http://1f4ae874b81a48ed8e22fe6e9d52ed1b@sentry.lumi.education/3',
@@ -212,22 +216,21 @@ if (!gotSingleInstanceLock) {
         }
     });
 
+    app.on('will-quit', (event) => {
+        log.debug(`Deleting temporary directory: ${tmpDir}`);
+        fsExtra.removeSync(tmpDir);
+    });
+
     // create main BrowserWindow when electron is ready
     app.on('ready', async () => {
         log.info('app is ready');
-        const server = await httpServerFactory(
-            serverConfigFactory(
-                process.env.USERDATA || app.getPath('userData')
-            ),
-            mainWindow,
-            {
-                devMode: app.commandLine.hasSwitch('dev'),
-                libraryDir:
-                    app.commandLine.getSwitchValue('libs') !== ''
-                        ? app.commandLine.getSwitchValue('libs')
-                        : undefined
-            }
-        );
+        const server = await httpServerFactory(serverConfig, mainWindow, {
+            devMode: app.commandLine.hasSwitch('dev'),
+            libraryDir:
+                app.commandLine.getSwitchValue('libs') !== ''
+                    ? app.commandLine.getSwitchValue('libs')
+                    : undefined
+        });
         log.info('server booted');
 
         port = (server.address() as any).port;

diff --git a/server/src/routes/__test__/analyticRoutes.test.ts b/server/src/routes/__test__/analyticRoutes.test.ts
@@ -14,7 +14,11 @@ describe('[analytics:routes]: GET /api/v1/analytics', () => {
                 configFile: path.resolve('test', 'data', 'config.json'),
                 librariesPath: path.resolve('test', 'data', `libraries`),
                 temporaryStoragePath: path.resolve('test', 'data', 'tmp'),
-                workingCachePath: path.resolve('test', 'data', 'workingCache'),
+                contentStoragePath: path.resolve(
+                    'test',
+                    'data',
+                    'workingCache'
+                ),
                 settingsFile: path.resolve('test', 'data', 'settings.json')
             },
             null

diff --git a/server/src/routes/__test__/h5pRoutes.test.ts b/server/src/routes/__test__/h5pRoutes.test.ts
@@ -15,7 +15,11 @@ describe('[export h5p as html]: GET /api/v1/h5p/:contentId/html', () => {
                 configFile: path.resolve('test', 'data', 'config.json'),
                 librariesPath: path.resolve('test', 'data', `libraries`),
                 temporaryStoragePath: path.resolve('test', 'data', 'tmp'),
-                workingCachePath: path.resolve('test', 'data', 'workingCache'),
+                contentStoragePath: path.resolve(
+                    'test',
+                    'data',
+                    'workingCache'
+                ),
                 settingsFile: path.resolve('test', 'data', 'settings.json')
             },
             null

diff --git a/server/src/routes/__test__/settingsRoutes.test.ts b/server/src/routes/__test__/settingsRoutes.test.ts
@@ -17,7 +17,11 @@ describe('GET /settings', () => {
                 configFile: path.resolve('test', 'data', 'config.json'),
                 librariesPath: path.resolve('test', 'data', `libraries`),
                 temporaryStoragePath: path.resolve('test', 'data', 'tmp'),
-                workingCachePath: path.resolve('test', 'data', 'workingCache'),
+                contentStoragePath: path.resolve(
+                    'test',
+                    'data',
+                    'workingCache'
+                ),
                 settingsFile: path.resolve('test', 'data', 'settings.json')
             },
             null
@@ -49,7 +53,11 @@ describe('PATCH /settings', () => {
                 librariesPath: path.resolve('test', 'data', `libraries`),
                 temporaryStoragePath: path.resolve('test', 'data', 'tmp'),
 
-                workingCachePath: path.resolve('test', 'data', 'workingCache'),
+                contentStoragePath: path.resolve(
+                    'test',
+                    'data',
+                    'workingCache'
+                ),
                 settingsFile: path.resolve('test', 'data', 'settings.json')
             },
             null

diff --git a/server/src/routes/index.ts b/server/src/routes/index.ts
@@ -60,7 +60,7 @@ export default function (
     // // multi-user environment.
     // router.use(
     //     h5pConfig.baseUrl + h5pConfig.contentFilesUrl,
-    //     express.static(`${serverConfig.workingCachePath}`)
+    //     express.static(`${serverConfig.contentStoragePath}`)
     // );
     // router.use(
     //     h5pConfig.baseUrl + h5pConfig.librariesUrl,

diff --git a/server/src/serverConfig.ts b/server/src/serverConfig.ts
@@ -1,12 +1,12 @@
 import path from 'path';
 import IServerConfig from './IServerConfig';
 
-export default (userData: string): IServerConfig => {
+export default (userData: string, tempData: string): IServerConfig => {
     return {
         librariesPath: path.join(userData, 'libraries'),
         cache: path.join(userData, 'store.json'),
-        temporaryStoragePath: path.join(userData, 'tmp'),
-        workingCachePath: path.join(userData, 'workingCache'),
+        temporaryStoragePath: path.join(tempData, 'tmp'),
+        contentStoragePath: path.join(tempData, 'contentStorage'),
         configFile: path.join(userData, 'config.json'),
         settingsFile: path.join(userData, 'settings.json')
     };