fix(1749): Use query param dir=true to indicate directory download (#…

…3224)
screwdriver-cd · Oct 15, 2024 · ef31fca · ef31fca
1 parent 4ae5570
commit ef31fca
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 11 deletions.
diff --git a/plugins/builds/README.md b/plugins/builds/README.md
@@ -75,12 +75,11 @@ Example payload:
 Arguments:
 
 * `type` - Return type for build artifact, `download` or `preview`
+* `dir` - If downloading directory or not (`true` or `false`, default `false`). Must be set with `type=download`.
 
 `GET /builds/{id}/artifacts/{name*}?type=preview`
 
-`GET /builds/{id}/artifacts/this/is/a/directory/path/?type=download`
-
-*Note: To download a directory, there must be a trailing slash (`/`) in the name and `type=download`.*
+`GET /builds/{id}/artifacts/this/is/a/directory/path?type=download&dir=true`
 
 #### Get build statuses
 `GET /builds/statuses`

diff --git a/plugins/builds/artifacts/get.js b/plugins/builds/artifacts/get.js
@@ -50,9 +50,9 @@ module.exports = config => ({
                 .then(async () => {
                     // Directory should fetch manifest and
                     // gather all files that belong to that directory
-                    if (artifact.endsWith('/') && req.query.type === 'download') {
+                    if (req.query.dir && req.query.type === 'download') {
                         // Create a zip name from the directory structure
-                        const zipName = artifact.split('/').slice(-2)[0];
+                        const zipName = artifact.split('/').slice(-1)[0];
 
                         try {
                             const token = jwt.sign({
@@ -69,7 +69,7 @@ module.exports = config => ({
                                 method: 'GET'
                             }).text();
                             const manifestArray = manifest.trim().split('\n');
-                            const directoryArray = manifestArray.filter(f => f.startsWith(`./${artifact}`));
+                            const directoryArray = manifestArray.filter(f => f.startsWith(`./${artifact}/`));
 
                             // Create a stream and set up archiver
                             const archive = archiver('zip', { zlib: { level: 9 } });
@@ -100,7 +100,7 @@ module.exports = config => ({
                                         archive.emit('error', err); // Emit error to stop the archive process
                                     });
 
-                                    const relativePath = file.replace(`./${artifact}`, `./${zipName}/`);
+                                    const relativePath = file.replace(`./${artifact}/`, `./${zipName}/`);
 
                                     // Append the file stream to the archive with the correct relative path
                                     archive.append(fileStream, { name: relativePath });
@@ -168,7 +168,8 @@ module.exports = config => ({
                 name: artifactSchema
             }),
             query: joi.object({
-                type: typeSchema
+                type: typeSchema,
+                dir: joi.boolean().truthy('true').falsy('false').default(false)
             }).options({ allowUnknown: true })
         }
     }

diff --git a/test/plugins/builds.test.js b/test/plugins/builds.test.js
@@ -7304,7 +7304,7 @@ describe('build plugin test', () => {
                     'Content-Length': largeFileContent.length
                 });
 
-            options.url = `/builds/${id}/artifacts/./artifacts/?type=download`;
+            options.url = `/builds/${id}/artifacts/./artifacts?type=download&dir=true`;
 
             return server.inject(options).then(reply => {
                 assert.equal(reply.statusCode, 200);
@@ -7327,7 +7327,7 @@ describe('build plugin test', () => {
                 .get('/v1/builds/12345/ARTIFACTS/./artifacts/sample-mp4-file.mp4?token=sign&type=download')
                 .reply(500);
 
-            options.url = `/builds/${id}/artifacts/./artifacts/?type=download`;
+            options.url = `/builds/${id}/artifacts/./artifacts?type=download&dir=true`;
 
             try {
                 await server.inject(options);
@@ -7344,7 +7344,7 @@ describe('build plugin test', () => {
         });
 
         it('returns 500 for a missing manifest for artifact directory', () => {
-            options.url = `/builds/${id}/artifacts/doesnotexist/?type=download`;
+            options.url = `/builds/${id}/artifacts/doesnotexist?type=download&dir=true`;
 
             nock(logBaseUrl)
                 .defaultReplyHeaders(headersMock)