fix(verdaccio-memory): race condition on save a package (#365)

* fix(verdaccio-memory): race condition on save a package * chore: reable node 8 * add new tests
verdaccio · Jun 7, 2020 · 70c1fb1 · 70c1fb1
1 parent 4a1fb42
commit 70c1fb1
Show file tree

Hide file tree

Showing 6 changed files with 338 additions and 164 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -9,7 +9,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        node_version: [10, 12, 14]
+        node_version: [8, 10, 12, 14]
         os: [ubuntu-latest, windows-latest, macOS-latest]
 
     runs-on: ${{ matrix.os }}

diff --git a/plugins/memory/src/local-memory.ts b/plugins/memory/src/local-memory.ts
@@ -89,21 +89,24 @@ class LocalMemory implements IPluginStorage<ConfigMemory> {
   }
 
   public saveToken(token: Token): Promise<void> {
-    this.logger.warn({ token }, 'save token has not been implemented yet @{token}');
+    this.logger.warn('[verdaccio/memory][saveToken] save token has not been implemented yet');
 
-    return Promise.reject(getServiceUnavailable('[saveToken] method not implemented'));
+    return Promise.reject(getServiceUnavailable('method not implemented'));
   }
 
   public deleteToken(user: string, tokenKey: string): Promise<void> {
-    this.logger.warn({ tokenKey, user }, 'delete token has not been implemented yet @{user}');
+    this.logger.warn(
+      { tokenKey, user },
+      '[verdaccio/memory][deleteToken] delete token has not been implemented yet @{user}'
+    );
 
-    return Promise.reject(getServiceUnavailable('[deleteToken] method not implemented'));
+    return Promise.reject(getServiceUnavailable('method not implemented'));
   }
 
   public readTokens(filter: TokenFilter): Promise<Token[]> {
-    this.logger.warn({ filter }, 'read tokens has not been implemented yet @{filter}');
+    this.logger.warn('[verdaccio/memory][readTokens] read tokens has not been implemented yet ');
 
-    return Promise.reject(getServiceUnavailable('[readTokens] method not implemented'));
+    return Promise.reject(getServiceUnavailable('method not implemented'));
   }
 }
 

diff --git a/plugins/memory/src/memory-handler.ts b/plugins/memory/src/memory-handler.ts
@@ -15,6 +15,8 @@ import {
   ReadPackageCallback,
 } from '@verdaccio/types';
 
+import { parsePackage, stringifyPackage } from './utils';
+
 const fs = new MemoryFileSystem();
 
 export type DataHandler = {
@@ -46,7 +48,7 @@ class MemoryHandler implements IPackageStorageManager {
     let pkg: Package;
 
     try {
-      pkg = JSON.parse(json) as Package;
+      pkg = parsePackage(json) as Package;
     } catch (err) {
       return onEnd(err);
     }
@@ -58,18 +60,18 @@ class MemoryHandler implements IPackageStorageManager {
       try {
         onWrite(pkgFileName, transformPackage(pkg), onEnd);
       } catch (err) {
-        return onEnd(getInternalError('error on parse packument'));
+        return onEnd(getInternalError('error on parse the metadata'));
       }
     });
   }
 
   public deletePackage(pkgName: string, callback: Callback): void {
     delete this.data[pkgName];
-    callback(null);
+    return callback(null);
   }
 
   public removePackage(callback: CallbackAction): void {
-    callback(null);
+    return callback(null);
   }
 
   public createPackage(name: string, value: Package, cb: CallbackAction): void {
@@ -78,24 +80,23 @@ class MemoryHandler implements IPackageStorageManager {
 
   public savePackage(name: string, value: Package, cb: CallbackAction): void {
     try {
-      const json: string = JSON.stringify(value, null, '\t');
+      const json: string = stringifyPackage(value);
 
       this.data[name] = json;
+      return cb(null);
     } catch (err) {
-      cb(getInternalError(err.message));
+      return cb(getInternalError(err.message));
     }
-
-    cb(null);
   }
 
   public readPackage(name: string, cb: ReadPackageCallback): void {
     const json = this._getStorage(name);
     const isJson = typeof json === 'undefined';
 
     try {
-      cb(isJson ? getNotFound() : null, JSON.parse(json));
+      return cb(isJson ? getNotFound() : null, parsePackage(json));
     } catch (err) {
-      cb(getNotFound());
+      return cb(getNotFound());
     }
   }
 
@@ -128,8 +129,10 @@ class MemoryHandler implements IPackageStorageManager {
           };
 
           uploadStream.emit('open');
+          return;
         } catch (err) {
           uploadStream.emit('error', err);
+          return;
         }
       });
     });
