Skip to content

Commit 17ac6a2

Browse files
theburningmonktheburningmonk
committed
fix: fixed bugs identified by int test
1 parent 4598829 commit 17ac6a2

File tree

2 files changed

+170
-39
lines changed

2 files changed

+170
-39
lines changed

‎lib/package/s3/compileMethodsToS3.js‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,25 @@ module.exports = {
2727
this.getMethodResponses(event.http)
2828
)
2929

30+
// ensure every integration request and response param mapping is
31+
// also configured in the method request and response param mappings
32+
Object.values(template.Properties.Integration.RequestParameters)
33+
.filter((x) => typeof x === 'string' && x.startsWith('method.'))
34+
.forEach((x) => {
35+
template.Properties.RequestParameters[x] = true
36+
})
37+
38+
template.Properties.Integration.IntegrationResponses.forEach((resp) => {
39+
Object.keys(resp.ResponseParameters)
40+
.filter((x) => x.startsWith('method.'))
41+
.forEach((x) => {
42+
const methodResp = template.Properties.MethodResponses.find(
43+
(y) => y.StatusCode === resp.StatusCode
44+
)
45+
methodResp.ResponseParameters[x] = true
46+
})
47+
})
48+
3049
const methodLogicalId = this.provider.naming.getMethodLogicalId(
3150
resourceName,
3251
event.http.method
@@ -61,10 +80,10 @@ module.exports = {
6180
return `method.request.querystring.${http.key.queryStringParam}`
6281
}
6382

64-
return http.key
83+
return `'${http.key}'`
6584
},
6685

