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

chore(cleanup)!: Remove teeny-request in favor of gaxios / authclient #2498

Draft
wants to merge 19 commits into
base: gaxios-feature
Choose a base branch
from
Draft
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
5 changes: 2 additions & 3 deletions conformance-test/conformanceCommon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import * as libraryMethods from './libraryMethods';
import {Bucket, File, HmacKey, Notification, Storage} from '../src/';
import * as uuid from 'uuid';
import * as assert from 'assert';
import {DecorateRequestOptions} from '../src/nodejs-common';
import fetch from 'node-fetch';

interface RetryCase {
Expand Down Expand Up @@ -127,12 +126,12 @@ export function executeScenario(testCase: RetryTestCase) {
);

storage.interceptors.push({
request: requestConfig => {
resolved: requestConfig => {
requestConfig.headers = requestConfig.headers || {};
Object.assign(requestConfig.headers, {
'x-retry-test-id': creationResult.id,
});
return requestConfig as DecorateRequestOptions;
return Promise.resolve(requestConfig);
},
});
});
Expand Down
15 changes: 11 additions & 4 deletions conformance-test/libraryMethods.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,16 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {Bucket, File, Notification, Storage, HmacKey, Policy} from '../src';
import {
Bucket,
File,
Notification,
Storage,
HmacKey,
Policy,
GaxiosError,
} from '../src';
import * as path from 'path';
import {ApiError} from '../src/nodejs-common';
import {
createTestBuffer,
createTestFileFromBuffer,
Expand Down Expand Up @@ -227,7 +234,7 @@ export async function getFilesStream(options: ConformanceTestOptions) {
.bucket!.getFilesStream()
.on('data', () => {})
.on('end', () => resolve(undefined))
.on('error', (err: ApiError) => reject(err));
.on('error', (err: GaxiosError) => reject(err));
});
}

Expand Down Expand Up @@ -496,7 +503,7 @@ export async function createReadStream(options: ConformanceTestOptions) {
.file!.createReadStream()
.on('data', () => {})
.on('end', () => resolve(undefined))
.on('error', (err: ApiError) => reject(err));
.on('error', (err: GaxiosError) => reject(err));
});
}

Expand Down
6 changes: 1 addition & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
},
"dependencies": {
"@google-cloud/paginator": "^5.0.0",
"@google-cloud/projectify": "^4.0.0",
"@google-cloud/promisify": "^4.0.0",
"abort-controller": "^3.0.0",
"async-retry": "^1.3.3",
Expand All @@ -84,8 +83,6 @@
"html-entities": "^2.5.2",
"mime": "^3.0.0",
"p-limit": "^3.0.1",
"retry-request": "^7.0.0",
"teeny-request": "^9.0.0",
"uuid": "^8.0.0"
},
"devDependencies": {
Expand All @@ -102,8 +99,7 @@
"@types/node": "^20.4.4",
"@types/node-fetch": "^2.1.3",
"@types/proxyquire": "^1.3.28",
"@types/request": "^2.48.4",
"@types/sinon": "^17.0.0",
"@types/sinon": "^17.0.3",
"@types/tmp": "0.2.6",
"@types/uuid": "^8.0.0",
"@types/yargs": "^17.0.10",
Expand Down
172 changes: 102 additions & 70 deletions src/acl.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
// Copyright 2019 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand All @@ -12,19 +13,18 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {
BodyResponseCallback,
DecorateRequestOptions,
BaseMetadata,
} from './nodejs-common/index.js';
import {BaseMetadata} from './nodejs-common/index.js';
import {promisifyAll} from '@google-cloud/promisify';
import {StorageQueryParameters, StorageTransport} from './storage-transport.js';
import {ServiceObjectParent} from './nodejs-common/service-object.js';
import {Bucket} from './bucket.js';
import {File} from './file.js';
import {GaxiosError} from 'gaxios';

