Skip to content

Commit eaab838

Browse files
sapessisapessi
committed
Address feedback from code review in #18 
1 parent 2ea85da commit eaab838

File tree

8 files changed

+128
-99
lines changed

8 files changed

+128
-99
lines changed

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java‎

Lines changed: 48 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import java.net.URLEncoder;
2525
import java.nio.charset.StandardCharsets;
2626
import java.util.*;
27+
import java.util.stream.Collectors;
28+
2729

2830
/**
2931
* Base HttpServletRequest object. This object exposes some utility methods to work with request values such as headers
@@ -52,8 +54,7 @@ public abstract class AwsHttpServletRequest implements HttpServletRequest {
5254
// Variables - Private
5355
//-------------------------------------------------------------
5456

55-
private Context lamdaContext;
56-
private DispatcherType dispatcherType;
57+
private Context lambdaContext;
5758
private Map<String, Object> attributes;
5859

5960

@@ -67,11 +68,8 @@ public abstract class AwsHttpServletRequest implements HttpServletRequest {
6768
* @param lambdaContext The Lambda function context. This object is used for utility methods such as log
6869
*/
6970
AwsHttpServletRequest(Context lambdaContext) {
70-
lamdaContext = lambdaContext;
71+
this.lambdaContext = lambdaContext;
7172
attributes = new HashMap<>();
72-
73-
// TODO: We are setting this to request by default
74-
dispatcherType = DispatcherType.REQUEST;
7573
}
7674

