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!: Remove extend and Treat Provided Options as Mutable #2204

Merged
merged 8 commits into from
Jun 13, 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: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
"compressible": "^2.0.12",
"duplexify": "^4.0.0",
"ent": "^2.2.0",
"extend": "^3.0.2",
"gaxios": "^5.0.0",
"google-auth-library": "^8.0.1",
"mime": "^3.0.0",
Expand All @@ -75,7 +74,6 @@
"@types/async-retry": "^1.4.3",
"@types/compressible": "^2.0.0",
"@types/ent": "^2.2.1",
"@types/extend": "^3.0.0",
"@types/mime": "^3.0.0",
"@types/mime-types": "^2.1.0",
"@types/mocha": "^9.1.1",
Expand Down
9 changes: 4 additions & 5 deletions src/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
} from './nodejs-common';
import {paginator} from '@google-cloud/paginator';
import {promisifyAll} from '@google-cloud/promisify';
import * as extend from 'extend';
import * as fs from 'fs';
import * as http from 'http';
import * as mime from 'mime-types';
Expand Down Expand Up @@ -3270,7 +3269,7 @@ class Bucket extends ServiceObject {

// You aren't allowed to set both predefinedAcl & acl properties on a bucket
// so acl must explicitly be nullified.
const metadata = extend({}, options.metadata, {acl: null});
const metadata = {...options.metadata, acl: null};

this.setMetadata(metadata, query, err => {
if (err) {
Expand Down Expand Up @@ -3400,7 +3399,7 @@ class Bucket extends ServiceObject {
callback =
typeof optionsOrCallback === 'function' ? optionsOrCallback : callback;

const req = extend(true, {public: true}, options);
const req = {public: true, ...options};

this.acl
.add({
Expand Down Expand Up @@ -3514,7 +3513,7 @@ class Bucket extends ServiceObject {
callback?: BodyResponseCallback
): void | Promise<[ResponseBody, Metadata]> {
if (this.userProject && (!reqOpts.qs || !reqOpts.qs.userProject)) {
reqOpts.qs = extend(reqOpts.qs, {userProject: this.userProject});
reqOpts.qs = {...reqOpts.qs, userProject: this.userProject};
}
return super.request(reqOpts, callback!);
}
Expand Down Expand Up @@ -3893,7 +3892,7 @@ class Bucket extends ServiceObject {
const methodConfig = this.methods[method];
if (typeof methodConfig === 'object') {
if (typeof methodConfig.reqOpts === 'object') {
extend(methodConfig.reqOpts.qs, {userProject});
Object.assign(methodConfig.reqOpts.qs, {userProject});
} else {
methodConfig.reqOpts = {
qs: {userProject},
Expand Down
59 changes: 21 additions & 38 deletions src/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {promisifyAll} from '@google-cloud/promisify';

import compressible = require('compressible');
import * as crypto from 'crypto';
import * as extend from 'extend';
import * as fs from 'fs';
import * as mime from 'mime';
import * as resumableUpload from './resumable-upload';
Expand Down Expand Up @@ -387,10 +386,6 @@ export interface SetStorageClassOptions {
preconditionOpts?: PreconditionOptions;
}

interface SetStorageClassRequest extends SetStorageClassOptions {
storageClass?: string;
}

export interface SetStorageClassCallback {
(err?: Error | null, apiResponse?: Metadata): void;
}
Expand Down Expand Up @@ -1158,10 +1153,9 @@ class File extends ServiceObject<File> {
if (typeof optionsOrCallback === 'function') {
callback = optionsOrCallback;
} else if (optionsOrCallback) {
options = optionsOrCallback;
options = {...optionsOrCallback};
}

options = extend(true, {}, options);
callback = callback || util.noop;

let destBucket: Bucket;
Expand Down Expand Up @@ -1848,7 +1842,7 @@ class File extends ServiceObject<File> {
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
createWriteStream(options: CreateWriteStreamOptions = {}): Writable {
options = extend(true, {metadata: {}}, options);
options.metadata ??= {};

if (options.contentType) {
options.metadata.contentType = options.contentType;
Expand Down Expand Up @@ -3069,7 +3063,7 @@ class File extends ServiceObject<File> {
// You aren't allowed to set both predefinedAcl & acl properties on a file,
// so acl must explicitly be nullified, destroying all previous acls on the
// file.
const metadata = extend({}, options.metadata, {acl: null});
const metadata = {...options.metadata, acl: null};

this.setMetadata(metadata, query, callback!);
}
Expand Down Expand Up @@ -3757,19 +3751,17 @@ class File extends ServiceObject<File> {
typeof optionsOrCallback === 'function' ? optionsOrCallback : callback;
const options =
typeof optionsOrCallback === 'object' ? optionsOrCallback : {};
const req = extend<SetStorageClassRequest, SetStorageClassOptions>(
true,
{},
options
);

// In case we get input like `storageClass`, convert to `storage_class`.
req.storageClass = storageClass
.replace(/-/g, '_')
.replace(/([a-z])([A-Z])/g, (_, low, up) => {
return low + '_' + up;
})
.toUpperCase();
const req = {
...options,
// In case we get input like `storageClass`, convert to `storage_class`.
storageClass: storageClass
.replace(/-/g, '_')
.replace(/([a-z])([A-Z])/g, (_, low, up) => {
return low + '_' + up;
})
.toUpperCase(),
};

this.copy(this, req, (err, file, apiResponse) => {
if (err) {
Expand Down Expand Up @@ -3813,20 +3805,14 @@ class File extends ServiceObject<File> {
*/
startResumableUpload_(
dup: Duplexify,
options: CreateResumableUploadOptions
options: CreateResumableUploadOptions = {}
): void {
options = extend(
true,
{
metadata: {},
},
options
);
options.metadata ??= {};

const retryOptions = this.storage.retryOptions;
if (
!this.shouldRetryBasedOnPreconditionAndIdempotencyStrat(
options?.preconditionOpts
options.preconditionOpts
)
) {
retryOptions.autoRetry = false;
Expand Down Expand Up @@ -3884,14 +3870,11 @@ class File extends ServiceObject<File> {
*
* @private
*/
startSimpleUpload_(dup: Duplexify, options?: CreateWriteStreamOptions): void {
options = extend(
true,
{
metadata: {},
},
options
);
startSimpleUpload_(
dup: Duplexify,
options: CreateWriteStreamOptions = {}
): void {
options.metadata ??= {};

const apiEndpoint = this.storage.apiEndpoint;
const bucketName = this.bucket.name;
Expand Down
63 changes: 29 additions & 34 deletions src/nodejs-common/service-object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
import {promisifyAll} from '@google-cloud/promisify';
import {EventEmitter} from 'events';
import * as extend from 'extend';
import * as r from 'teeny-request';

import {StreamRequestOptions} from '.';
Expand Down Expand Up @@ -290,17 +289,15 @@ class ServiceObject<T = any> extends EventEmitter {
const methodConfig =
(typeof this.methods.delete === 'object' && this.methods.delete) || {};

const reqOpts = extend(
true,
{
method: 'DELETE',
uri: '',
const reqOpts = {
method: 'DELETE',
uri: '',
...methodConfig.reqOpts,
qs: {
...methodConfig.reqOpts?.qs,
...options,
},
methodConfig.reqOpts,
{
qs: options,
}
);
};

// The `request` method may have been overridden to hold any special
// behavior. Ensure we call the original `request` method.
Expand Down Expand Up @@ -440,16 +437,14 @@ class ServiceObject<T = any> extends EventEmitter {
(typeof this.methods.getMetadata === 'object' &&
this.methods.getMetadata) ||
{};
const reqOpts = extend(
true,
{
uri: '',
const reqOpts = {
uri: '',
...methodConfig.reqOpts,
qs: {
...methodConfig.reqOpts?.qs,
...options,
},
methodConfig.reqOpts,
{
qs: options,
}
);
};

// The `request` method may have been overridden to hold any special
// behavior. Ensure we call the original `request` method.
Expand Down Expand Up @@ -507,19 +502,19 @@ class ServiceObject<T = any> extends EventEmitter {
this.methods.setMetadata) ||
{};

const reqOpts = extend(
true,
{},
{
method: 'PATCH',
uri: '',
const reqOpts = {
method: 'PATCH',
uri: '',
...methodConfig.reqOpts,
json: {
...methodConfig.reqOpts?.json,
...metadata,
},
methodConfig.reqOpts,
{
json: metadata,
qs: options,
}
);
qs: {
...methodConfig.reqOpts?.qs,
...options,
},
};

// The `request` method may have been overridden to hold any special
// behavior. Ensure we call the original `request` method.
Expand Down Expand Up @@ -551,7 +546,7 @@ class ServiceObject<T = any> extends EventEmitter {
reqOpts: DecorateRequestOptions | StreamRequestOptions,
callback?: BodyResponseCallback
): void | r.Request {
reqOpts = extend(true, {}, reqOpts);
reqOpts = {...reqOpts};

if (this.projectId) {
reqOpts.projectId = this.projectId;
Expand Down Expand Up @@ -612,7 +607,7 @@ class ServiceObject<T = any> extends EventEmitter {
* @param {string} reqOpts.uri - A URI relative to the baseUrl.
*/
requestStream(reqOpts: DecorateRequestOptions): r.Request {
const opts = extend(true, reqOpts, {shouldReturnStream: true});
const opts = {...reqOpts, shouldReturnStream: true};
return this.request_(opts as StreamRequestOptions);
}
}
Expand Down
17 changes: 9 additions & 8 deletions src/nodejs-common/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import * as extend from 'extend';
import {AuthClient, GoogleAuth, GoogleAuthOptions} from 'google-auth-library';
import * as r from 'teeny-request';
import * as uuid from 'uuid';
Expand Down Expand Up @@ -111,15 +110,16 @@ export class Service {
this.projectIdRequired = config.projectIdRequired !== false;
this.providedUserAgent = options.userAgent;

const reqCfg = extend({}, config, {
const reqCfg = {
...config,
projectIdRequired: this.projectIdRequired,
projectId: this.projectId,
authClient: options.authClient,
authClient: options.authClient || config.authClient,
credentials: options.credentials,
keyFile: options.keyFilename,
email: options.email,
token: options.token,
});
};

this.makeAuthenticatedRequest =
util.makeAuthenticatedRequestFactory(reqCfg);
Expand Down Expand Up @@ -192,7 +192,7 @@ export class Service {
reqOpts: DecorateRequestOptions | StreamRequestOptions,
callback?: BodyResponseCallback
): void | r.Request {
reqOpts = extend(true, {}, reqOpts, {timeout: this.timeout});
reqOpts = {...reqOpts, timeout: this.timeout};
const isAbsoluteUrl = reqOpts.uri.indexOf('http') === 0;
const uriComponents = [this.baseUrl];

Expand Down Expand Up @@ -244,12 +244,13 @@ export class Service {
if (this.providedUserAgent) {
userAgent = `${this.providedUserAgent} ${userAgent}`;
}
reqOpts.headers = extend({}, reqOpts.headers, {
reqOpts.headers = {
...reqOpts.headers,
'User-Agent': userAgent,
'x-goog-api-client': `gl-node/${process.versions.node} gccl/${
pkg.version
} gccl-invocation-id/${uuid.v4()}`,
});
};

if (reqOpts.shouldReturnStream) {
return this.makeAuthenticatedRequest(reqOpts) as {} as r.Request;
Expand Down Expand Up @@ -279,7 +280,7 @@ export class Service {
* @param {string} reqOpts.uri - A URI relative to the baseUrl.
*/
requestStream(reqOpts: DecorateRequestOptions): r.Request {
const opts = extend(true, reqOpts, {shouldReturnStream: true});
const opts = {...reqOpts, shouldReturnStream: true};
return (Service.prototype.request_ as Function).call(this, opts);
}
}
Loading