export interface AclOptions {
pathPrefix: string;
request: (
reqOpts: DecorateRequestOptions,
callback: BodyResponseCallback
) => void;
storageTransport: StorageTransport;
parent: ServiceObjectParent;
}

export type GetAclResponse = [
Expand Down Expand Up @@ -68,7 +68,7 @@ export interface AddAclOptions {
export type AddAclResponse = [AccessControlObject, AclMetadata];
export interface AddAclCallback {
(
err: Error | null,
err: GaxiosError | null,
acl?: AccessControlObject | null,
apiResponse?: AclMetadata
): void;
Expand All @@ -91,7 +91,10 @@ interface AclQuery {
export interface AccessControlObject {
entity: string;
role: string;
projectTeam: string;
projectTeam?: {
projectNumber?: string;
team?: 'editors' | 'owners' | 'viewers' | string;
};
}

export interface AclMetadata extends BaseMetadata {
Expand All @@ -103,7 +106,7 @@ export interface AclMetadata extends BaseMetadata {
object?: string;
projectTeam?: {
projectNumber?: string;
team?: 'editors' | 'owners' | 'viewers';
team?: 'editors' | 'owners' | 'viewers' | string;
};
role?: 'OWNER' | 'READER' | 'WRITER' | 'FULL_CONTROL';
[key: string]: unknown;
Expand Down Expand Up @@ -418,15 +421,14 @@ class AclRoleAccessorMethods {
class Acl extends AclRoleAccessorMethods {
default!: Acl;
pathPrefix: string;
request_: (
reqOpts: DecorateRequestOptions,
callback: BodyResponseCallback
) => void;
storageTransport: StorageTransport;
parent: ServiceObjectParent;

constructor(options: AclOptions) {
super();
this.pathPrefix = options.pathPrefix;
this.request_ = options.request;
this.storageTransport = options.storageTransport;
this.parent = options.parent;
}

add(options: AddAclOptions): Promise<AddAclResponse>;
Expand Down Expand Up @@ -520,24 +522,41 @@ class Acl extends AclRoleAccessorMethods {
query.userProject = options.userProject;
}

this.request(
let url = this.pathPrefix;
if (this.parent instanceof File) {
const file = this.parent as File;
const bucket = file.parent;
url = `${bucket.baseUrl}/${bucket.name}/${file.baseUrl}/${file.name}${url}`;
} else if (this.parent instanceof Bucket) {
const bucket = this.parent as Bucket;
url = `${bucket.baseUrl}/${bucket.name}${url}`;
}

this.storageTransport.makeRequest(
{
method: 'POST',
uri: '',
qs: query,
maxRetries: 0, //explicitly set this value since this is a non-idempotent function
json: {
url,
queryParameters: query as unknown as StorageQueryParameters,
retry: false,
body: {
entity: options.entity,
role: options.role.toUpperCase(),
},
},
(err, resp) => {
(err, data, resp) => {
if (err) {
callback!(err, null, resp);
callback!(
err,
data as unknown as AccessControlObject,
resp as unknown as AclMetadata
);
return;
}

callback!(null, this.makeAclObject_(resp), resp);
callback!(
null,
this.makeAclObject_(data as unknown as AccessControlObject),
data as unknown as AclMetadata
);
}
);
}
Expand Down Expand Up @@ -620,14 +639,24 @@ class Acl extends AclRoleAccessorMethods {
query.userProject = options.userProject;
}

this.request(
let url = `${this.pathPrefix}/${options.entity}`;
if (this.parent instanceof File) {
const file = this.parent as File;
const bucket = file.parent;
url = `${bucket.baseUrl}/${bucket.name}/${file.baseUrl}/${file.name}${url}`;
} else if (this.parent instanceof Bucket) {
const bucket = this.parent as Bucket;
url = `${bucket.baseUrl}/${bucket.name}${url}`;
}

this.storageTransport.makeRequest(
{
method: 'DELETE',
uri: '/' + encodeURIComponent(options.entity),
qs: query,
url,
queryParameters: query as unknown as StorageQueryParameters,
},
(err, resp) => {
callback!(err, resp);
(err, data) => {
callback!(err, data as unknown as AclMetadata);
}
);
}
Expand Down Expand Up @@ -728,12 +757,11 @@ class Acl extends AclRoleAccessorMethods {
typeof optionsOrCallback === 'object' ? optionsOrCallback : null;
const callback =
typeof optionsOrCallback === 'function' ? optionsOrCallback : cb;
let path = '';
const query = {} as AclQuery;

let url = `${this.pathPrefix}`;
if (options) {
path = '/' + encodeURIComponent(options.entity);

url = `${url}/${options.entity}`;
if (options.generation) {
query.generation = options.generation;
}
Expand All @@ -743,26 +771,36 @@ class Acl extends AclRoleAccessorMethods {
}
}

this.request(
if (this.parent instanceof File) {
const file = this.parent as File;
const bucket = file.parent;
url = `${bucket.baseUrl}/${bucket.name}/${file.baseUrl}/${file.name}${url}`;
} else if (this.parent instanceof Bucket) {
const bucket = this.parent as Bucket;
url = `${bucket.baseUrl}/${bucket.name}${url}`;
}

this.storageTransport.makeRequest(
{
uri: path,
qs: query,
method: 'GET',
url,
queryParameters: query as unknown as StorageQueryParameters,
},
(err, resp) => {
(err, data, resp) => {
if (err) {
callback!(err, null, resp);
callback!(err, null, resp as unknown as AclMetadata);
return;
}

let results;

if (resp.items) {
results = resp.items.map(this.makeAclObject_);
if ((data as any).items) {
results = (data as any).items.map(this.makeAclObject_);
} else {
results = this.makeAclObject_(resp);
results = this.makeAclObject_(data as AccessControlObject);
}

callback!(null, results, resp);
callback!(null, results, resp as unknown as AclMetadata);
}
);
}
Expand Down Expand Up @@ -842,22 +880,35 @@ class Acl extends AclRoleAccessorMethods {
query.userProject = options.userProject;
}

this.request(
let url = `${this.pathPrefix}/${options.entity}`;
if (this.parent instanceof File) {
const file = this.parent as File;
const bucket = file.parent;
url = `${bucket.baseUrl}/${bucket.name}/${file.baseUrl}/${file.name}${url}`;
} else if (this.parent instanceof Bucket) {
const bucket = this.parent as Bucket;
url = `${bucket.baseUrl}/${bucket.name}${url}`;
}

this.storageTransport.makeRequest(
{
method: 'PUT',
uri: '/' + encodeURIComponent(options.entity),
qs: query,
json: {
url,
queryParameters: query as unknown as StorageQueryParameters,
body: {
role: options.role.toUpperCase(),
},
},
(err, resp) => {
(err, data, resp) => {
if (err) {
callback!(err, null, resp);
callback!(err, null, resp as unknown as AclMetadata);
return;
}

callback!(null, this.makeAclObject_(resp), resp);
callback!(
null,
this.makeAclObject_(data as unknown as AccessControlObject),
data as unknown as AclMetadata
);
}
);
}
Expand All @@ -881,25 +932,6 @@ class Acl extends AclRoleAccessorMethods {

return obj;
}

/**
* Patch requests up to the bucket's request object.
*
* @private
*
* @param {string} method Action.
* @param {string} path Request path.
* @param {*} query Request query object.
* @param {*} body Request body contents.
* @param {function} callback Callback function.
*/
request(
reqOpts: DecorateRequestOptions,
callback: BodyResponseCallback
): void {
reqOpts.uri = this.pathPrefix + reqOpts.uri;
this.request_(reqOpts, callback);
}
}

/*! Developer Documentation
Expand Down
Loading
Loading