Skip to content

Commit

Permalink
Merge branch 'storage-seven' into metadata-typing
Browse files Browse the repository at this point in the history
  • Loading branch information
ddelgrosso1 authored Jul 24, 2023
2 parents 3026b74 + 816df20 commit acae384
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 102 deletions.
2 changes: 1 addition & 1 deletion samples/addBucketLabel.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function main(
};

async function addBucketLabel() {
await storage.bucket(bucketName).setLabels(labels);
await storage.bucket(bucketName).setMetadata({labels});
console.log(`Added label to bucket ${bucketName}`);
}

Expand Down
7 changes: 6 additions & 1 deletion samples/removeBucketLabel.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ function main(bucketName = 'my-bucket', labelKey = ['label1', 'label2']) {
const storage = new Storage();

async function removeBucketLabel() {
await storage.bucket(bucketName).deleteLabels(labelKey);
const labels = {};
// To remove a label set the value of the key to null.
for (const curLabel of labelKey) {
labels[curLabel] = null;
}
await storage.bucket(bucketName).setMetadata({labels});
console.log(`Removed labels from bucket ${bucketName}`);
}

Expand Down
20 changes: 11 additions & 9 deletions src/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ import {
PreconditionOptions,
IdempotencyStrategy,
BucketOptions,
ExceptionMessages,
} from './storage';
import {
GetSignedUrlResponse,
Expand Down Expand Up @@ -493,7 +492,6 @@ export enum BucketExceptionMessages {
PROVIDE_SOURCE_FILE = 'You must provide at least one source file.',
DESTINATION_FILE_NOT_SPECIFIED = 'A destination file must be specified.',
CHANNEL_ID_REQUIRED = 'An ID is required to create a channel.',
CHANNEL_ADDRESS_REQUIRED = 'An address is required to create a channel.',
TOPIC_NAME_REQUIRED = 'A valid topic name is required.',
CONFIGURATION_OBJECT_PREFIX_REQUIRED = 'A configuration object with a prefix is required.',
SPECIFY_FILE_NAME = 'A file name must be specified.',
Expand Down Expand Up @@ -1791,10 +1789,6 @@ class Bucket extends ServiceObject<Bucket, BucketMetadata> {
throw new Error(BucketExceptionMessages.CHANNEL_ID_REQUIRED);
}

if (typeof config.address !== 'string') {
throw new Error(BucketExceptionMessages.CHANNEL_ADDRESS_REQUIRED);
}

let options: CreateChannelOptions = {};
if (typeof optionsOrCallback === 'function') {
callback = optionsOrCallback;
Expand Down Expand Up @@ -2158,15 +2152,18 @@ class Bucket extends ServiceObject<Bucket, BucketMetadata> {
callback: DeleteLabelsCallback
): void;
/**
* @deprecated
* @typedef {array} DeleteLabelsResponse
* @property {object} 0 The full API response.
*/
/**
* @deprecated
* @callback DeleteLabelsCallback
* @param {?Error} err Request error, if any.
* @param {object} metadata Bucket's metadata.
*/
/**
* @deprecated Use setMetadata directly
* Delete one or more labels from this bucket.
*
* @param {string|string[]} [labels] The labels to delete. If no labels are
Expand Down Expand Up @@ -2820,20 +2817,24 @@ class Bucket extends ServiceObject<Bucket, BucketMetadata> {
getLabels(callback: GetLabelsCallback): void;
getLabels(options: GetLabelsOptions, callback: GetLabelsCallback): void;
/**
* @deprecated
* @typedef {object} GetLabelsOptions Configuration options for Bucket#getLabels().
* @param {string} [userProject] The ID of the project which will be
* billed for the request.
*/
/**
* @deprecated
* @typedef {array} GetLabelsResponse
* @property {object} 0 Object of labels currently set on this bucket.
*/
/**
* @deprecated
* @callback GetLabelsCallback
* @param {?Error} err Request error, if any.
* @param {object} labels Object of labels currently set on this bucket.
*/
/**
* @deprecated Use getMetadata directly.
* Get the labels currently set on this bucket.
*
* @param {object} [options] Configuration options.
Expand Down Expand Up @@ -3123,9 +3124,6 @@ class Bucket extends ServiceObject<Bucket, BucketMetadata> {
callback?: GetSignedUrlCallback
): void | Promise<GetSignedUrlResponse> {
const method = BucketActionToHTTPMethod[cfg.action];
if (!method) {
throw new Error(ExceptionMessages.INVALID_ACTION);
}

const signConfig = {
method,
Expand Down Expand Up @@ -3610,20 +3608,24 @@ class Bucket extends ServiceObject<Bucket, BucketMetadata> {
callback: SetLabelsCallback
): void;
/**
* @deprecated
* @typedef {array} SetLabelsResponse
* @property {object} 0 The bucket metadata.
*/
/**
* @deprecated
* @callback SetLabelsCallback
* @param {?Error} err Request error, if any.
* @param {object} metadata The bucket metadata.
*/
/**
* @deprecated
* @typedef {object} SetLabelsOptions Configuration options for Bucket#setLabels().
* @property {string} [userProject] The ID of the project which will be
* billed for the request.
*/
/**
* @deprecated Use setMetadata directly.
* Set labels on the bucket.
*
* This makes an underlying call to {@link Bucket#setMetadata}, which
Expand Down
3 changes: 0 additions & 3 deletions src/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2866,9 +2866,6 @@ class File extends ServiceObject<File, FileMetadata> {
callback?: GetSignedUrlCallback
): void | Promise<GetSignedUrlResponse> {
const method = ActionToHTTPMethod[cfg.action];
if (!method) {
throw new Error(ExceptionMessages.INVALID_ACTION);
}
const extensionHeaders = objectKeyToLowercase(cfg.extensionHeaders || {});
if (cfg.action === 'resumable') {
extensionHeaders['x-goog-resumable'] = 'start';
Expand Down
5 changes: 2 additions & 3 deletions src/nodejs-common/service-object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,14 +310,13 @@ class ServiceObject<T, K extends BaseMetadata> extends EventEmitter {
ServiceObject.prototype.request.call(
this,
reqOpts,
(err: ApiError | null, ...args) => {
(err: ApiError | null, body?: ResponseBody, res?: r.Response) => {
if (err) {
if (err.code === 404 && ignoreNotFound) {
err = null;
}
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
callback(err, ...(args as any));
callback(err, res);
}
);
}
Expand Down
1 change: 0 additions & 1 deletion src/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ export interface GetHmacKeysCallback {
export enum ExceptionMessages {
EXPIRATION_DATE_INVALID = 'The expiration date provided was invalid.',
EXPIRATION_DATE_PAST = 'An expiration date cannot be in the past.',
INVALID_ACTION = 'The action is not provided or invalid.',
}

export enum StorageExceptionMessages {
Expand Down
54 changes: 37 additions & 17 deletions system-test/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1107,8 +1107,8 @@ describe('storage', () => {
});

it('should be available from updating a bucket', async () => {
await bucket.setLabels({a: 'b'});
assert(types.includes(bucket.metadata.locationType!));
await bucket.setMetadata({labels: {a: 'b'}});
assert(types.includes(bucket.metadata.locationType));
});
});

Expand All @@ -1119,23 +1119,33 @@ describe('storage', () => {
};

beforeEach(async () => {
await bucket.deleteLabels();
const [metadata] = await bucket.getMetadata();
const labels: {[index: string]: string | null} = {};
if (metadata.labels) {
for (const curLabel of Object.keys(metadata.labels)) {
labels[curLabel] = null;
}
await bucket.setMetadata({labels});
}
});

it('should set labels', async () => {
await bucket.setLabels(LABELS);
const [labels] = await bucket.getLabels();
assert.deepStrictEqual(labels, LABELS);
await bucket.setMetadata({labels: LABELS});
const [metadata] = await bucket.getMetadata();
assert.deepStrictEqual(metadata.labels, LABELS);
});

it('should update labels', async () => {
const newLabels = {
siblinglabel: 'labelvalue',
};
await bucket.setLabels(LABELS);
await bucket.setLabels(newLabels);
const [labels] = await bucket.getLabels();
assert.deepStrictEqual(labels, Object.assign({}, LABELS, newLabels));
await bucket.setMetadata({labels: LABELS});
await bucket.setMetadata({labels: newLabels});
const [metadata] = await bucket.getMetadata();
assert.deepStrictEqual(
metadata.labels,
Object.assign({}, LABELS, newLabels)
);
});

it('should delete a single label', async () => {
Expand All @@ -1144,19 +1154,29 @@ describe('storage', () => {
}

const labelKeyToDelete = Object.keys(LABELS)[0];
await bucket.setLabels(LABELS);
await bucket.deleteLabels(labelKeyToDelete);
const [labels] = await bucket.getLabels();
await bucket.setMetadata({labels: LABELS});
const labelsToDelete = {
[labelKeyToDelete]: null,
};
await bucket.setMetadata({labels: labelsToDelete});
const [metadata] = await bucket.getMetadata();
const expectedLabels = Object.assign({}, LABELS);
delete (expectedLabels as {[index: string]: {}})[labelKeyToDelete];

assert.deepStrictEqual(labels, expectedLabels);
assert.deepStrictEqual(metadata.labels, expectedLabels);
});

it('should delete all labels', async () => {
await bucket.deleteLabels();
const [labels] = await bucket.getLabels();
assert.deepStrictEqual(labels, {});
let [metadata] = await bucket.getMetadata();
if (metadata.labels) {
const labels: {[index: string]: string | null} = {};
for (const curLabel of Object.keys(metadata.labels)) {
labels[curLabel] = null;
}
await bucket.setMetadata({labels});
}
[metadata] = await bucket.getMetadata();
assert.deepStrictEqual(metadata.labels, undefined);
});
});
});
Expand Down
39 changes: 1 addition & 38 deletions test/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import {AddAclOptions} from '../src/acl';
import {Policy} from '../src/iam';
import sinon = require('sinon');
import {Transform} from 'stream';
import {ExceptionMessages, IdempotencyStrategy} from '../src/storage';
import {IdempotencyStrategy} from '../src/storage';
import {convertObjKeysToSnakeCase} from '../src/util';

class FakeFile {
Expand Down Expand Up @@ -923,12 +923,6 @@ describe('Bucket', () => {
});
});

it('should throw if an address is not provided', () => {
assert.throws(() => {
bucket.createChannel(ID, {});
}, /An address is required to create a channel\./);
});

it('should make the correct request', done => {
const config = Object.assign({}, CONFIG, {
a: 'b',
Expand Down Expand Up @@ -2136,37 +2130,6 @@ describe('Bucket', () => {
}
);
});

it('should error if action is null', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
SIGNED_URL_CONFIG.action = null as any;

assert.throws(() => {
bucket.getSignedUrl(SIGNED_URL_CONFIG, () => {}),
ExceptionMessages.INVALID_ACTION;
});
});

it('should error if action is undefined', () => {
const urlConfig = {
...SIGNED_URL_CONFIG,
} as Partial<GetBucketSignedUrlConfig>;
delete urlConfig.action;
assert.throws(() => {
bucket.getSignedUrl(urlConfig, () => {}),
ExceptionMessages.INVALID_ACTION;
});
});

it('should error for an invalid action', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
SIGNED_URL_CONFIG.action = 'watch' as any;

assert.throws(() => {
bucket.getSignedUrl(SIGNED_URL_CONFIG, () => {}),
ExceptionMessages.INVALID_ACTION;
});
});
});

describe('lock', () => {
Expand Down
29 changes: 0 additions & 29 deletions test/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3552,35 +3552,6 @@ describe('File', () => {
});
});

it('should error if action is null', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
SIGNED_URL_CONFIG.action = null as any;

assert.throws(() => {
file.getSignedUrl(SIGNED_URL_CONFIG, () => {}),
ExceptionMessages.INVALID_ACTION;
});
});

it('should error if action is undefined', () => {
const urlConfig = {...SIGNED_URL_CONFIG} as Partial<GetSignedUrlConfig>;
delete urlConfig.action;
assert.throws(() => {
file.getSignedUrl(urlConfig, () => {}),
ExceptionMessages.INVALID_ACTION;
});
});

it('should error for an invalid action', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
SIGNED_URL_CONFIG.action = 'watch' as any;

assert.throws(() => {
file.getSignedUrl(SIGNED_URL_CONFIG, () => {}),
ExceptionMessages.INVALID_ACTION;
});
});

it('should add "x-goog-resumable: start" header if action is resumable', done => {
SIGNED_URL_CONFIG.action = 'resumable';
SIGNED_URL_CONFIG.extensionHeaders = {
Expand Down

0 comments on commit acae384

Please sign in to comment.