Skip to content

Commit 8bce92f

Browse files
authored
Fix file upload bug (fixes #56) (#57)
* Fix file upload bug (fixes #56) * Promisified the upload controller * test added * python runner in repository deployment fixed
1 parent 65063fa commit 8bce92f

File tree

5 files changed

+80
-36
lines changed

5 files changed

+80
-36
lines changed

src/app.ts

+2
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,5 @@ export class Application {
2121
}
2222

2323
export const Applications: Record<string, Application> = {};
24+
25+
export const unzipPromises: Record<string, Promise<void>> = {};

src/controller/deploy.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { hostname } from 'os';
33

44
import AppError from '../utils/appError';
55

6-
import { Applications } from '../app';
6+
import { Applications, unzipPromises } from '../app';
77
import { deployProcess } from '../utils/deploy';
88
import { installDependencies } from '../utils/install';
99
import { catchAsync } from './catch';
@@ -25,6 +25,8 @@ export const deploy = catchAsync(
2525
next: NextFunction
2626
) => {
2727
try {
28+
await unzipPromises[req.body.suffix];
29+
2830
const application = Applications[req.body.suffix];
2931

3032
// Check if the application exists and it is stored
@@ -37,6 +39,8 @@ export const deploy = catchAsync(
3739

3840
const resource = await application.resource;
3941

42+
// Wait for the unzipping to complete
43+
4044
await installDependencies(resource);
4145

4246
await deployProcess(resource);

src/controller/repository.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ const handleRunners = async (repoPath: string): Promise<string[]> => {
4444
if (file === 'package.json') runners.push('nodejs');
4545

4646
if (stat.isDirectory()) {
47-
await handleRunners(fullPath);
47+
const subRunners = await handleRunners(fullPath);
48+
runners.push(...subRunners);
4849
}
4950
}
5051
return runners;

src/controller/upload.ts

+32-22
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import path from 'path';
44

55
import busboy from 'busboy';
66
import { NextFunction, Request, Response } from 'express';
7-
import { Extract, ParseOptions } from 'unzipper';
7+
import { Extract } from 'unzipper';
88

99
import { MetaCallJSON } from '@metacall/protocol/deployment';
1010
import { Application, Applications, Resource } from '../app';
@@ -56,10 +56,10 @@ export const uploadPackage = (
5656
jsons: []
5757
};
5858

59-
let handled = false; // TODO: Promisify the whole controller
59+
let handled = false;
6060

6161
const errorHandler = (error: AppError) => {
62-
if (handled == false) {
62+
if (handled === false) {
6363
req.unpipe(bb);
6464
next(error);
6565
}
@@ -88,8 +88,8 @@ export const uploadPackage = (
8888
const { mimeType, filename } = info;
8989

9090
if (
91-
mimeType != 'application/x-zip-compressed' &&
92-
mimeType != 'application/zip'
91+
mimeType !== 'application/x-zip-compressed' &&
92+
mimeType !== 'application/zip'
9393
) {
9494
return errorHandler(
9595
new AppError('Please upload a zip file', 404)
@@ -147,7 +147,7 @@ export const uploadPackage = (
147147

148148
eventHandler('finish', () => {
149149
if (resource.blob === undefined) {
150-
throw Error('Invalid file upload, blob path is not defined');
150+
throw new Error('Invalid file upload, blob path is not defined');
151151
}
152152

153153
const deleteBlob = () => {
@@ -201,33 +201,43 @@ export const uploadPackage = (
201201
}
202202
);
203203

204-
const options: ParseOptions = { path: resource.path };
204+
const unzipAndResolve = () => {
205+
return new Promise<void>((resolve, reject) => {
206+
fs.createReadStream(resource.blob as string)
207+
.pipe(Extract({ path: resource.path }))
208+
.on('close', () => {
209+
deleteBlob();
210+
resolve();
211+
})
212+
.on('error', error => {
213+
deleteBlob();
214+
deleteFolder();
215+
reject(
216+
new AppError(
217+
`Failed to unzip the resource at: ${error.toString()}`,
218+
500
219+
)
220+
);
221+
});
222+
});
223+
};
205224

206-
fs.createReadStream(resource.blob)
207-
.pipe(Extract(options))
208-
.on('close', () => {
209-
deleteBlob();
225+
unzipAndResolve()
226+
.then(() => {
210227
resourceResolve(resource);
211228
})
212-
.on('error', error => {
213-
deleteBlob();
214-
deleteFolder();
215-
const appError = new AppError(
216-
`Failed to unzip the resource at: ${error.toString()}`,
217-
500
218-
);
219-
errorHandler(appError);
220-
resourceReject(appError);
229+
.catch(error => {
230+
resourceReject(error);
231+
errorHandler(error);
221232
});
222233
});
223234

224235
eventHandler('close', () => {
225-
if (handled == false) {
236+
if (handled === false) {
226237
res.status(201).json({
227238
id: resource.id
228239
});
229240
}
230-
231241
handled = true;
232242
});
233243

test/test.sh

+39-12
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,6 @@ function test_nodejs_dependency_app() {
185185
[[ $sum_response = 3 ]] || exit 1
186186
}
187187

188-
# Run tests
189-
run_tests "nodejs-base-app" test_nodejs_app
190-
run_tests "python-base-app" test_python_base_app
191-
run_tests "python-dependency-app" test_python_dependency_app
192-
run_tests "nodejs-dependency-app" test_nodejs_dependency_app
193188

194189
echo "Integration tests for package deployment passed without errors."
195190

@@ -308,20 +303,52 @@ function test_nodejs_dependency_app() {
308303
echo "Node.js Dependency App tests passed"
309304
}
310305

311-
echo "starting repository deployment tests"
306+
# without Dependencies
312307

313-
#without Dependencies
314-
315-
#Test NodeJs app
308+
# Test NodeJs app
316309
test_deploy_from_repo "https://github.com/HeeManSu/nodejs-parameter-example" "nodejs-parameter-example" test_nodejs-parameter-example
317-
# #Test Python app
310+
# Test Python app
318311
test_deploy_from_repo "https://github.com/HeeManSu/metacall-python-example" "metacall-python-example" test_python_time_app
319312

320-
# #With Dependencies
313+
# With Dependencies
321314

322-
# # Test Python app
315+
# Test Python app
323316
test_deploy_from_repo "https://github.com/HeeManSu/python-dependency-metacall" "python-dependency-metacall" test_python_dependency_metacall
324317
#Test NodeJs app
325318
test_deploy_from_repo "https://github.com/HeeManSu/auth-middleware-metacall" "auth-middleware-metacall" test_nodejs_dependency_app
326319

327320
echo "Repository deployment tests completed."
321+
322+
323+
# Simultaneous deployment tests
324+
function test_simultaneous_deploy() {
325+
echo "Testing simultaneous deployments..."
326+
pids=()
327+
328+
# Run all tests simultaneously in background
329+
run_tests "nodejs-base-app" test_nodejs_app &
330+
pids+=($!)
331+
332+
run_tests "python-base-app" test_python_base_app &
333+
pids+=($!)
334+
335+
run_tests "python-dependency-app" test_python_dependency_app &
336+
pids+=($!)
337+
338+
run_tests "nodejs-dependency-app" test_nodejs_dependency_app &
339+
pids+=($!)
340+
341+
for pid in "${pids[@]}"; do
342+
if ! wait $pid; then
343+
echo "Simultaneous deployment test failed - Test failed"
344+
return 1
345+
exit 1
346+
fi
347+
done
348+
349+
echo "Simultaneous deployment test passed - All tests passed"
350+
return 0
351+
}
352+
353+
echo "Testing simultaneous deployments..."
354+
test_simultaneous_deploy

0 commit comments

Comments
 (0)