From 7237ab340f03a700a802b8fc37a46e0ad06fbd55 Mon Sep 17 00:00:00 2001 From: "Juan Picado @jotadeveloper" Date: Sat, 17 Mar 2018 11:54:57 +0100 Subject: [PATCH] refactor: add flow support on publish endpoint --- src/api/endpoint/api/package.js | 2 +- src/api/endpoint/api/publish.js | 49 +++++++++++++++++++-------------- src/lib/local-storage.js | 10 +++---- src/lib/storage.js | 8 +++--- src/lib/utils.js | 7 +++-- test/unit/store.spec.js | 2 +- test/unit/tag_version.spec.js | 2 +- types/index.js | 10 ++++--- 8 files changed, 51 insertions(+), 39 deletions(-) diff --git a/src/api/endpoint/api/package.js b/src/api/endpoint/api/package.js index 951b4a2f7..311b19809 100644 --- a/src/api/endpoint/api/package.js +++ b/src/api/endpoint/api/package.js @@ -47,7 +47,7 @@ export default function(route: Router, auth: IAuth, storage: IStorageHandler, co }); route.get('/:package/-/:filename', can('access'), function(req: $RequestExtend, res: $ResponseExtend) { - const stream = storage.get_tarball(req.params.package, req.params.filename); + const stream = storage.getTarball(req.params.package, req.params.filename); stream.on('content-length', function(content) { res.header('Content-Length', content); diff --git a/src/api/endpoint/api/publish.js b/src/api/endpoint/api/publish.js index 2355ed84f..f929a9858 100644 --- a/src/api/endpoint/api/publish.js +++ b/src/api/endpoint/api/publish.js @@ -1,23 +1,27 @@ -const _ = require('lodash'); -const Path = require('path'); -const createError = require('http-errors'); +// @flow -const {media, expect_json, allow} = require('../../middleware'); -const Notify = require('../../../lib/notify'); -const {DIST_TAGS, validate_metadata, isObject} = require('../../../lib/utils'); -const mime = require('mime'); +import _ from 'lodash'; +import Path from 'path'; +import mime from 'mime'; -const notify = Notify.notify; +import {DIST_TAGS, validate_metadata, isObject, ErrorCode} from '../../../lib/utils'; +import {media, expect_json, allow} from '../../middleware'; +import {notify} from '../../../lib/notify'; -export default function(router, auth, storage, config) { +import type {Router} from 'express'; +import type {Config, Callback} from '@verdaccio/types'; +import type {IAuth, $ResponseExtend, $RequestExtend, $NextFunctionVer, IStorageHandler} from '../../../../types'; + +export default function(router: Router, auth: IAuth, storage: IStorageHandler, config: Config) { const can = allow(auth); // publishing a package - router.put('/:package/:_rev?/:revision?', can('publish'), media(mime.getType('json')), expect_json, function(req, res, next) { + router.put('/:package/:_rev?/:revision?', can('publish'), + media(mime.getType('json')), expect_json, function(req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer) { const name = req.params.package; let metadata; - const create_tarball = function(filename, data, cb) { - let stream = storage.add_tarball(name, filename); + const create_tarball = function(filename: string, data, cb: Callback) { + let stream = storage.addTarball(name, filename); stream.on('error', function(err) { cb(err); }); @@ -26,6 +30,8 @@ export default function(router, auth, storage, config) { }); // this is dumb and memory-consuming, but what choices do we have? + // flow: we need first refactor this file before decides which type use here + // $FlowFixMe stream.end(new Buffer(data.data, 'base64')); stream.done(); }; @@ -59,7 +65,7 @@ export default function(router, auth, storage, config) { || Object.keys(metadata.versions).length !== 1) { // npm is doing something strange again // if this happens in normal circumstances, report it as a bug - return next( createError[400]('unsupported registry call') ); + return next(ErrorCode.get400('unsupported registry call')); } if (err && err.status != 409) { @@ -94,13 +100,13 @@ export default function(router, auth, storage, config) { if (Object.keys(req.body).length === 1 && isObject(req.body.users)) { // 501 status is more meaningful, but npm doesn't show error message for 5xx - return next( createError[404]('npm star|unstar calls are not implemented') ); + return next(ErrorCode.get404('npm star|unstar calls are not implemented')); } try { metadata = validate_metadata(req.body, name); } catch(err) { - return next( createError[422]('bad incoming package data') ); + return next(ErrorCode.get422('bad incoming package data')); } if (req.params._rev) { @@ -115,7 +121,7 @@ export default function(router, auth, storage, config) { }); // unpublishing an entire package - router.delete('/:package/-rev/*', can('publish'), function(req, res, next) { + router.delete('/:package/-rev/*', can('publish'), function(req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer) { storage.remove_package(req.params.package, function(err) { if (err) { return next(err); @@ -126,7 +132,8 @@ export default function(router, auth, storage, config) { }); // removing a tarball - router.delete('/:package/-/:filename/-rev/:revision', can('publish'), function(req, res, next) { + router.delete('/:package/-/:filename/-rev/:revision', can('publish'), + function(req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer) { storage.remove_tarball(req.params.package, req.params.filename, req.params.revision, function(err) { if (err) { return next(err); @@ -137,9 +144,10 @@ export default function(router, auth, storage, config) { }); // uploading package tarball - router.put('/:package/-/:filename/*', can('publish'), media('application/octet-stream'), function(req, res, next) { + router.put('/:package/-/:filename/*', can('publish'), media('application/octet-stream'), + function(req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer) { const name = req.params.package; - const stream = storage.add_tarball(name, req.params.filename); + const stream = storage.addTarball(name, req.params.filename); req.pipe(stream); // checking if end event came before closing @@ -166,7 +174,8 @@ export default function(router, auth, storage, config) { }); // adding a version - router.put('/:package/:version/-tag/:tag', can('publish'), media(mime.getType('json')), expect_json, function(req, res, next) { + router.put('/:package/:version/-tag/:tag', can('publish'), + media(mime.getType('json')), expect_json, function(req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer) { let name = req.params.package; let version = req.params.version; let tag = req.params.tag; diff --git a/src/lib/local-storage.js b/src/lib/local-storage.js index 08f5caa67..969a50a35 100644 --- a/src/lib/local-storage.js +++ b/src/lib/local-storage.js @@ -10,7 +10,7 @@ import UrlNode from 'url'; import _ from 'lodash'; // $FlowFixMe import async from 'async'; -import {ErrorCode, isObject, getLatestVersion, tag_version, validate_name, semverSort, DIST_TAGS} from './utils'; +import {ErrorCode, isObject, getLatestVersion, tagVersion, validate_name, semverSort, DIST_TAGS} from './utils'; import { generatePackageTemplate, normalizePackage, generateRevision, cleanUpReadme, fileExist, noSuchFile, DEFAULT_REVISION, pkgFileName, @@ -35,7 +35,7 @@ import type { IUploadTarball, IReadTarball, } from '@verdaccio/streams'; -import type {IStorage} from '../../types'; +import type {IStorage, StringValue} from '../../types'; /** * Implements Storage interface (same for storage.js, local-storage.js, up-storage.js). @@ -233,7 +233,7 @@ class LocalStorage implements IStorage { addVersion(name: string, version: string, metadata: Version, - tag: string, + tag: StringValue, callback: Callback) { this._updatePackage(name, (data, cb) => { // keep only one readme per package @@ -272,7 +272,7 @@ class LocalStorage implements IStorage { } data.versions[version] = metadata; - tag_version(data, version, tag); + tagVersion(data, version, tag); let addFailed = this.localData.add(name); if (addFailed) { @@ -302,7 +302,7 @@ class LocalStorage implements IStorage { return cb( this._getVersionNotFound() ); } const key: string = tags[t]; - tag_version(data, key, t); + tagVersion(data, key, t); } cb(); }, callback); diff --git a/src/lib/storage.js b/src/lib/storage.js index a0f74388b..8040e4913 100644 --- a/src/lib/storage.js +++ b/src/lib/storage.js @@ -12,7 +12,7 @@ import LocalStorage from './local-storage'; import {ReadTarball} from '@verdaccio/streams'; import ProxyStorage from './up-storage'; import {ErrorCode, normalize_dist_tags, validate_metadata, isObject, DIST_TAGS} from './utils'; -import type {IStorage, IProxy, IStorageHandler, ProxyList} from '../../types'; +import type {IStorage, IProxy, IStorageHandler, ProxyList, StringValue} from '../../types'; import type { Versions, Package, @@ -168,7 +168,7 @@ class Storage implements IStorageHandler { * @param {*} tag * @param {*} callback */ - addVersion(name: string, version: string, metadata: Version, tag: string, callback: Callback) { + addVersion(name: string, version: string, metadata: Version, tag: StringValue, callback: Callback) { this.localStorage.addVersion(name, version, metadata, tag, callback); } @@ -244,7 +244,7 @@ class Storage implements IStorageHandler { * @param {*} filename * @return {Stream} */ - add_tarball(name: string, filename: string): IUploadTarball { + addTarball(name: string, filename: string): IUploadTarball { return this.localStorage.addTarball(name, filename); } @@ -258,7 +258,7 @@ class Storage implements IStorageHandler { * @param {*} filename * @return {Stream} */ - get_tarball(name: string, filename: string) { + getTarball(name: string, filename: string) { let readStream = new ReadTarball(); readStream.abort = function() {}; diff --git a/src/lib/utils.js b/src/lib/utils.js index 3f5708acb..d8ce9d1dd 100644 --- a/src/lib/utils.js +++ b/src/lib/utils.js @@ -10,6 +10,7 @@ import _ from 'lodash'; import createError from 'http-errors'; import type {Package, Config} from '@verdaccio/types'; import type {$Request} from 'express'; +import type {StringValue} from '../../types'; const Logger = require('./logger'); @@ -145,8 +146,8 @@ function filter_tarball_urls(pkg: Package, req: $Request, config: Config) { * @param {*} tag * @return {Boolean} whether a package has been tagged */ -function tag_version(data: Package, version: string, tag: string) { - if (_.isEmpty(tag) === false) { +function tagVersion(data: Package, version: string, tag: StringValue) { + if (tag) { if (data[DIST_TAGS][tag] !== version) { if (semver.parse(version, true)) { // valid version - store @@ -443,7 +444,7 @@ export { parse_address, get_version, normalize_dist_tags, - tag_version, + tagVersion, combineBaseUrl, filter_tarball_urls, validate_metadata, diff --git a/test/unit/store.spec.js b/test/unit/store.spec.js index a82eefe53..b66f5d104 100644 --- a/test/unit/store.spec.js +++ b/test/unit/store.spec.js @@ -39,7 +39,7 @@ describe('StorageTest', () => { name: 'react', req: request, callback: () => { - const stream = storage.get_tarball('react', 'react-16.1.0.tgz'); + const stream = storage.getTarball('react', 'react-16.1.0.tgz'); stream.on('content-length', function(content) { if (content) { expect(content).toBeTruthy(); diff --git a/test/unit/tag_version.spec.js b/test/unit/tag_version.spec.js index 5e3749c8f..1d684fbc3 100644 --- a/test/unit/tag_version.spec.js +++ b/test/unit/tag_version.spec.js @@ -1,5 +1,5 @@ let assert = require('assert'); -let tag_version = require('../../src/lib/utils').tag_version; +let tag_version = require('../../src/lib/utils').tagVersion; require('../../src/lib/logger').setup([]); diff --git a/types/index.js b/types/index.js index c9ea711ba..cf3b2ded7 100644 --- a/types/index.js +++ b/types/index.js @@ -16,6 +16,8 @@ import type { import type {ILocalData} from '@verdaccio/local-storage'; import type {NextFunction, $Request, $Response} from 'request'; +export type StringValue = string | void | null; + export interface IAuth { config: Config; logger: Logger; @@ -77,14 +79,14 @@ export interface IStorageHandler { logger: Logger; uplinks: ProxyList; addPackage(name: string, metadata: any, callback: Function): void; - addVersion(name: string, version: string, metadata: Version, tag: string, callback: Callback): void; + addVersion(name: string, version: string, metadata: Version, tag: StringValue, callback: Callback): void; mergeTags(name: string, tagHash: MergeTags, callback: Callback): void; replace_tags(name: string, tagHash: MergeTags, callback: Callback): void; change_package(name: string, metadata: Package, revision: string, callback: Callback): void; remove_package(name: string, callback: Callback): void; remove_tarball(name: string, filename: string, revision: string, callback: Callback): void; - add_tarball(name: string, filename: string): IUploadTarball; - get_tarball(name: string, filename: string): IReadTarball; + addTarball(name: string, filename: string): IUploadTarball; + getTarball(name: string, filename: string): IReadTarball; getPackage(options: any): void; search(startkey: string, options: any): void; getLocalDatabase(callback: Callback): void; @@ -100,7 +102,7 @@ export interface IStorage { addPackage(name: string, info: Package, callback: Callback): void; removePackage(name: string, callback: Callback): void; updateVersions(name: string, packageInfo: Package, callback: Callback): void; - addVersion(name: string, version: string, metadata: Version, tag: string, callback: Callback): void; + addVersion(name: string, version: string, metadata: Version, tag: StringValue, callback: Callback): void; mergeTags(name: string, tags: MergeTags, callback: Callback): void; changePackage(name: string, metadata: Package, revision: string, callback: Callback): void; removeTarball(name: string, filename: string, revision: string, callback: Callback): void;