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

fix!: Remove extraneous validation in favor of server errors #2237

Merged
merged 2 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
9 changes: 0 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 @@ -413,7 +412,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 @@ -1714,10 +1712,6 @@ class Bucket extends ServiceObject {
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 @@ -3041,9 +3035,6 @@ class Bucket extends ServiceObject {
callback?: GetSignedUrlCallback
): void | Promise<GetSignedUrlResponse> {
const method = BucketActionToHTTPMethod[cfg.action];
if (!method) {
throw new Error(ExceptionMessages.INVALID_ACTION);
}

const signConfig = {
method,
Expand Down
3 changes: 0 additions & 3 deletions src/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2827,9 +2827,6 @@ class File extends ServiceObject<File> {
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
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
39 changes: 1 addition & 38 deletions test/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,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 @@ -920,12 +920,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 @@ -2130,37 +2124,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 @@ -3549,35 +3549,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