7775
//-------------------------------------------------------------
@@ -80,8 +78,7 @@ public abstract class AwsHttpServletRequest implements HttpServletRequest {
8078

8179
@Override
8280
public String getRequestedSessionId() {
83-
// TODO: Throw not implemented
84-
return null;
81+
throw new UnsupportedOperationException();
8582
}
8683

8784

@@ -188,7 +185,7 @@ public int getLocalPort() {
188185

189186
@Override
190187
public ServletContext getServletContext() {
191-
return AwsServletContext.getInstance(lamdaContext);
188+
return AwsServletContext.getInstance(lambdaContext);
192189
}
193190

194191

@@ -212,7 +209,7 @@ public AsyncContext getAsyncContext() {
212209

213210
@Override
214211
public DispatcherType getDispatcherType() {
215-
return dispatcherType;
212+
return DispatcherType.REQUEST;
216213
}
217214

218215

@@ -225,63 +222,47 @@ public DispatcherType getDispatcherType() {
225222
* @param headerValue The string value of the HTTP Cookie header
226223
* @return An array of Cookie objects from the header
227224
*/
228-
protected Cookie[] parseCookies(String headerValue) {
229-
List<Cookie> output = new ArrayList<>();
230-
231-
for (AbstractMap.SimpleEntry<String, String> entry : this.parseHeaderValue(headerValue)) {
232-
if (entry.getKey() != null) {
233-
output.add(new Cookie(entry.getKey(), entry.getValue()));
234-
}
235-
}
236-
Cookie[] returnValue = new Cookie[output.size()];
237-
return output.toArray(returnValue);
238-
}
239-
240-
241-
protected String readPathInfo(String path, String resource) {
242-
// TODO: Implement
243-
return "/";
244-
}
225+
protected Cookie[] parseCookieHeaderValue(String headerValue) {
245226

227+
List<Map.Entry<String, String>> parsedHeaders = this.parseHeaderValue(headerValue);
246228

247-
protected String readPathTranslated(String path) {
248-
// TODO: Implement
249-
return path;
229+
return parsedHeaders.stream()
230+
.filter(e -> e.getKey() != null)
231+
.map(e -> new Cookie(e.getKey(), e.getValue()))
232+
.toArray(Cookie[]::new);
250233
}
251234

252-
253235
/**
254236
* Given a map of key/values query string parameters from API Gateway, creates a query string as it would have
255237
* been in the original url.
256238
* @param parameters A Map<String, String> of query string parameters
257239
* @return The generated query string for the URI
258240
*/
259241
protected String generateQueryString(Map<String, String> parameters) {
260-
String params = null;
261-
if (parameters != null && parameters.size() > 0) {
262-
params = "";
263-
for (String key : parameters.keySet()) {
264-
String separator = params.equals("") ? "?" : "&";
265-
String queryStringKey = key;
266-
String queryStringValue = parameters.get(key);
267-
try {
268-
// if they were URLDecoded along the way we should re-encode them for the URI
269-
if (!URLEncoder.encode(queryStringKey, StandardCharsets.UTF_8.name()).equals(key)) {
270-
queryStringKey = URLEncoder.encode(queryStringKey, StandardCharsets.UTF_8.name());
271-
}
272-
if (!URLEncoder.encode(queryStringValue, StandardCharsets.UTF_8.name()).equals(queryStringValue)) {
273-
queryStringValue = URLEncoder.encode(queryStringValue, StandardCharsets.UTF_8.name());
274-
}
275-
} catch (UnsupportedEncodingException e) {
276-
// TODO: Should we stop for the exception?
277-
lamdaContext.getLogger().log("Could not URLEncode: " + queryStringKey);
278-
e.printStackTrace();
279-
}
280-
params += separator + queryStringKey + "=" + queryStringValue;
281-
}
242+
if (parameters == null || parameters.size() == 0) {
243+
return null;
282244
}
283245

284-
return params;
246+
return parameters.keySet().stream()
247+
.map(key -> {
248+
String newKey = key;
249+
String newValue = parameters.get(key);
250+
try {
251+
if (!URLEncoder.encode(newKey, StandardCharsets.UTF_8.name()).equals(newKey)) {
252+
newKey = URLEncoder.encode(key, StandardCharsets.UTF_8.name());
253+
}
254+
255+
if (!URLEncoder.encode(newValue, StandardCharsets.UTF_8.name()).equals(newValue)) {
256+
newValue = URLEncoder.encode(newValue, StandardCharsets.UTF_8.name());
257+
}
258+
} catch (UnsupportedEncodingException e) {
259+
lambdaContext.getLogger().log("Could not URLEncode: " + newKey + "\n" + e.getLocalizedMessage());
260+
e.printStackTrace();
261+
262+
}
263+
return newKey + "=" + newValue;
264+
})
265+
.collect(Collectors.joining("&"));
285266
}
286267

287268

@@ -291,20 +272,21 @@ protected String generateQueryString(Map<String, String> parameters) {
291272
* is populated. For example, The header <code>Accept: application/json; application/xml</code> will contain two
292273
* key value pairs with key null and the value set to application/json and application/xml respectively.
293274
*
294-
* @param headerContent The string value for the HTTP header
275+
* @param headerValue The string value for the HTTP header
295276
* @return A list of SimpleMapEntry objects with all of the possible values for the header.
296277
*/
297-
protected List<AbstractMap.SimpleEntry<String, String>> parseHeaderValue(String headerContent) {
298-
List<AbstractMap.SimpleEntry<String, String>> values = new ArrayList<>();
299-
if (headerContent != null) {
300-
for (String kv : headerContent.split(HEADER_VALUE_SEPARATOR)) {
301-
String[] kvSplit = kv.split(HEADER_KEY_VALUE_SEPARATOR);
302-
303-
if (kvSplit.length != 2) {
304-
values.add(new AbstractMap.SimpleEntry<>(null, kv.trim()));
305-
} else {
306-
values.add(new AbstractMap.SimpleEntry<>(kvSplit[0].trim(), kvSplit[1].trim()));
307-
}
278+
protected List<Map.Entry<String, String>> parseHeaderValue(String headerValue) {
279+
List<Map.Entry<String, String>> values = new ArrayList<>();
280+
if (headerValue == null) {
281+
return values;
282+
}
283+
for (String kv : headerValue.split(HEADER_VALUE_SEPARATOR)) {
284+
String[] kvSplit = kv.split(HEADER_KEY_VALUE_SEPARATOR);
285+
286+
if (kvSplit.length != 2) {
287+
values.add(new AbstractMap.SimpleEntry<>(null, kv.trim()));
288+
} else {
289+
values.add(new AbstractMap.SimpleEntry<>(kvSplit[0].trim(), kvSplit[1].trim()));
308290
}
309291
}
310292
return values;

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsLambdaServletContainerHandler.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@
1212
*/
1313
package com.amazonaws.serverless.proxy.internal.servlet;
1414

15-
import com.amazonaws.serverless.proxy.internal.*;
15+
import com.amazonaws.serverless.proxy.internal.ExceptionHandler;
16+
import com.amazonaws.serverless.proxy.internal.LambdaContainerHandler;
17+
import com.amazonaws.serverless.proxy.internal.RequestReader;
18+
import com.amazonaws.serverless.proxy.internal.ResponseWriter;
19+
import com.amazonaws.serverless.proxy.internal.SecurityContextWriter;
1620

1721
import javax.servlet.ServletContext;
1822
import javax.servlet.ServletException;
@@ -30,8 +34,8 @@
3034
* @param <ContainerResponseType>
3135
*/
3236
public abstract class AwsLambdaServletContainerHandler<RequestType, ResponseType,
33-
ContainerRequestType extends HttpServletRequest,
34-
ContainerResponseType extends HttpServletResponse>
37+
ContainerRequestType extends HttpServletRequest,
38+
ContainerResponseType extends HttpServletResponse>
3539
extends LambdaContainerHandler<RequestType, ResponseType, ContainerRequestType, ContainerResponseType> {
3640

3741
//-------------------------------------------------------------

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,20 @@
4444
import java.security.Principal;
4545
import java.text.ParseException;
4646
import java.text.SimpleDateFormat;
47-
import java.util.*;
47+
import java.util.ArrayList;
48+
import java.util.Arrays;
49+
import java.util.Base64;
50+
import java.util.Collection;
51+
import java.util.Collections;
52+
import java.util.Date;
53+
import java.util.Enumeration;
54+
import java.util.HashMap;
55+
import java.util.Iterator;
56+
import java.util.List;
57+
import java.util.Locale;
58+
import java.util.Map;
59+
import java.util.TreeMap;
60+
4861

4962
/**
5063
* Implementation of the <code>HttpServletRequest</code> interface that supports <code>AwsProxyRequest</code> object.
@@ -93,7 +106,7 @@ public Cookie[] getCookies() {
93106
if (cookieHeader == null) {
94107
return new Cookie[0];
95108
}
96-
return this.parseCookies(cookieHeader);
109+
return this.parseCookieHeaderValue(cookieHeader);
97110
}
98111

99112

@@ -233,21 +246,20 @@ public String getServletPath() {
233246
@Override
234247
public boolean authenticate(HttpServletResponse httpServletResponse)
235248
throws IOException, ServletException {
236-
// TODO: Throw not implemented
237-
return false;
249+
throw new UnsupportedOperationException();
238250
}
239251

240252

241253
@Override
242254
public void login(String s, String s1)
243255
throws ServletException {
244-
// TODO: Throw not implemented
256+
throw new UnsupportedOperationException();
245257
}
246258

247259
@Override
248260
public void logout()
249261
throws ServletException {
250-
// TODO: Throw not implemented
262+
throw new UnsupportedOperationException();
251263
}
252264

253265

@@ -531,7 +543,7 @@ public String getRemoteHost() {
531543

532544
@Override
533545
public Locale getLocale() {
534-
List<AbstractMap.SimpleEntry<String, String>> values = this.parseHeaderValue(
546+
List<Map.Entry<String, String>> values = this.parseHeaderValue(
535547
getHeaderCaseInsensitive(HttpHeaders.ACCEPT_LANGUAGE)
536548
);
537549
if (values.size() == 0) {
@@ -543,14 +555,14 @@ public Locale getLocale() {
543555

544556
@Override
545557
public Enumeration<Locale> getLocales() {
546-
List<AbstractMap.SimpleEntry<String, String>> values = this.parseHeaderValue(
558+
List<Map.Entry<String, String>> values = this.parseHeaderValue(
547559
getHeaderCaseInsensitive(HttpHeaders.ACCEPT_LANGUAGE)
548560
);
549561
List<Locale> locales = new ArrayList<>();
550562
if (values.size() == 0) {
551563
locales.add(Locale.getDefault());
552564
} else {
553-
for (AbstractMap.SimpleEntry<String, String> locale : values) {
565+
for (Map.Entry<String, String> locale : values) {
554566
locales.add(new Locale(locale.getValue()));
555567
}
556568
}

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsServletContext.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,14 @@
3535
import java.net.URL;
3636
import java.nio.file.Files;
3737
import java.nio.file.Paths;
38-
import java.util.*;
38+
import java.util.Collections;
39+
import java.util.Enumeration;
40+
import java.util.EventListener;
41+
import java.util.HashMap;
42+
import java.util.LinkedHashMap;
43+
import java.util.Map;
44+
import java.util.Set;
45+
3946

4047
/**
4148
* Basic implementation of the <code>ServletContext</code> object. Because this library is not a complete container
@@ -50,12 +57,13 @@ public class AwsServletContext
5057
// -------------------------------------------------------------
5158
public static final int SERVLET_API_MAJOR_VERSION = 3;
5259
public static final int SERVLET_API_MINOR_VERSION = 1;
60+
public static final String SERVER_INFO = LambdaContainerHandler.SERVER_INFO + "/" + SERVLET_API_MAJOR_VERSION + "." + SERVLET_API_MINOR_VERSION;
5361

5462

5563
//-------------------------------------------------------------
5664
// Variables - Private
5765
//-------------------------------------------------------------
58-
private static AwsServletContext instance;
66+
private Map<String, FilterHolder> filters;
5967
private Context lambdaContext;
6068
private Map<String, Object> attributes;
6169
private Map<String, String> initParameters;
@@ -64,7 +72,7 @@ public class AwsServletContext
6472
//-------------------------------------------------------------
6573
// Variables - Private - Static
6674
//-------------------------------------------------------------
67-
private Map<String, FilterHolder> filters;
75+
private static AwsServletContext instance;
6876

6977

7078
//-------------------------------------------------------------
@@ -228,7 +236,7 @@ public String getRealPath(String s) {
228236
try {
229237
absPath = new File(fileUrl.toURI()).getAbsolutePath();
230238
} catch (URISyntaxException e) {
231-
e.printStackTrace();
239+
lambdaContext.getLogger().log("Error while looking for real path: " + s + "\n" + e.getMessage());
232240
}
233241
}
234242
return absPath;
@@ -237,7 +245,7 @@ public String getRealPath(String s) {
237245

238246
@Override
239247
public String getServerInfo() {
240-
return LambdaContainerHandler.SERVER_INFO + "/" + SERVLET_API_MAJOR_VERSION + "." + SERVLET_API_MINOR_VERSION;
248+
return SERVER_INFO;
241249
}
242250

243251

@@ -380,7 +388,7 @@ public FilterRegistration.Dynamic addFilter(String name, Class<? extends Filter>
380388
} catch (ServletException e) {
381389
// TODO: There is no clear indication in the servlet specs on whether we should throw an exception here.
382390
// See JavaDoc here: http://docs.oracle.com/javaee/7/api/javax/servlet/ServletContext.html#addFilter-java.lang.String-java.lang.Class-
383-
e.printStackTrace();
391+
lambdaContext.getLogger().log("Could not register filter: " + e.getMessage());
384392
}
385393
return null;
386394
}
@@ -391,7 +399,7 @@ public <T extends Filter> T createFilter(Class<T> aClass) throws ServletExceptio
391399
try {
392400
return aClass.newInstance();
393401
} catch (InstantiationException | IllegalAccessException e) {
394-
e.printStackTrace();
402+
lambdaContext.getLogger().log("Could not initialize filter class " + aClass.getName() + "\n" + e.getMessage());
395403
throw new ServletException();
396404
}
397405
}

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainHolder.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@
1212
*/
1313
package com.amazonaws.serverless.proxy.internal.servlet;
1414

15-
import javax.servlet.*;
15+
import javax.servlet.FilterChain;
16+
import javax.servlet.ServletException;
17+
import javax.servlet.ServletRequest;
18+
import javax.servlet.ServletResponse;
19+
1620
import java.io.IOException;
1721
import java.util.ArrayList;
1822
import java.util.List;

0 commit comments

Comments
 (0)