bug: fix parent detection for single files (#269)

Author: sethvargo

dist/index.js

@@ -22,10 +22,11 @@ import {
   randomFilepath,
   inParallel,
   toPlatformPath,
+  toPosixPath,
 } from '@google-github-actions/actions-utils';
 
 import { Metadata } from './headers';
-import { deepClone, parseBucketNameAndPrefix } from './util';
+import { deepClone } from './util';
 
 // Do not listen to the linter - this can NOT be rewritten as an ES6 import statement.
 // eslint-disable-next-line @typescript-eslint/no-var-requires
@@ -39,47 +40,49 @@ const userAgent = `google-github-actions:upload-cloud-storage/${appVersion}`;
  *
  * @param credentials GCP JSON credentials (default uses ADC).
  */
-type ClientOptions = {
+export type ClientOptions = {
   credentials?: string;
   projectID?: string;
 };
 
 /**
- * ClientUploadOptions is the list of available options during file upload.
+ * ClientFileUpload represents a file to upload. It keeps track of the local
+ * source path and remote destination.
  */
-export interface ClientUploadOptions {
+export type ClientFileUpload = {
   /**
-   * destination is the name of the bucket and optionally the path within the
-   * bucket in which to upload. This value is split on the first instance of a
-   * slash character. Everything preceeding of the first slash is the bucket
-   * name, everything following the first slash is the path.
+   * source is the absolute, local path on disk to the file.
+   */
+  source: string;
+
+  /**
+   * destination is the remote location for the file, relative to the bucket
+   * root.
    */
   destination: string;
+};
 
+/**
+ * ClientUploadOptions is the list of available options during file upload.
+ */
+export interface ClientUploadOptions {
   /**
-   * root is the parent directory from which all files originated on local disk.
-   * This must be the platform-specific path separators.
+   * bucket is the name of the bucket in which to upload.
    */
-  root: string;
+  bucket: string;
 
   /**
    * files is the list of absolute file paths on local disk to upload. This list
    * must use posix path separators for files.
    */
-  files: string[];
+  files: ClientFileUpload[];
 
   /**
    * concurrency is the maximum number of parallel upload operations that will
    * take place.
    */
   concurrency?: number;
 
-  /**
-   * includeParent indicates whether the local directory parent name (dirname of
-   * root) should be included in the destination path in the bucket.
-   */
-  includeParent?: boolean;
-
   /**
    * metadata is object metadata to set. These are usually populated from
    * headers.
@@ -110,10 +113,43 @@ export interface ClientUploadOptions {
 /**
  * FOnUploadObject is the function interface for the upload callback signature.
  */
-interface FOnUploadObject {
+export interface FOnUploadObject {
   (source: string, destination: string, opts: Record<string, unknown>): void;
 }
 
+/**
+ * ClientComputeDestinationOptions is the list of options to compute file
+ * destinations in a target bucket.
+ */
+export interface ClientComputeDestinationOptions {
+  /**
+   * givenRoot is the root given by the input to the function.
+   */
+  givenRoot: string;
+
+  /**
+   * absoluteRoot is the absolute root path, used for resolving the files.
+   */
+  absoluteRoot: string;
+
+  /**
+   * files is a list of filenames, for a glob expansion. All files are relative
+   * to absoluteRoot.
+   */
+  files: string[];
+
+  /**
+   * prefix is an optional prefix to predicate on all paths.
+   */
+  prefix?: string;
+
+  /**
+   * includeParent indicates whether the local directory parent name (dirname of
+   * givenRoot) should be included in the destination path in the bucket.
+   */
+  includeParent?: boolean;
+}
+
 /**
  * Handles credential lookup, registration and wraps interactions with the GCS
  * Helper.
@@ -136,6 +172,37 @@ export class Client {
     this.storage = new Storage(options);
   }
 
+  /**
+   * computeDestinations builds a collection of files to their intended upload
+   * paths in a Cloud Storage bucket, based on the given options.
+   *
+   * @param opts List of inputs and files to compute.
+   * @return List of files to upload with the source as a local file path and
+   * the remote destination path.
+   */
+  static computeDestinations(opts: ClientComputeDestinationOptions): ClientFileUpload[] {
+    const list: ClientFileUpload[] = [];
+    for (let i = 0; i < opts.files.length; i++) {
+      const name = opts.files[i];
+
+      // Calculate destination by joining the prefix (if one exists), the parent
+      // directory name (if includeParent is true), and the file name. path.join
+      // ignores empty strings. We only want to do this if
+      const base = opts.includeParent ? path.posix.basename(toPosixPath(opts.givenRoot)) : '';
+      const destination = path.posix.join(opts.prefix || '', base, name);
+
+      // Compute the absolute path of the file.
+      const source = path.resolve(opts.absoluteRoot, toPlatformPath(name));
+
+      list.push({
+        source: source,
+        destination: destination,
+      });
+    }
+
+    return list;
+  }
+
   /**
    * upload puts the given collection of files into the bucket. It will
    * overwrite any existing objects with the same name and create any new
@@ -146,19 +213,12 @@ export class Client {
    * @return The list of files uploaded.
    */
   async upload(opts: ClientUploadOptions): Promise<string[]> {
-    const [bucket, prefix] = parseBucketNameAndPrefix(opts.destination);
-
+    const bucket = opts.bucket;
     const storageBucket = this.storage.bucket(bucket);
 
-    const uploadOne = async (file: string): Promise<string> => {
-      // Calculate destination by joining the prefix (if one exists), the parent
-      // directory name (if includeParent is true), and the file name. path.join
-      // ignores empty strings.
-      const base = opts.includeParent ? path.basename(opts.root) : '';
-      const destination = path.posix.join(prefix, base, file);
-
-      // Build options.
-      const abs = path.resolve(opts.root, toPlatformPath(file));
+    const uploadOne = async (file: ClientFileUpload): Promise<string> => {
+      const source = file.source;
+      const destination = file.destination;
 
       // Apparently the Cloud Storage SDK modifies this object, so we need to
       // make our own deep copy before passing it to upload. See #258 for more
@@ -174,16 +234,16 @@ export class Client {
 
       // Execute callback if defined
       if (opts.onUploadObject) {
-        opts.onUploadObject(abs, path.posix.join(bucket, destination), uploadOpts);
+        opts.onUploadObject(source, path.posix.join(bucket, destination), uploadOpts);
       }
 
       // Do the upload
-      const response = await storageBucket.upload(abs, uploadOpts);
+      const response = await storageBucket.upload(source, uploadOpts);
       const name = response[0].name;
       return name;
     };
 
-    const args: [file: string][] = opts.files.map((file) => [file]);
+    const args: [file: ClientFileUpload][] = opts.files.map((file) => [file]);
     const results = await inParallel(uploadOne, args, {
       concurrency: opts.concurrency,
     });

src/client.ts

@@ -28,7 +28,12 @@ import * as path from 'path';
 
 import { Client } from './client';
 import { parseHeadersInput } from './headers';
-import { absoluteRootAndComputedGlob, deepClone, expandGlob } from './util';
+import {
+  absoluteRootAndComputedGlob,
+  deepClone,
+  parseBucketNameAndPrefix,
+  expandGlob,
+} from './util';
 
 const NO_FILES_WARNING =
   `There are no files to upload! Make sure the workflow uses the "checkout"` +
@@ -53,7 +58,7 @@ export async function run(): Promise<void> {
     const destination = core.getInput('destination', { required: true });
     const gzip = core.getBooleanInput('gzip');
     const resumable = core.getBooleanInput('resumable');
-    const parent = core.getBooleanInput('parent');
+    const includeParent = core.getBooleanInput('parent');
     const glob = core.getInput('glob');
     const concurrency = Number(core.getInput('concurrency'));
     const predefinedAclInput = core.getInput('predefinedAcl');
@@ -75,8 +80,8 @@ export async function run(): Promise<void> {
     }
 
     // Compute the absolute root and compute the glob.
-    const [absoluteRoot, computedGlob] = await absoluteRootAndComputedGlob(root, glob);
-    core.debug(`Computed absoluteRoot from "${root}" to "${absoluteRoot}"`);
+    const [absoluteRoot, computedGlob, rootIsDir] = await absoluteRootAndComputedGlob(root, glob);
+    core.debug(`Computed absoluteRoot from "${root}" to "${absoluteRoot}" (isDir: ${rootIsDir})`);
     core.debug(`Computed computedGlob from "${glob}" to "${computedGlob}"`);
 
     // Build complete file list.
@@ -128,18 +133,35 @@ export async function run(): Promise<void> {
       core.warning(NO_FILES_WARNING);
     }
 
+    // Compute the bucket and prefix.
+    const [bucket, prefix] = parseBucketNameAndPrefix(destination);
+    core.debug(`Computed bucket as "${bucket}"`);
+    core.debug(`Computed prefix as "${prefix}"`);
+
+    // Compute the list of file destinations in the bucket based on given
+    // parameters.
+    const destinations = Client.computeDestinations({
+      givenRoot: root,
+      absoluteRoot: absoluteRoot,
+      files: files,
+      prefix: prefix,
+
+      // Only include the parent if the given root was a directory. Without
+      // this, uploading a single object will cause the object to be nested in
+      // its own name: google-github-actions/upload-cloud-storage#259.
+      includeParent: includeParent && rootIsDir,
+    });
+
     // Create the client and upload files.
     core.startGroup('Upload files');
     const client = new Client({
       credentials: credentials,
       projectID: projectID,
     });
     const uploadResponses = await client.upload({
-      destination: destination,
-      root: absoluteRoot,
-      files: files,
+      bucket: bucket,
+      files: destinations,
       concurrency: concurrency,
-      includeParent: parent,
       metadata: metadata,
       gzip: gzip,
       resumable: resumable,

src/main.ts

@@ -33,12 +33,14 @@ import { toPlatformPath, toPosixPath } from '@google-github-actions/actions-util
  *
  * @param root The root path to expand.
  * @param glob The glob to compute.
- * @return [string, string] The absolute and expanded root and computed glob.
+ * @return [string, string, boolean] The absolute and expanded root, the
+ * computed glob, and a boolean indicating whether the given root was a
+ * directory.
  */
 export async function absoluteRootAndComputedGlob(
   root: string,
   glob: string,
-): Promise<[absoluteRoot: string, computedGlob: string]> {
+): Promise<[absoluteRoot: string, computedGlob: string, isFile: boolean]> {
   // Resolve the root input path, relative to the active workspace. If the
   // value was already an absolute path, this has no effect.
   const githubWorkspace = process.env.GITHUB_WORKSPACE;
@@ -57,10 +59,10 @@ export async function absoluteRootAndComputedGlob(
 
     const computedGlob = path.basename(resolvedRoot);
     const absoluteRoot = path.dirname(resolvedRoot);
-    return [absoluteRoot, toPosixPath(computedGlob)];
+    return [absoluteRoot, toPosixPath(computedGlob), false];
   }
 
-  return [resolvedRoot, toPosixPath(glob)];
+  return [resolvedRoot, toPosixPath(glob), true];
 }
 
 /**