From ed52b673297367900e8a17a194598bba3eaecb76 Mon Sep 17 00:00:00 2001 From: ricardo Date: Mon, 20 Apr 2020 19:33:38 -0500 Subject: [PATCH 1/3] :zap: improvements --- .../nodes-base/nodes/Aws/S3/AwsS3.node.ts | 78 +++++++++++++++---- .../nodes/Aws/S3/BucketDescription.ts | 7 ++ .../nodes/Aws/S3/GenericFunctions.ts | 18 +++-- 3 files changed, 78 insertions(+), 25 deletions(-) diff --git a/packages/nodes-base/nodes/Aws/S3/AwsS3.node.ts b/packages/nodes-base/nodes/Aws/S3/AwsS3.node.ts index f86939ff7c..0dff8f8619 100644 --- a/packages/nodes-base/nodes/Aws/S3/AwsS3.node.ts +++ b/packages/nodes-base/nodes/Aws/S3/AwsS3.node.ts @@ -137,6 +137,11 @@ export class AwsS3 implements INodeType { if (additionalFields.grantWriteAcp) { headers['x-amz-grant-write-acp']=additionalFields.grantWriteAcp; } + let region = credentials!.region as string; + + if (additionalFields.region) { + region = additionalFields.region as string; + } const body: IDataObject = { CreateBucketConfiguration: { @@ -147,9 +152,9 @@ export class AwsS3 implements INodeType { }; let data = ''; // if credentials has the S3 defaul region (us-east-1) the body (XML) does not have to be sent. - if (credentials!.region !== 'us-east-1') { + if (region !== 'us-east-1') { // @ts-ignore - body.CreateBucketConfiguration.LocationConstraint = [credentials!.region]; + body.CreateBucketConfiguration.LocationConstraint = [region]; const builder = new Builder(); data = builder.buildObject(body); } @@ -169,6 +174,7 @@ export class AwsS3 implements INodeType { } returnData.push.apply(returnData, responseData); } + //https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html if (operation === 'search') { const bucketName = this.getNodeParameter('bucketName', i) as string; @@ -201,11 +207,16 @@ export class AwsS3 implements INodeType { qs['list-type'] = 2; + + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'GET', '', '', { location: '' }); + + const region = responseData.LocationConstraint._ as string; + if (returnAll) { - responseData = await awsApiRequestSOAPAllItems.call(this, 'ListBucketResult.Contents', `${bucketName}.s3`, 'GET', '', '', qs); + responseData = await awsApiRequestSOAPAllItems.call(this, 'ListBucketResult.Contents', `${bucketName}.s3`, 'GET', '', '', qs, {}, {}, region); } else { qs['max-keys'] = this.getNodeParameter('limit', 0) as number; - responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'GET', '', '', qs); + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'GET', '', '', qs, {}, {}, region); responseData = responseData.ListBucketResult.Contents; } if (Array.isArray(responseData)) { @@ -232,7 +243,11 @@ export class AwsS3 implements INodeType { if (additionalFields.storageClass) { headers['x-amz-storage-class'] = (snakeCase(additionalFields.storageClass as string)).toUpperCase(); } - responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'PUT', path, '', qs, headers); + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'GET', '', '', { location: '' }); + + const region = responseData.LocationConstraint._; + + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'PUT', path, '', qs, headers, {}, region); returnData.push({ success: true }); } //https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html @@ -240,12 +255,16 @@ export class AwsS3 implements INodeType { const bucketName = this.getNodeParameter('bucketName', i) as string; const folderKey = this.getNodeParameter('folderKey', i) as string; - responseData = await awsApiRequestSOAPAllItems.call(this, 'ListBucketResult.Contents', `${bucketName}.s3`, 'GET', '/', '', { 'list-type': 2, prefix: folderKey }); + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'GET', '', '', { location: '' }); + + const region = responseData.LocationConstraint._; + + responseData = await awsApiRequestSOAPAllItems.call(this, 'ListBucketResult.Contents', `${bucketName}.s3`, 'GET', '/', '', { 'list-type': 2, prefix: folderKey }, {}, {}, region); // folder empty then just delete it if (responseData.length === 0) { - responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'DELETE', `/${folderKey}`, '', qs); + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'DELETE', `/${folderKey}`, '', qs, {}, {}, region); responseData = { deleted: [ { 'Key': folderKey } ] }; @@ -274,7 +293,7 @@ export class AwsS3 implements INodeType { headers['Content-Type'] = 'application/xml'; - responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'POST', '/', data, { delete: '' } , headers); + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'POST', '/', data, { delete: '' } , headers, {}, region); responseData = { deleted: responseData.DeleteResult.Deleted }; } @@ -296,11 +315,15 @@ export class AwsS3 implements INodeType { qs['list-type'] = 2; + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'GET', '', '', { location: '' }); + + const region = responseData.LocationConstraint._; + if (returnAll) { - responseData = await awsApiRequestSOAPAllItems.call(this, 'ListBucketResult.Contents', `${bucketName}.s3`, 'GET', '', '', qs); + responseData = await awsApiRequestSOAPAllItems.call(this, 'ListBucketResult.Contents', `${bucketName}.s3`, 'GET', '', '', qs, {}, {}, region); } else { qs.limit = this.getNodeParameter('limit', 0) as number; - responseData = await awsApiRequestSOAPAllItems.call(this, 'ListBucketResult.Contents', `${bucketName}.s3`, 'GET', '', '', qs); + responseData = await awsApiRequestSOAPAllItems.call(this, 'ListBucketResult.Contents', `${bucketName}.s3`, 'GET', '', '', qs, {}, {}, region); } if (Array.isArray(responseData)) { responseData = responseData.filter((e: IDataObject) => (e.Key as string).endsWith('/') && e.Size === '0' && e.Key !== options.folderKey); @@ -376,7 +399,11 @@ export class AwsS3 implements INodeType { headers['x-amz-metadata-directive'] = (additionalFields.metadataDirective as string).toUpperCase(); } - responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'PUT', `${destination}`, '', qs, headers); + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'GET', '', '', { location: '' }); + + const region = responseData.LocationConstraint._; + + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'PUT', `${destination}`, '', qs, headers, {}, region); returnData.push(responseData.CopyObjectResult); } @@ -393,7 +420,11 @@ export class AwsS3 implements INodeType { throw new Error('Downloding a whole directory is not yet supported, please provide a file key'); } - const response = await awsApiRequestREST.call(this, `${bucketName}.s3`, 'GET', `/${fileKey}`, '', qs, {}, { encoding: null, resolveWithFullResponse: true }); + let region = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'GET', '', '', { location: '' }); + + region = region.LocationConstraint._; + + const response = await awsApiRequestREST.call(this, `${bucketName}.s3`, 'GET', `/${fileKey}`, '', qs, {}, { encoding: null, resolveWithFullResponse: true }, region); let mimeType: string | undefined; if (response.headers['content-type']) { @@ -432,7 +463,11 @@ export class AwsS3 implements INodeType { qs.versionId = options.versionId as string; } - responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'DELETE', `/${fileKey}`, '', qs); + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'GET', '', '', { location: '' }); + + const region = responseData.LocationConstraint._; + + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'DELETE', `/${fileKey}`, '', qs, {}, {}, region); returnData.push({ success: true }); } @@ -454,11 +489,15 @@ export class AwsS3 implements INodeType { qs['list-type'] = 2; + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'GET', '', '', { location: '' }); + + const region = responseData.LocationConstraint._; + if (returnAll) { - responseData = await awsApiRequestSOAPAllItems.call(this, 'ListBucketResult.Contents', `${bucketName}.s3`, 'GET', '', '', qs); + responseData = await awsApiRequestSOAPAllItems.call(this, 'ListBucketResult.Contents', `${bucketName}.s3`, 'GET', '', '', qs, {}, {}, region); } else { qs.limit = this.getNodeParameter('limit', 0) as number; - responseData = await awsApiRequestSOAPAllItems.call(this, 'ListBucketResult.Contents', `${bucketName}.s3`, 'GET', '', '', qs); + responseData = await awsApiRequestSOAPAllItems.call(this, 'ListBucketResult.Contents', `${bucketName}.s3`, 'GET', '', '', qs, {}, {}, region); responseData = responseData.splice(0, qs.limit); } if (Array.isArray(responseData)) { @@ -536,6 +575,11 @@ export class AwsS3 implements INodeType { tagsValues.forEach((o: IDataObject) => { tags.push(`${o.key}=${o.value}`); }); headers['x-amz-tagging'] = tags.join('&'); } + + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'GET', '', '', { location: '' }); + + const region = responseData.LocationConstraint._; + if (isBinaryData) { const binaryPropertyName = this.getNodeParameter('binaryPropertyName', 0) as string; @@ -555,7 +599,7 @@ export class AwsS3 implements INodeType { headers['Content-MD5'] = createHash('md5').update(body).digest('base64'); - responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'PUT', `${path}${fileName || binaryData.fileName}`, body, qs, headers); + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'PUT', `${path}${fileName || binaryData.fileName}`, body, qs, headers, {}, region); } else { @@ -567,7 +611,7 @@ export class AwsS3 implements INodeType { headers['Content-MD5'] = createHash('md5').update(fileContent).digest('base64'); - responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'PUT', `${path}${fileName}`, body, qs, headers); + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'PUT', `${path}${fileName}`, body, qs, headers, {}, region); } returnData.push({ success: true }); } diff --git a/packages/nodes-base/nodes/Aws/S3/BucketDescription.ts b/packages/nodes-base/nodes/Aws/S3/BucketDescription.ts index 822dd4781c..1294678904 100644 --- a/packages/nodes-base/nodes/Aws/S3/BucketDescription.ts +++ b/packages/nodes-base/nodes/Aws/S3/BucketDescription.ts @@ -143,6 +143,13 @@ export const bucketFields = [ default: false, description: 'Allows grantee to write the ACL for the applicable bucket.', }, + { + displayName: 'Region', + name: 'region', + type: 'string', + default: '', + description: 'Region you want to create the bucket in, by default the buckets are created on the region defined on the credentials.', + }, ], }, /* -------------------------------------------------------------------------- */ diff --git a/packages/nodes-base/nodes/Aws/S3/GenericFunctions.ts b/packages/nodes-base/nodes/Aws/S3/GenericFunctions.ts index e27ce52ba6..92d620d102 100644 --- a/packages/nodes-base/nodes/Aws/S3/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Aws/S3/GenericFunctions.ts @@ -25,12 +25,13 @@ import { IDataObject, } from 'n8n-workflow'; -export async function awsApiRequest(this: IHookFunctions | IExecuteFunctions | ILoadOptionsFunctions | IWebhookFunctions, service: string, method: string, path: string, body?: string | Buffer, query: IDataObject = {}, headers?: object, option: IDataObject = {}): Promise { // tslint:disable-line:no-any +export async function awsApiRequest(this: IHookFunctions | IExecuteFunctions | ILoadOptionsFunctions | IWebhookFunctions, service: string, method: string, path: string, body?: string | Buffer, query: IDataObject = {}, headers?: object, option: IDataObject = {}, region?: string): Promise { // tslint:disable-line:no-any const credentials = this.getCredentials('aws'); if (credentials === undefined) { throw new Error('No credentials got returned!'); } - const endpoint = `${service}.${credentials.region}.amazonaws.com`; + + const endpoint = `${service}.${region || credentials.region}.amazonaws.com`; // Sign AWS API request with the user credentials const signOpts = {headers: headers || {}, host: endpoint, method, path: `${path}?${queryToString(query).replace(/\+/g, '%2B')}`, body}; @@ -44,6 +45,7 @@ export async function awsApiRequest(this: IHookFunctions | IExecuteFunctions | I uri: `https://${endpoint}${signOpts.path}`, body: signOpts.body, }; + if (Object.keys(option).length !== 0) { Object.assign(options, option); } @@ -64,8 +66,8 @@ export async function awsApiRequest(this: IHookFunctions | IExecuteFunctions | I } } -export async function awsApiRequestREST(this: IHookFunctions | IExecuteFunctions | ILoadOptionsFunctions, service: string, method: string, path: string, body?: string, query: IDataObject = {}, headers?: object, options: IDataObject = {}): Promise { // tslint:disable-line:no-any - const response = await awsApiRequest.call(this, service, method, path, body, query, headers, options); +export async function awsApiRequestREST(this: IHookFunctions | IExecuteFunctions | ILoadOptionsFunctions, service: string, method: string, path: string, body?: string, query: IDataObject = {}, headers?: object, options: IDataObject = {}, region?: string): Promise { // tslint:disable-line:no-any + const response = await awsApiRequest.call(this, service, method, path, body, query, headers, options, region); try { return JSON.parse(response); } catch (e) { @@ -73,8 +75,8 @@ export async function awsApiRequestREST(this: IHookFunctions | IExecuteFunctions } } -export async function awsApiRequestSOAP(this: IHookFunctions | IExecuteFunctions | ILoadOptionsFunctions | IWebhookFunctions, service: string, method: string, path: string, body?: string | Buffer, query: IDataObject = {}, headers?: object): Promise { // tslint:disable-line:no-any - const response = await awsApiRequest.call(this, service, method, path, body, query, headers); +export async function awsApiRequestSOAP(this: IHookFunctions | IExecuteFunctions | ILoadOptionsFunctions | IWebhookFunctions, service: string, method: string, path: string, body?: string | Buffer, query: IDataObject = {}, headers?: object, option: IDataObject = {}, region?: string): Promise { // tslint:disable-line:no-any + const response = await awsApiRequest.call(this, service, method, path, body, query, headers, option, region); try { return await new Promise((resolve, reject) => { parseString(response, { explicitArray: false }, (err, data) => { @@ -89,14 +91,14 @@ export async function awsApiRequestSOAP(this: IHookFunctions | IExecuteFunctions } } -export async function awsApiRequestSOAPAllItems(this: IHookFunctions | IExecuteFunctions | ILoadOptionsFunctions | IWebhookFunctions, propertyName: string, service: string, method: string, path: string, body?: string, query: IDataObject = {}, headers: IDataObject = {}): Promise { // tslint:disable-line:no-any +export async function awsApiRequestSOAPAllItems(this: IHookFunctions | IExecuteFunctions | ILoadOptionsFunctions | IWebhookFunctions, propertyName: string, service: string, method: string, path: string, body?: string, query: IDataObject = {}, headers: IDataObject = {}, option: IDataObject = {}, region?: string): Promise { // tslint:disable-line:no-any const returnData: IDataObject[] = []; let responseData; do { - responseData = await awsApiRequestSOAP.call(this, service, method, path, body, query, headers); + responseData = await awsApiRequestSOAP.call(this, service, method, path, body, query, headers, option, region); //https://forums.aws.amazon.com/thread.jspa?threadID=55746 if (get(responseData, `${propertyName.split('.')[0]}.NextContinuationToken`)) { From bedc7f88e48ead75ef5f8811b7f3046b9f777d01 Mon Sep 17 00:00:00 2001 From: ricardo Date: Tue, 21 Apr 2020 23:13:30 -0500 Subject: [PATCH 2/3] :zap: Small improvements --- .../nodes-base/nodes/Aws/S3/AwsS3.node.ts | 15 ++++++---- .../nodes/Aws/S3/FileDescription.ts | 30 ++++--------------- 2 files changed, 16 insertions(+), 29 deletions(-) diff --git a/packages/nodes-base/nodes/Aws/S3/AwsS3.node.ts b/packages/nodes-base/nodes/Aws/S3/AwsS3.node.ts index 0dff8f8619..cc7d141e68 100644 --- a/packages/nodes-base/nodes/Aws/S3/AwsS3.node.ts +++ b/packages/nodes-base/nodes/Aws/S3/AwsS3.node.ts @@ -337,12 +337,11 @@ export class AwsS3 implements INodeType { if (resource === 'file') { //https://docs.aws.amazon.com/AmazonS3/latest/API/API_CopyObject.html if (operation === 'copy') { - const source = this.getNodeParameter('source', i) as string; - const bucketName = this.getNodeParameter('bucketName', i) as string; - const destination = this.getNodeParameter('destination', i) as string; + const sourcePath = this.getNodeParameter('sourcePath', i) as string; + const destinationPath = this.getNodeParameter('destinationPath', i) as string; const additionalFields = this.getNodeParameter('additionalFields', i) as IDataObject; - headers['x-amz-copy-source'] = source; + headers['x-amz-copy-source'] = sourcePath; if (additionalFields.requesterPays) { headers['x-amz-request-payer'] = 'requester'; @@ -399,11 +398,17 @@ export class AwsS3 implements INodeType { headers['x-amz-metadata-directive'] = (additionalFields.metadataDirective as string).toUpperCase(); } + const destinationParts = destinationPath.split('/'); + + const bucketName = destinationParts[1]; + + const destination = `/${destinationParts.slice(2, destinationParts.length).join('/')}`; + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'GET', '', '', { location: '' }); const region = responseData.LocationConstraint._; - responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'PUT', `${destination}`, '', qs, headers, {}, region); + responseData = await awsApiRequestSOAP.call(this, `${bucketName}.s3`, 'PUT', destination, '', qs, headers, {}, region); returnData.push(responseData.CopyObjectResult); } diff --git a/packages/nodes-base/nodes/Aws/S3/FileDescription.ts b/packages/nodes-base/nodes/Aws/S3/FileDescription.ts index b1c6d133ac..17e7642ff2 100644 --- a/packages/nodes-base/nodes/Aws/S3/FileDescription.ts +++ b/packages/nodes-base/nodes/Aws/S3/FileDescription.ts @@ -52,8 +52,8 @@ export const fileFields = [ /* file:copy */ /* -------------------------------------------------------------------------- */ { - displayName: 'Source', - name: 'source', + displayName: 'Source Path', + name: 'sourcePath', type: 'string', required: true, default: '', @@ -71,11 +71,12 @@ export const fileFields = [ description: 'The name of the source bucket and key name of the source object, separated by a slash (/)', }, { - displayName: 'Bucket Name', - name: 'bucketName', + displayName: 'Destination Path', + name: 'destinationPath', type: 'string', required: true, default: '', + placeholder: '/bucket/my-second-image.jpg', displayOptions: { show: { resource: [ @@ -86,26 +87,7 @@ export const fileFields = [ ], }, }, - description: 'The name of the destination bucket.', - }, - { - displayName: 'Destination', - name: 'destination', - type: 'string', - required: true, - default: '', - placeholder: '/my-second-image.jpg', - displayOptions: { - show: { - resource: [ - 'file', - ], - operation: [ - 'copy', - ], - }, - }, - description: 'The key of the destination object.', + description: 'The name of the destination bucket and key name of the destination object, separated by a slash (/)', }, { displayName: 'Additional Fields', From b24537b4a9f905b539e3e47427ba651294efe2b4 Mon Sep 17 00:00:00 2001 From: Jan Oberhauser Date: Wed, 22 Apr 2020 11:20:26 +0200 Subject: [PATCH 3/3] :zap: Change one default value --- packages/nodes-base/nodes/Aws/S3/FileDescription.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nodes-base/nodes/Aws/S3/FileDescription.ts b/packages/nodes-base/nodes/Aws/S3/FileDescription.ts index 17e7642ff2..208b847b3d 100644 --- a/packages/nodes-base/nodes/Aws/S3/FileDescription.ts +++ b/packages/nodes-base/nodes/Aws/S3/FileDescription.ts @@ -395,7 +395,7 @@ export const fileFields = [ displayName: 'Binary Data', name: 'binaryData', type: 'boolean', - default: false, + default: true, displayOptions: { show: { operation: [