From 85441c84595a1fd23494f6dc9881ecc3a8101b6f Mon Sep 17 00:00:00 2001 From: alecmerdler Date: Wed, 8 Mar 2017 11:43:53 -0800 Subject: [PATCH] refactoring to promises --- karma.conf.js | 4 + package.json | 1 + .../js/directives/ui/dockerfile-build-form.js | 55 +++++++++---- static/js/quay.module.ts | 1 + .../dockerfile.service.impl.spec.ts | 80 ++++++++++++++++++- .../dockerfile/dockerfile.service.impl.ts | 53 +++++++++++- .../services/dockerfile/dockerfile.service.ts | 9 ++- webpack.config.js | 4 +- yarn.lock | 12 +-- 9 files changed, 191 insertions(+), 28 deletions(-) diff --git a/karma.conf.js b/karma.conf.js index 841933cd6..38d9cd46e 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -24,6 +24,9 @@ module.exports = function(config) { 'node_modules/raven-js/dist/raven.js', 'node_modules/cal-heatmap/cal-heatmap.js', + // Polyfills + 'node_modules/core-js/index.js', + // static/lib resources 'static/lib/**/*.js', @@ -37,6 +40,7 @@ module.exports = function(config) { preprocessors: { 'static/lib/ngReact/react.ngReact.min.js': ['webpack'], 'static/lib/angular-moment.min.js': ['webpack'], + 'node_modules/core-js/index.js': ['webpack'], 'static/js/**/*.spec.ts*': ['webpack'], }, webpack: webpackConfig, diff --git a/package.json b/package.json index f509e7e51..a18699fd4 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ "bootstrap": "^3.3.2", "bootstrap-datepicker": "^1.6.4", "cal-heatmap": "^3.3.10", + "core-js": "^2.4.1", "d3": "^3.3.3", "eonasdan-bootstrap-datetimepicker": "^4.17.43", "jquery": "1.12.4", diff --git a/static/js/directives/ui/dockerfile-build-form.js b/static/js/directives/ui/dockerfile-build-form.js index fdd705f21..8fae11bb7 100644 --- a/static/js/directives/ui/dockerfile-build-form.js +++ b/static/js/directives/ui/dockerfile-build-form.js @@ -39,25 +39,48 @@ angular.module('quay').directive('dockerfileBuildForm', function () { $scope.state = 'checking'; $scope.selectedFiles = files; - DockerfileService.getDockerfile(files[0], function(df) { - var baseImage = df.getRegistryBaseImage(); - if (baseImage) { - checkPrivateImage(baseImage); - } else { - $scope.state = 'ready'; - } + // FIXME: Remove this + // DockerfileService.getDockerfile(files[0], function(df) { + // var baseImage = df.getRegistryBaseImage(); + // if (baseImage) { + // checkPrivateImage(baseImage); + // } else { + // $scope.state = 'ready'; + // } + // + // $scope.$apply(function() { + // opt_callback && opt_callback(true, 'Dockerfile found and valid') + // }); + // }, function(msg) { + // $scope.state = 'empty'; + // $scope.privateBaseRepository = null; + // + // $scope.$apply(function() { + // opt_callback && opt_callback(false, msg || 'Could not find valid Dockerfile'); + // }); + // }); - $scope.$apply(function() { - opt_callback && opt_callback(true, 'Dockerfile found and valid') - }); - }, function(msg) { - $scope.state = 'empty'; - $scope.privateBaseRepository = null; + DockerfileService.extractDockerfile(files[0]) + .then(function(dockerfileInfo) { + var baseImage = dockerfileInfo.getRegistryBaseImage(); + if (baseImage) { + checkPrivateImage(baseImage); + } else { + $scope.state = 'ready'; + } - $scope.$apply(function() { - opt_callback && opt_callback(false, msg || 'Could not find valid Dockerfile'); + $scope.$apply(function() { + opt_callback && opt_callback(true, 'Dockerfile found and valid') + }); + }) + .catch(function(error) { + $scope.state = 'empty'; + $scope.privateBaseRepository = null; + + $scope.$apply(function() { + opt_callback && opt_callback(false, error || 'Could not find valid Dockerfile'); + }); }); - }); }; $scope.handleFilesCleared = function() { diff --git a/static/js/quay.module.ts b/static/js/quay.module.ts index c375d625f..3b558e5d9 100644 --- a/static/js/quay.module.ts +++ b/static/js/quay.module.ts @@ -1,4 +1,5 @@ import * as angular from "angular"; +import 'core-js'; import { ViewArrayImpl } from "./services/view-array/view-array.impl"; import { NAME_PATTERNS } from "./constants/name-patterns.constant"; import { INJECTED_CONFIG, INJECTED_FEATURES, INJECTED_ENDPOINTS } from "./constants/injected-values.constant"; diff --git a/static/js/services/dockerfile/dockerfile.service.impl.spec.ts b/static/js/services/dockerfile/dockerfile.service.impl.spec.ts index 9f3ba1747..20ad4f6f1 100644 --- a/static/js/services/dockerfile/dockerfile.service.impl.spec.ts +++ b/static/js/services/dockerfile/dockerfile.service.impl.spec.ts @@ -170,7 +170,77 @@ describe("DockerfileServiceImpl", () => { }); describe("extractDockerfile", () => { - // TODO: TDD promise-based method with same functionality as getDockerfile + var file: any; + var invalidArchiveFile: any[]; + var validArchiveFile: any[]; + var readAsFileBufferSpy: Spy; + var forDataSpy: Spy; + + beforeEach(() => { + dataFileServiceMock.readDataArrayAsPossibleArchive.and.callFake((buf, success, failure) => { + failure([]); + }); + + dataFileServiceMock.arrayToString.and.callFake((buf, callback) => { + var contents: string = ""; + callback(contents); + }); + + dataFileServiceMock.blobToString.and.callFake((blob, callback) => { + callback(blob.toString()); + }); + + forDataSpy = spyOn(DockerfileInfoImpl, "forData").and.returnValue(new DockerfileInfoImpl(file, configMock)); + readAsFileBufferSpy = spyOn(fileReaderMock, "readAsArrayBuffer").and.callFake(() => { + var event: any = {target: {result: file}}; + fileReaderMock.onload(event); + }); + + file = "FROM quay.io/coreos/nginx:latest"; + validArchiveFile = [{name: 'Dockerfile', toBlob: jasmine.createSpy('toBlobSpy').and.returnValue(file)}]; + invalidArchiveFile = [{name: 'main.exe', toBlob: jasmine.createSpy('toBlobSpy').and.returnValue("")}]; + }); + + it("calls datafile service to read given file as possible archive file", (done) => { + dockerfileServiceImpl.extractDockerfile(file) + .then((dockerfile: DockerfileInfoImpl) => { + expect(readAsFileBufferSpy.calls.argsFor(0)[0]).toEqual(file); + expect(dataFileServiceMock.readDataArrayAsPossibleArchive).toHaveBeenCalled(); + done(); + }) + .catch((error: string) => { + fail('Promise should be resolved'); + done(); + }); + }); + + it("calls datafile service to convert file to string if given file is not an archive", (done) => { + done(); + }); + + it("returns rejected promise if given non-archive file that is not a valid Dockerfile", (done) => { + done(); + }); + + it("returns resolved promise with new DockerfileInfoImpl instance if given valid Dockerfile", (done) => { + done(); + }); + + it("returns rejected promise if given archive file with no Dockerfile present in root directory", (done) => { + done(); + }); + + it("calls datafile service to convert blob to string if given file is an archive", (done) => { + done(); + }); + + it("returns rejected promise if given archive file with invalid Dockerfile", (done) => { + done(); + }); + + it("returns resolved promise of new DockerfileInfoImpl instance if given archive with valid Dockerfile", (done) => { + done(); + }); }); }); @@ -201,9 +271,13 @@ describe("DockerfileInfoImpl", () => { describe("getRegistryBaseImage", () => { var domain: string; + var baseImage: string; beforeEach(() => { domain = "quay.io"; + baseImage = "coreos/nginx"; + + configMock.getDomain.and.returnValue(domain); }); it("returns null if instance's contents do not contain a 'FROM' command", () => { @@ -222,7 +296,9 @@ describe("DockerfileInfoImpl", () => { }); it("returns the registry base image", () => { - spyOn(dockerfileInfoImpl, "getBaseImage").and.returnValue(null); + spyOn(dockerfileInfoImpl, "getBaseImage").and.returnValue(`${domain}/${baseImage}`); + + expect(dockerfileInfoImpl.getRegistryBaseImage()).toEqual(baseImage); }); }); diff --git a/static/js/services/dockerfile/dockerfile.service.impl.ts b/static/js/services/dockerfile/dockerfile.service.impl.ts index d2060b08d..f865ae70f 100644 --- a/static/js/services/dockerfile/dockerfile.service.impl.ts +++ b/static/js/services/dockerfile/dockerfile.service.impl.ts @@ -11,7 +11,20 @@ export class DockerfileServiceImpl implements DockerfileService { public extractDockerfile(file: any): Promise { return new Promise((resolve, reject) => { - // TODO: Replace callbacks with promises + var reader: FileReader = this.FileReaderFactory(); + reader.onload = (event: any) => { + this.DataFileService.readDataArrayAsPossibleArchive(event.target.result, + (files: any[]) => { + this.processFiles1(files); + }, + () => { + // Not an archive. Read directly as a single file. + this.processFile1(event.target.result); + }); + }; + + reader.onerror = (event: any) => reject(event); + reader.readAsArrayBuffer(file); }); } @@ -48,6 +61,20 @@ export class DockerfileServiceImpl implements DockerfileService { }); } + private processFile1(dataArray: any): Promise { + return new Promise((resolve, reject) => { + this.DataFileService.arrayToString(dataArray, (contents: string) => { + var result: DockerfileInfoImpl | null = DockerfileInfoImpl.forData(contents, this.Config); + if (result == null) { + reject('File chosen is not a valid Dockerfile'); + } + else { + resolve(result); + } + }); + }); + } + private processFiles(files: any[], success: (dockerfile: DockerfileInfoImpl) => void, failure: (error: ErrorEvent | string) => void): void { @@ -71,6 +98,30 @@ export class DockerfileServiceImpl implements DockerfileService { failure('No Dockerfile found in root of archive'); } } + + private processFiles1(files: any[]): Promise { + return new Promise((resolve, reject) => { + var found: boolean = false; + files.forEach((file) => { + if (file['name'] == 'Dockerfile') { + this.DataFileService.blobToString(file.toBlob(), (contents: string) => { + var result: DockerfileInfoImpl | null = DockerfileInfoImpl.forData(contents, this.Config); + if (result == null) { + reject('Dockerfile inside archive is not a valid Dockerfile'); + } + else { + resolve(result); + } + }); + found = true; + } + }); + + if (!found) { + reject('No Dockerfile found in root of archive'); + } + }); + } } diff --git a/static/js/services/dockerfile/dockerfile.service.ts b/static/js/services/dockerfile/dockerfile.service.ts index db62913c2..3af34b0ee 100644 --- a/static/js/services/dockerfile/dockerfile.service.ts +++ b/static/js/services/dockerfile/dockerfile.service.ts @@ -5,8 +5,8 @@ export abstract class DockerfileService { /** - * Retrieve Dockerfile from given archive file. - * @param file File containing Dockerfile. + * Retrieve Dockerfile from given file. + * @param file Dockerfile or archive file containing Dockerfile. * @param success Success callback with retrieved Dockerfile as parameter. * @param failure Failure callback with failure message as parameter. */ @@ -14,6 +14,11 @@ export abstract class DockerfileService { success: (dockerfile: DockerfileInfo) => void, failure: (error: ErrorEvent | string) => void): void; + /** + * Retrieve Dockerfile from given file. + * @param file Dockerfile or archive file containing Dockerfile. + * @return promise Promise resolving to new DockerfileInfo instance or rejecting to error message. + */ public abstract extractDockerfile(file: any): Promise; } diff --git a/webpack.config.js b/webpack.config.js index 788f51445..68f51889c 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -21,7 +21,9 @@ var config = { rules: [ { test: /\.tsx?$/, - loader: "ts-loader", + use: [ + "ts-loader", + ], exclude: /node_modules/ }, { diff --git a/yarn.lock b/yarn.lock index 6a2b4aa68..08b9adf09 100644 --- a/yarn.lock +++ b/yarn.lock @@ -796,7 +796,7 @@ core-js@^1.0.0: version "1.2.7" resolved "https://registry.yarnpkg.com/core-js/-/core-js-1.2.7.tgz#652294c14651db28fa93bd2d5ff2983a4f08c636" -core-js@^2.1.0: +core-js@^2.1.0, core-js@^2.4.1: version "2.4.1" resolved "https://registry.yarnpkg.com/core-js/-/core-js-2.4.1.tgz#4de911e667b0eae9124e34254b53aea6fc618d3e" @@ -3514,14 +3514,14 @@ semver-diff@^2.0.0: dependencies: semver "^5.0.3" -"semver@2 || 3 || 4 || 5", "semver@2.x || 3.x || 4 || 5", semver@~4.3.3: - version "4.3.6" - resolved "https://registry.yarnpkg.com/semver/-/semver-4.3.6.tgz#300bc6e0e86374f7ba61068b5b1ecd57fc6532da" - -semver@^5.0.1, semver@^5.0.3, semver@^5.1.0, semver@~5.3.0: +"semver@2 || 3 || 4 || 5", "semver@2.x || 3.x || 4 || 5", semver@^5.0.1, semver@^5.0.3, semver@^5.1.0, semver@~5.3.0: version "5.3.0" resolved "https://registry.yarnpkg.com/semver/-/semver-5.3.0.tgz#9b2ce5d3de02d17c6012ad326aa6b4d0cf54f94f" +semver@~4.3.3: + version "4.3.6" + resolved "https://registry.yarnpkg.com/semver/-/semver-4.3.6.tgz#300bc6e0e86374f7ba61068b5b1ecd57fc6532da" + semver@~5.0.1: version "5.0.3" resolved "https://registry.yarnpkg.com/semver/-/semver-5.0.3.tgz#77466de589cd5d3c95f138aa78bc569a3cb5d27a"