67-
getRequestParameters(http) {
86+
getIntegrationRequestParameters(http) {
6887
switch (http.action) {
6988
case 'GetObject':
7089
return {}
@@ -78,7 +97,7 @@ module.exports = {
7897
}
7998
},
8099

81-
getResponseParameters(http) {
100+
getIntegrationResponseParameters(http) {
82101
switch (http.action) {
83102
case 'GetObject':
84103
return {
@@ -102,11 +121,13 @@ module.exports = {
102121
const bucket = http.bucket
103122
const httpMethod = this.getIntegrationHttpMethod(http)
104123
const objectRequestParam = this.getObjectRequestParameter(http)
105-
const requestParams = _.merge(this.getRequestParameters(http), {
124+
const requestParams = _.merge(this.getIntegrationRequestParameters(http), {
106125
'integration.request.path.object': objectRequestParam,
107-
'integration.request.path.bucket': bucket
126+
'integration.request.path.bucket': {
127+
'Fn::Sub': ["'${bucket}'", { bucket }]
128+
}
108129
})
109-
const responseParams = this.getResponseParameters(http)
130+
const responseParams = this.getIntegrationResponseParameters(http)
110131

111132
const integration = {
112133
IntegrationHttpMethod: httpMethod,
@@ -117,7 +138,7 @@ module.exports = {
117138
Uri: {
118139
'Fn::Sub': ['arn:aws:apigateway:${AWS::Region}:s3:path/{bucket}/{object}', {}]
119140
},
120-
PassthroughBehavior: 'NEVER',
141+
PassthroughBehavior: 'WHEN_NO_MATCH',
121142
RequestParameters: requestParams
122143
}
123144

‎lib/package/s3/compileMethodsToS3.test.js‎

Lines changed: 142 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ const template = {
2121
Uri: {
2222
'Fn::Sub': ['arn:aws:apigateway:${AWS::Region}:s3:path/{bucket}/{object}', {}]
2323
},
24-
PassthroughBehavior: 'NEVER',
24+
PassthroughBehavior: 'WHEN_NO_MATCH',
2525
RequestParameters: {},
2626
IntegrationResponses: [
2727
{
@@ -69,7 +69,18 @@ describe('#compileMethodsToS3()', () => {
6969
serverlessApigatewayServiceProxy = new ServerlessApigatewayServiceProxy(serverless, options)
7070
})
7171

72-
const testSingleProxy = (http, logicalId, method, intMethod, requestParams, responseParams) => {
72+
const testSingleProxy = (opts) => {
73+
const {
74+
http,
75+
logicalId,
76+
method,
77+
intMethod,
78+
requestParams,
79+
intRequestParams,
80+
responseParams,
81+
intResponseParams
82+
} = opts
83+
7384
serverlessApigatewayServiceProxy.validated = {
7485
events: [
7586
{
@@ -91,9 +102,10 @@ describe('#compileMethodsToS3()', () => {
91102
const diff = {
92103
Properties: {
93104
HttpMethod: method,
105+
RequestParameters: requestParams,
94106
Integration: {
95107
IntegrationHttpMethod: intMethod,
96-
RequestParameters: requestParams,
108+
RequestParameters: intRequestParams,
97109
IntegrationResponses: [
98110
{
99111
StatusCode: 400,
@@ -110,20 +122,23 @@ describe('#compileMethodsToS3()', () => {
110122
{
111123
StatusCode: 200,
112124
SelectionPattern: '200',
113-
ResponseParameters: responseParams,
125+
ResponseParameters: intResponseParams,
114126
ResponseTemplates: {}
115127
}
116128
]
117129
}
118130
}
119131
}
120132
const resource = _.merge({}, template, diff)
133+
const methodResponse = resource.Properties.MethodResponses.find((x) => x.StatusCode === 200)
134+
methodResponse.ResponseParameters = responseParams
135+
121136
expect(serverless.service.provider.compiledCloudFormationTemplate.Resources).to.deep.equal({
122137
[logicalId]: resource
123138
})
124139
}
125140

126-
const testGetObject = (key, pathRequestParam) => {
141+
const testGetObject = (key, keyRequestParam) => {
127142
const http = {
128143
path: 's3',
129144
method: 'get',
@@ -134,17 +149,45 @@ describe('#compileMethodsToS3()', () => {
134149
key
135150
}
136151

137-
const requestParams = {
138-
'integration.request.path.object': pathRequestParam,
139-
'integration.request.path.bucket': { Ref: 'MyBucket' }
152+
const requestParams = {}
153+
if (keyRequestParam.startsWith('method.')) {
154+
requestParams[keyRequestParam] = true
155+
}
156+
157+
const intRequestParams = {
158+
'integration.request.path.object': keyRequestParam,
159+
'integration.request.path.bucket': {
160+
'Fn::Sub': [
161+
"'${bucket}'",
162+
{
163+
bucket: {
164+
Ref: 'MyBucket'
165+
}
166+
}
167+
]
168+
}
140169
}
141170

142171
const responseParams = {
172+
'method.response.header.content-type': true,
173+
'method.response.header.Content-Type': true
174+
}
175+
176+
const intResponseParams = {
143177
'method.response.header.content-type': 'integration.response.header.content-type',
144178
'method.response.header.Content-Type': 'integration.response.header.Content-Type'
145179
}
146180

147-
testSingleProxy(http, 'ApiGatewayMethods3Get', 'GET', 'GET', requestParams, responseParams)
181+
testSingleProxy({
182+
http,
183+
logicalId: 'ApiGatewayMethods3Get',
184+
method: 'GET',
185+
intMethod: 'GET',
186+
requestParams,
187+
intRequestParams,
188+
responseParams,
189+
intResponseParams
190+
})
148191
}
149192

150193
it('should create corresponding resources when s3 GetObject proxy is given with a path key', () => {
@@ -156,10 +199,10 @@ describe('#compileMethodsToS3()', () => {
156199
})
157200

158201
it('should create corresponding resources when s3 GetObject proxy is given with a static key', () => {
159-
testGetObject('myKey', 'myKey')
202+
testGetObject('myKey', "'myKey'")
160203
})
161204

162-
const testPutObject = (key, pathRequestParam) => {
205+
const testPutObject = (key, keyRequestParam) => {
163206
const http = {
164207
path: 's3',
165208
method: 'post',
@@ -171,18 +214,48 @@ describe('#compileMethodsToS3()', () => {
171214
}
172215

173216
const requestParams = {
174-
'integration.request.path.object': pathRequestParam,
175-
'integration.request.path.bucket': { Ref: 'MyBucket' },
217+
'method.request.header.Content-Type': true
218+
}
219+
if (keyRequestParam.startsWith('method.')) {
220+
requestParams[keyRequestParam] = true
221+
}
222+
223+
const intRequestParams = {
224+
'integration.request.path.object': keyRequestParam,
225+
'integration.request.path.bucket': {
226+
'Fn::Sub': [
227+
"'${bucket}'",
228+
{
229+
bucket: {
230+
Ref: 'MyBucket'
231+
}
232+
}
233+
]
234+
},
176235
'integration.request.header.x-amz-acl': "'authenticated-read'",
177236
'integration.request.header.Content-Type': 'method.request.header.Content-Type'
178237
}
179238

180239
const responseParams = {
240+
'method.response.header.Content-Type': true,
241+
'method.response.header.Content-Length': true
242+
}
243+
244+
const intResponseParams = {
181245
'method.response.header.Content-Type': 'integration.response.header.Content-Type',
182246
'method.response.header.Content-Length': 'integration.response.header.Content-Length'
183247
}
184248

185-
testSingleProxy(http, 'ApiGatewayMethods3Post', 'POST', 'PUT', requestParams, responseParams)
249+
testSingleProxy({
250+
http,
251+
logicalId: 'ApiGatewayMethods3Post',
252+
method: 'POST',
253+
intMethod: 'PUT',
254+
requestParams,
255+
intRequestParams,
256+
responseParams,
257+
intResponseParams
258+
})
186259
}
187260

188261
it('should create corresponding resources when s3 PutObject proxy is given with a path key', () => {
@@ -194,10 +267,10 @@ describe('#compileMethodsToS3()', () => {
194267
})
195268

196269
it('should create corresponding resources when s3 PutObject proxy is given with a static key', () => {
197-
testPutObject('myKey', 'myKey')
270+
testPutObject('myKey', "'myKey'")
198271
})
199272

200-
const testDeleteObject = (key, pathRequestParam) => {
273+
const testDeleteObject = (key, keyRequestParam) => {
201274
const http = {
202275
path: 's3',
203276
method: 'delete',
@@ -208,24 +281,45 @@ describe('#compileMethodsToS3()', () => {
208281
key
209282
}
210283

211-
const requestParams = {
212-
'integration.request.path.object': pathRequestParam,
213-
'integration.request.path.bucket': { Ref: 'MyBucket' }
284+
const requestParams = {}
285+
if (keyRequestParam.startsWith('method.')) {
286+
requestParams[keyRequestParam] = true
287+
}
288+
289+
const intRequestParams = {
290+
'integration.request.path.object': keyRequestParam,
291+
'integration.request.path.bucket': {
292+
'Fn::Sub': [
293+
"'${bucket}'",
294+
{
295+
bucket: {
296+
Ref: 'MyBucket'
297+
}
298+
}
299+
]
300+
}
214301
}
215302

216303
const responseParams = {
304+
'method.response.header.Content-Type': true,
305+
'method.response.header.Date': true
306+
}
307+
308+
const intResponseParams = {
217309
'method.response.header.Content-Type': 'integration.response.header.Content-Type',
218310
'method.response.header.Date': 'integration.response.header.Date'
219311
}
220312

221-
testSingleProxy(
313+
testSingleProxy({
222314
http,
223-
'ApiGatewayMethods3Delete',
224-
'DELETE',
225-
'DELETE',
315+
logicalId: 'ApiGatewayMethods3Delete',
316+
method: 'DELETE',
317+
intMethod: 'DELETE',
226318
requestParams,
227-
responseParams
228-
)
319+
intRequestParams,
320+
responseParams,
321+
intResponseParams
322+
})
229323
}
230324

231325
it('should create corresponding resources when s3 DeleteObject proxy is given with a path key', () => {
@@ -237,7 +331,7 @@ describe('#compileMethodsToS3()', () => {
237331
})
238332

239333
it('should create corresponding resources when s3 DeleteObject proxy is given with a static key', () => {
240-
testDeleteObject('myKey', 'myKey')
334+
testDeleteObject('myKey', "'myKey'")
241335
})
242336

243337
it('should create corresponding resources when a s3 proxy is given with cors', async () => {
@@ -287,7 +381,10 @@ describe('#compileMethodsToS3()', () => {
287381
Type: 'AWS::ApiGateway::Method',
288382
Properties: {
289383
HttpMethod: 'POST',
290-
RequestParameters: {},
384+
RequestParameters: {
385+
'method.request.header.Content-Type': true,
386+
'method.request.path.key': true
387+
},
291388
AuthorizationType: 'NONE',
292389
ApiKeyRequired: false,
293390
ResourceId: { Ref: 'ApiGatewayResourceS3' },
@@ -299,11 +396,20 @@ describe('#compileMethodsToS3()', () => {
299396
Uri: {
300397
'Fn::Sub': ['arn:aws:apigateway:${AWS::Region}:s3:path/{bucket}/{object}', {}]
301398
},
302-
PassthroughBehavior: 'NEVER',
399+
PassthroughBehavior: 'WHEN_NO_MATCH',
303400
RequestParameters: {
304401
'integration.request.header.Content-Type': 'method.request.header.Content-Type',
305402
'integration.request.header.x-amz-acl': "'authenticated-read'",
306-
'integration.request.path.bucket': { Ref: 'MyBucket' },
403+
'integration.request.path.bucket': {
404+
'Fn::Sub': [
405+
"'${bucket}'",
406+
{
407+
bucket: {
408+
Ref: 'MyBucket'
409+
}
410+
}
411+
]
412+
},
307413
'integration.request.path.object': 'method.request.path.key'
308414
},
309415
IntegrationResponses: [
@@ -334,17 +440,21 @@ describe('#compileMethodsToS3()', () => {
334440
},
335441
MethodResponses: [
336442
{
337-
ResponseParameters: { 'method.response.header.Access-Control-Allow-Origin': "'*'" },
443+
ResponseParameters: {
444+
'method.response.header.Access-Control-Allow-Origin': true,
445+
'method.response.header.Content-Type': true,
446+
'method.response.header.Content-Length': true
447+
},
338448
ResponseModels: {},
339449
StatusCode: 200
340450
},
341451
{
342-
ResponseParameters: { 'method.response.header.Access-Control-Allow-Origin': "'*'" },
452+
ResponseParameters: { 'method.response.header.Access-Control-Allow-Origin': true },
343453
ResponseModels: {},
344454
StatusCode: 400
345455
},
346456
{
347-
ResponseParameters: { 'method.response.header.Access-Control-Allow-Origin': "'*'" },
457+
ResponseParameters: { 'method.response.header.Access-Control-Allow-Origin': true },
348458
ResponseModels: {},
349459
StatusCode: 500
350460
}

0 commit comments

Comments
 (0)