refactoring to promises

This commit is contained in:
alecmerdler 2017-03-08 11:43:53 -08:00
parent 390e389027
commit 85441c8459
9 changed files with 191 additions and 28 deletions

View file

@ -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() {

View file

@ -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";

View file

@ -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);
});
});

View file

@ -11,7 +11,20 @@ export class DockerfileServiceImpl implements DockerfileService {
public extractDockerfile(file: any): Promise<DockerfileInfoImpl | string> {
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<DockerfileInfoImpl | string> {
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<DockerfileInfoImpl | string> {
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');
}
});
}
}

View file

@ -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<DockerfileInfo | string>;
}