Skip to content

Commit a4ae1cd

Browse files
thiyaguk09thiyaguk09
committed
bug fix
1 parent e637966 commit a4ae1cd

2 files changed

Lines changed: 111 additions & 103 deletions

File tree

‎handwritten/storage/src/file.ts‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3446,16 +3446,16 @@ class File extends ServiceObject<File, FileMetadata> {
34463446

34473447
moveFileAtomic(
34483448
destination: string | File,
3449-
options?: MoveFileAtomicOptions
3449+
options?: MoveFileAtomicOptions,
34503450
): Promise<MoveFileAtomicResponse>;
34513451
moveFileAtomic(
34523452
destination: string | File,
3453-
callback: MoveFileAtomicCallback
3453+
callback: MoveFileAtomicCallback,
34543454
): void;
34553455
moveFileAtomic(
34563456
destination: string | File,
34573457
options: MoveFileAtomicOptions,
3458-
callback: MoveFileAtomicCallback
3458+
callback: MoveFileAtomicCallback,
34593459
): void;
34603460
/**
34613461
* @typedef {array} MoveFileAtomicResponse
@@ -3555,10 +3555,10 @@ class File extends ServiceObject<File, FileMetadata> {
35553555
moveFileAtomic(
35563556
destination: string | File,
35573557
optionsOrCallback?: MoveFileAtomicOptions | MoveFileAtomicCallback,
3558-
callback?: MoveFileAtomicCallback
3558+
callback?: MoveFileAtomicCallback,
35593559
): Promise<MoveFileAtomicResponse> | void {
35603560
const noDestinationError = new Error(
3561-
FileExceptionMessages.DESTINATION_NO_NAME
3561+
FileExceptionMessages.DESTINATION_NO_NAME,
35623562
);
35633563

35643564
if (!destination) {
@@ -3595,7 +3595,7 @@ class File extends ServiceObject<File, FileMetadata> {
35953595

35963596
if (
35973597
!this.shouldRetryBasedOnPreconditionAndIdempotencyStrat(
3598-
options?.preconditionOpts
3598+
options?.preconditionOpts,
35993599
)
36003600
) {
36013601
this.storage.retryOptions.autoRetry = false;
@@ -3610,23 +3610,25 @@ class File extends ServiceObject<File, FileMetadata> {
36103610
delete options.preconditionOpts;
36113611
}
36123612

3613-
this.request(
3614-
{
3615-
method: 'POST',
3616-
uri: `/moveTo/o/${encodeURIComponent(newFile.name)}`,
3617-
qs: query,
3618-
json: options,
3619-
},
3620-
(err, resp) => {
3621-
this.storage.retryOptions.autoRetry = this.instanceRetryValue;
3622-
if (err) {
3623-
callback!(err, null, resp);
3624-
return;
3625-
}
3613+
this.storageTransport
3614+
.makeRequest(
3615+
{
3616+
method: 'POST',
3617+
url: `${this.baseUrl}/moveTo/o/${encodeURIComponent(newFile.name)}`,
3618+
queryParameters: query as StorageQueryParameters,
3619+
body: JSON.stringify(options),
3620+
},
3621+
(err, data, resp) => {
3622+
this.storage.retryOptions.autoRetry = this.instanceRetryValue;
3623+
if (err) {
3624+
callback!(err, null, resp);
3625+
return;
3626+
}
36263627

3627-
callback!(null, newFile, resp);
3628-
}
3629-
);
3628+
callback!(null, newFile, resp);
3629+
},
3630+
)
3631+
.catch(err => callback!(err));
36303632
}
36313633

36323634
move(

‎handwritten/storage/test/file.ts‎

Lines changed: 87 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -3697,74 +3697,71 @@ describe('File', () => {
36973697
function assertmoveFileAtomic(
36983698
// eslint-disable-next-line @typescript-eslint/no-explicit-any
36993699
file: any,
3700-
expectedDestination: string,
3701-
callback: Function
3700+
expectedDestination: string | File,
3701+
callback: Function,
37023702
) {
3703-
file.moveFileAtomic = (destination: string) => {
3703+
file.moveFileAtomic = (destination: string | File) => {
37043704
assert.strictEqual(destination, expectedDestination);
37053705
callback();
37063706
};
37073707
}
37083708

3709-
it('should throw if no destination is provided', () => {
3710-
assert.throws(() => {
3711-
file.moveFileAtomic();
3712-
}, /Destination file should have a name\./);
3709+
it('should throw if no destination is provided', async () => {
3710+
try {
3711+
await file.moveFileAtomic(undefined as unknown as string);
3712+
} catch (error) {
3713+
assert.strictEqual(
3714+
(error as Error).message,
3715+
FileExceptionMessages.DESTINATION_NO_NAME,
3716+
);
3717+
}
37133718
});
37143719

3715-
it('should URI encode file names', done => {
3720+
it('should URI encode file names', async () => {
37163721
const newFile = new File(BUCKET, 'nested/file.jpg');
37173722

3718-
const expectedPath = `/moveTo/o/${encodeURIComponent(newFile.name)}`;
3723+
const expectedPath = `/o/moveTo/o/${encodeURIComponent(newFile.name)}`;
37193724

3720-
directoryFile.request = (reqOpts: DecorateRequestOptions) => {
3721-
assert.strictEqual(reqOpts.uri, expectedPath);
3722-
done();
3723-
};
3724-
3725-
directoryFile.moveFileAtomic(newFile);
3725+
directoryFile.storageTransport.makeRequest = sandbox
3726+
.stub()
3727+
.callsFake(reqOpts => {
3728+
assert.strictEqual(reqOpts.url, expectedPath);
3729+
return Promise.resolve();
3730+
});
3731+
await directoryFile.moveFileAtomic(newFile, err => {
3732+
assert.ifError(err);
3733+
});
37263734
});
37273735

3728-
it('should call moveFileAtomic with string', done => {
3736+
it('should call moveFileAtomic with string', async done => {
37293737
const newFileName = 'new-file-name.png';
37303738
assertmoveFileAtomic(file, newFileName, done);
3731-
file.moveFileAtomic(newFileName);
3739+
await file.moveFileAtomic(newFileName);
37323740
});
37333741

3734-
it('should call moveFileAtomic with File', done => {
3742+
it('should call moveFileAtomic with File', async done => {
37353743
const newFile = new File(BUCKET, 'new-file');
37363744
assertmoveFileAtomic(file, newFile, done);
3737-
file.moveFileAtomic(newFile);
3738-
});
3739-
3740-
it('should accept an options object', done => {
3741-
const newFile = new File(BUCKET, 'name');
3742-
const options = {};
3743-
3744-
file.moveFileAtomic = (destination: {}, options_: {}) => {
3745-
assert.strictEqual(options_, options);
3746-
done();
3747-
};
3748-
3749-
file.moveFileAtomic(newFile, options, assert.ifError);
3745+
await file.moveFileAtomic(newFile);
37503746
});
37513747

3752-
it('should execute callback with error & API response', done => {
3753-
const error = new Error('Error.');
3748+
it('should execute callback with error & API response', async () => {
3749+
const error = new GaxiosError('Error.', {});
37543750
const apiResponse = {};
37553751

37563752
const newFile = new File(BUCKET, 'new-file');
37573753

3758-
file.request = (reqOpts: DecorateRequestOptions, callback: Function) => {
3759-
callback(error, apiResponse);
3760-
};
3754+
file.storageTransport.makeRequest = sandbox
3755+
.stub()
3756+
.callsFake((reqOpts, callback) => {
3757+
callback(error, null, apiResponse);
3758+
return Promise.resolve();
3759+
});
37613760

3762-
file.moveFileAtomic(newFile, (err: Error, file: {}, apiResponse_: {}) => {
3761+
await file.moveFileAtomic(newFile, (err, file, apiResponse_) => {
37633762
assert.strictEqual(err, error);
37643763
assert.strictEqual(file, null);
37653764
assert.strictEqual(apiResponse_, apiResponse);
3766-
3767-
done();
37683765
});
37693766
});
37703767

@@ -3775,12 +3772,15 @@ describe('File', () => {
37753772
const originalOptions = Object.assign({}, options);
37763773
const newFile = new File(BUCKET, 'new-file');
37773774

3778-
file.request = (reqOpts: DecorateRequestOptions) => {
3779-
assert.strictEqual(reqOpts.qs.userProject, options.userProject);
3780-
assert.strictEqual(reqOpts.json.userProject, undefined);
3775+
file.storageTransport.makeRequest = sandbox.stub().callsFake(reqOpts => {
3776+
assert.strictEqual(
3777+
reqOpts.queryParameters?.userProject,
3778+
options.userProject,
3779+
);
3780+
assert.strictEqual(reqOpts.body.userProject, undefined);
37813781
assert.deepStrictEqual(options, originalOptions);
37823782
done();
3783-
};
3783+
});
37843784

37853785
file.moveFileAtomic(newFile, options, assert.ifError);
37863786
});
@@ -3792,15 +3792,15 @@ describe('File', () => {
37923792
const originalOptions = Object.assign({}, options);
37933793
const newFile = new File(BUCKET, 'new-file');
37943794

3795-
file.request = (reqOpts: DecorateRequestOptions) => {
3795+
file.storageTransport.makeRequest = sandbox.stub().callsFake(reqOpts => {
37963796
assert.strictEqual(
3797-
reqOpts.qs.ifGenerationMatch,
3798-
options.preconditionOpts.ifGenerationMatch
3797+
reqOpts.queryParameters?.ifGenerationMatch,
3798+
options.preconditionOpts.ifGenerationMatch,
37993799
);
3800-
assert.strictEqual(reqOpts.json.userProject, undefined);
3800+
assert.strictEqual(reqOpts.body?.userProject, undefined);
38013801
assert.deepStrictEqual(options, originalOptions);
38023802
done();
3803-
};
3803+
});
38043804

38053805
file.moveFileAtomic(newFile, options, assert.ifError);
38063806
});
@@ -3810,77 +3810,83 @@ describe('File', () => {
38103810
// eslint-disable-next-line @typescript-eslint/no-explicit-any
38113811
file: any,
38123812
expectedPath: string,
3813-
callback: Function
3813+
callback: Function,
38143814
) {
3815-
file.request = (reqOpts: DecorateRequestOptions) => {
3816-
assert.strictEqual(reqOpts.uri, expectedPath);
3817-
callback();
3818-
};
3815+
file.storageTransport.makeRequest = sandbox
3816+
.stub()
3817+
.callsFake(reqOpts => {
3818+
assert.strictEqual(reqOpts.url, expectedPath);
3819+
callback();
3820+
});
38193821
}
38203822

3821-
it('should allow a string', done => {
3823+
it('should allow a string', async done => {
38223824
const newFileName = 'new-file-name.png';
38233825
const newFile = new File(BUCKET, newFileName);
3824-
const expectedPath = `/moveTo/o/${newFile.name}`;
3826+
const expectedPath = `/o/moveTo/o/${newFile.name}`;
38253827
assertPathEquals(file, expectedPath, done);
3826-
file.moveFileAtomic(newFileName);
3828+
await file.moveFileAtomic(newFileName);
38273829
});
38283830

3829-
it('should allow a string with leading slash.', done => {
3831+
it('should allow a string with leading slash.', async done => {
38303832
const newFileName = '/new-file-name.png';
38313833
const newFile = new File(BUCKET, newFileName);
3832-
const expectedPath = `/moveTo/o/${encodeURIComponent(newFile.name)}`;
3834+
const expectedPath = `/o/moveTo/o/${encodeURIComponent(newFile.name)}`;
38333835
assertPathEquals(file, expectedPath, done);
3834-
file.moveFileAtomic(newFileName);
3836+
await file.moveFileAtomic(newFileName);
38353837
});
38363838

3837-
it('should allow a "gs://..." string', done => {
3839+
it('should allow a "gs://..." string', async done => {
38383840
const newFileName = 'gs://other-bucket/new-file-name.png';
3839-
const expectedPath = '/moveTo/o/new-file-name.png';
3841+
const expectedPath = '/o/moveTo/o/new-file-name.png';
38403842
assertPathEquals(file, expectedPath, done);
3841-
file.moveFileAtomic(newFileName);
3843+
await file.moveFileAtomic(newFileName);
38423844
});
38433845

3844-
it('should allow a File', done => {
3846+
it('should allow a File', async done => {
38453847
const newFile = new File(BUCKET, 'new-file');
3846-
const expectedPath = `/moveTo/o/${newFile.name}`;
3848+
const expectedPath = `/o/moveTo/o/${newFile.name}`;
38473849
assertPathEquals(file, expectedPath, done);
3848-
file.moveFileAtomic(newFile);
3850+
await file.moveFileAtomic(newFile);
38493851
});
38503852

3851-
it('should throw if a destination cannot be parsed', () => {
3852-
assert.throws(() => {
3853-
file.moveFileAtomic(() => {});
3854-
}, /Destination file should have a name\./);
3853+
it('should throw if a destination cannot be parsed', async () => {
3854+
try {
3855+
await file.moveFileAtomic(undefined as unknown as string);
3856+
} catch (error) {
3857+
assert.strictEqual(
3858+
(error as Error).message,
3859+
FileExceptionMessages.DESTINATION_NO_NAME,
3860+
);
3861+
}
38553862
});
38563863
});
38573864

38583865
describe('returned File object', () => {
38593866
beforeEach(() => {
38603867
const resp = {success: true};
3861-
file.request = (
3862-
reqOpts: DecorateRequestOptions,
3863-
callback: Function
3864-
) => {
3865-
callback(null, resp);
3866-
};
3868+
file.storageTransport.makeRequest = sandbox
3869+
.stub()
3870+
.callsFake((reqOpts, callback) => {
3871+
callback(null, resp);
3872+
});
38673873
});
38683874

3869-
it('should re-use file object if one is provided', done => {
3875+
it('should re-use file object if one is provided', async done => {
38703876
const newFile = new File(BUCKET, 'new-file');
3871-
file.moveFileAtomic(newFile, (err: Error, copiedFile: {}) => {
3877+
await file.moveFileAtomic(newFile, (err, copiedFile) => {
38723878
assert.ifError(err);
38733879
assert.deepStrictEqual(copiedFile, newFile);
38743880
done();
38753881
});
38763882
});
38773883

3878-
it('should create new file on the same bucket', done => {
3884+
it('should create new file on the same bucket', async done => {
38793885
const newFilename = 'new-filename';
3880-
file.moveFileAtomic(newFilename, (err: Error, copiedFile: File) => {
3886+
await file.moveFileAtomic(newFilename, (err, copiedFile) => {
38813887
assert.ifError(err);
3882-
assert.strictEqual(copiedFile.bucket.name, BUCKET.name);
3883-
assert.strictEqual(copiedFile.name, newFilename);
3888+
assert.strictEqual(copiedFile?.bucket.name, BUCKET.name);
3889+
assert.strictEqual(copiedFile?.name, newFilename);
38843890
done();
38853891
});
38863892
});

0 commit comments

Comments
 (0)