@@ -162,8 +165,10 @@ class MemoryHandler implements IPackageStorageManager {
           readTarballStream.abort = function(): void {
             readStream.destroy(getBadRequest('read has been aborted'));
           };
+          return;
         } catch (err) {
           readTarballStream.emit('error', err);
+          return;
         }
       });
     });

diff --git a/plugins/memory/src/utils.ts b/plugins/memory/src/utils.ts
@@ -0,0 +1,9 @@
+import { Package } from '@verdaccio/types';
+
+export function stringifyPackage(pkg: Package) {
+  return JSON.stringify(pkg, null, '\t');
+}
+
+export function parsePackage(pkg: string) {
+  return JSON.parse(pkg);
+}
diff --git a/plugins/memory/test/local-memory.spec.ts b/plugins/memory/test/local-memory.spec.ts
@@ -0,0 +1,75 @@
+import { Logger, IPluginStorage, IPackageStorage } from '@verdaccio/types';
+import { VerdaccioError } from '@verdaccio/commons-api';
+
+import { ConfigMemory } from '../src/local-memory';
+import { DataHandler } from '../src/memory-handler';
+import LocalMemory from '../src/index';
+
+import config from './partials/config';
+
+const logger: Logger = {
+  error: e => console.warn(e),
+  info: e => console.warn(e),
+  debug: e => console.warn(e),
+  child: e => console.warn(e),
+  warn: e => console.warn(e),
+  http: e => console.warn(e),
+  trace: e => console.warn(e),
+};
+
+const defaultConfig = { logger, config: null };
+
+describe('memory unit test .', () => {
+  describe('LocalMemory', () => {
+    test('should create an LocalMemory instance', () => {
+      const localMemory: IPluginStorage<ConfigMemory> = new LocalMemory(config, defaultConfig);
+
+      expect(localMemory).toBeDefined();
+    });
+
+    test('should create add a package', done => {
+      const localMemory: IPluginStorage<ConfigMemory> = new LocalMemory(config, defaultConfig);
+      localMemory.add('test', (err: VerdaccioError) => {
+        expect(err).toBeNull();
+        localMemory.get((err: VerdaccioError, data: DataHandler) => {
+          expect(err).toBeNull();
+          expect(data).toHaveLength(1);
+          done();
+        });
+      });
+    });
+
+    test('should reach max limit', done => {
+      config.limit = 2;
+      const localMemory: IPluginStorage<ConfigMemory> = new LocalMemory(config, defaultConfig);
+
+      localMemory.add('test1', err => {
+        expect(err).toBeNull();
+        localMemory.add('test2', err => {
+          expect(err).toBeNull();
+          localMemory.add('test3', err => {
+            expect(err).not.toBeNull();
+            expect(err.message).toMatch(/Storage memory has reached limit of limit packages/);
+            done();
+          });
+        });
+      });
+    });
+
+    test('should remove a package', done => {
+      const pkgName = 'test';
+      const localMemory: IPluginStorage<ConfigMemory> = new LocalMemory(config, defaultConfig);
+      localMemory.add(pkgName, err => {
+        expect(err).toBeNull();
+        localMemory.remove(pkgName, err => {
+          expect(err).toBeNull();
+          localMemory.get((err, data) => {
+            expect(err).toBeNull();
+            expect(data).toHaveLength(0);
+            done();
+          });
+        });
+      });
+    });
+  });
+});