Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: mark bucket.setLabels and associated interfaces as deprecated #2214

Merged
merged 3 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
11 changes: 11 additions & 0 deletions src/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2083,15 +2083,18 @@ class Bucket extends ServiceObject {
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 @@ -2742,20 +2745,24 @@ class Bucket extends ServiceObject {
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 @@ -3530,20 +3537,24 @@ class Bucket extends ServiceObject {
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.
Copy link
Member

@frankyn frankyn Jun 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the expectation to update setLabels samples as well in a follow-up PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good catch, I will update that in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated addBucketLabel sample to use setMetadata.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Denis!

* Set labels on the bucket.
*
* This makes an underlying call to {@link Bucket#setMetadata}, which
Expand Down
52 changes: 36 additions & 16 deletions system-test/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,7 @@ describe('storage', () => {
});

it('should be available from updating a bucket', async () => {
await bucket.setLabels({a: 'b'});
await bucket.setMetadata({labels: {a: 'b'}});
assert(types.includes(bucket.metadata.locationType));
});
});
Expand All @@ -1120,23 +1120,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 @@ -1145,19 +1155,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