Skip to content

Commit d401f98

Browse files
greengeronggreengerong
committed
[Tech] clean code.
1 parent 4febb8e commit d401f98

4 files changed

Lines changed: 55 additions & 40 deletions

File tree

‎src/main/java/com/github/greengerong/PreRenderSEOFilter.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.github.greengerong;
22

33
import com.google.common.collect.Lists;
4+
import com.google.common.collect.Maps;
45

56
import javax.servlet.*;
67
import javax.servlet.http.HttpServletRequest;
@@ -41,7 +42,7 @@ protected void setPrerenderSeoService(PrerenderSeoService prerenderSeoService) {
4142
}
4243

4344
protected Map<String, String> toMap(FilterConfig filterConfig) {
44-
HashMap<String, String> config = new HashMap<String, String>();
45+
Map<String, String> config = Maps.newHashMap();
4546
for (String parameterName : PARAMETER_NAMES) {
4647
config.put(parameterName, filterConfig.getInitParameter(parameterName));
4748
}

‎src/main/java/com/github/greengerong/PrerenderConfig.java‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@
1717
import java.util.List;
1818
import java.util.Map;
1919

20+
import static org.apache.commons.lang.StringUtils.isNotBlank;
21+
2022
public class PrerenderConfig {
2123
private final static Logger log = LoggerFactory.getLogger(PrerenderConfig.class);
24+
public static final String PRERENDER_IO_SERVICE_URL = "http://service.prerender.io/";
2225
private Map<String, String> config;
2326

2427
public PrerenderConfig(Map<String, String> config) {
@@ -27,7 +30,7 @@ public PrerenderConfig(Map<String, String> config) {
2730

2831
public PreRenderEventHandler getEventHandler() {
2932
final String preRenderEventHandler = config.get("preRenderEventHandler");
30-
if (StringUtils.isNotBlank(preRenderEventHandler)) {
33+
if (isNotBlank(preRenderEventHandler)) {
3134
try {
3235
return (PreRenderEventHandler) Class.forName(preRenderEventHandler).newInstance();
3336
} catch (Exception e) {
@@ -38,19 +41,20 @@ public PreRenderEventHandler getEventHandler() {
3841
}
3942

4043
public CloseableHttpClient getHttpClient() {
41-
HttpClientBuilder builder = HttpClients.custom();
42-
builder = configureProxy(builder);
43-
builder = configureTimeout(builder);
44-
builder = builder.setConnectionManager(new PoolingHttpClientConnectionManager());
44+
HttpClientBuilder builder = HttpClients.custom()
45+
.setConnectionManager(new PoolingHttpClientConnectionManager());
46+
47+
configureProxy(builder);
48+
configureTimeout(builder);
4549
return builder.build();
4650
}
4751

4852
private HttpClientBuilder configureProxy(HttpClientBuilder builder) {
4953
final String proxy = config.get("proxy");
50-
if (StringUtils.isNotBlank(proxy)) {
54+
if (isNotBlank(proxy)) {
5155
final int proxyPort = Integer.parseInt(config.get("proxyPort"));
5256
DefaultProxyRoutePlanner routePlanner = new DefaultProxyRoutePlanner(new HttpHost(proxy, proxyPort));
53-
builder = builder.setRoutePlanner(routePlanner);
57+
builder.setRoutePlanner(routePlanner);
5458
}
5559
return builder;
5660
}
@@ -59,7 +63,7 @@ private HttpClientBuilder configureTimeout(HttpClientBuilder builder) {
5963
final String socketTimeout = getSocketTimeout();
6064
if (socketTimeout != null) {
6165
RequestConfig config = RequestConfig.custom().setSocketTimeout(Integer.parseInt(socketTimeout)).build();
62-
builder = builder.setDefaultRequestConfig(config);
66+
builder.setDefaultRequestConfig(config);
6367
}
6468
return builder;
6569
}
@@ -80,20 +84,20 @@ public List<String> getCrawlerUserAgents() {
8084
List<String> crawlerUserAgents = Lists.newArrayList("googlebot", "yahoo", "bingbot", "baiduspider",
8185
"facebookexternalhit", "twitterbot", "rogerbot", "linkedinbot", "embedly");
8286
final String crawlerUserAgentsFromConfig = config.get("crawlerUserAgents");
83-
if (StringUtils.isNotBlank(crawlerUserAgentsFromConfig)) {
87+
if (isNotBlank(crawlerUserAgentsFromConfig)) {
8488
crawlerUserAgents.addAll(Arrays.asList(crawlerUserAgentsFromConfig.trim().split(",")));
8589
}
8690

8791
return crawlerUserAgents;
8892
}
8993

9094
public List<String> getExtensionsToIgnore() {
91-
List<String> extensionsToIgnore = Lists.newArrayList(".xml",".js", ".css", ".less", ".png", ".jpg", ".jpeg",
95+
List<String> extensionsToIgnore = Lists.newArrayList(".xml", ".js", ".css", ".less", ".png", ".jpg", ".jpeg",
9296
".gif", ".pdf", ".doc", ".txt", ".zip", ".mp3", ".rar", ".exe", ".wmv", ".doc", ".avi", ".ppt", ".mpg",
9397
".mpeg", ".tif", ".wav", ".mov", ".psd", ".ai", ".xls", ".mp4", ".m4a", ".swf", ".dat", ".dmg",
9498
".iso", ".flv", ".m4v", ".torrent");
9599
final String extensionsToIgnoreFromConfig = config.get("extensionsToIgnore");
96-
if (StringUtils.isNotBlank(extensionsToIgnoreFromConfig)) {
100+
if (isNotBlank(extensionsToIgnoreFromConfig)) {
97101
extensionsToIgnore.addAll(Arrays.asList(extensionsToIgnoreFromConfig.trim().split(",")));
98102
}
99103

@@ -102,22 +106,22 @@ public List<String> getExtensionsToIgnore() {
102106

103107
public List<String> getWhitelist() {
104108
final String whitelist = config.get("whitelist");
105-
if (StringUtils.isNotBlank(whitelist)) {
109+
if (isNotBlank(whitelist)) {
106110
return Arrays.asList(whitelist.trim().split(","));
107111
}
108112
return null;
109113
}
110114

111115
public List<String> getBlacklist() {
112116
final String blacklist = config.get("blacklist");
113-
if (StringUtils.isNotBlank(blacklist)) {
117+
if (isNotBlank(blacklist)) {
114118
return Arrays.asList(blacklist.trim().split(","));
115119
}
116120
return null;
117121
}
118122

119123
public String getPrerenderServiceUrl() {
120124
final String prerenderServiceUrl = config.get("prerenderServiceUrl");
121-
return StringUtils.isNotBlank(prerenderServiceUrl) ? prerenderServiceUrl : "http://service.prerender.io/";
125+
return isNotBlank(prerenderServiceUrl) ? prerenderServiceUrl : PRERENDER_IO_SERVICE_URL;
122126
}
123127
}

‎src/main/java/com/github/greengerong/PrerenderSeoService.java‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.github.greengerong;
22

3+
import com.google.common.base.Function;
34
import com.google.common.base.Predicate;
45
import org.apache.commons.lang.StringUtils;
56
import org.apache.http.*;
@@ -20,15 +21,18 @@
2021
import java.io.PrintWriter;
2122
import java.net.URI;
2223
import java.net.URISyntaxException;
24+
import java.util.Arrays;
2325
import java.util.Enumeration;
2426
import java.util.List;
2527
import java.util.Map;
2628
import java.util.regex.Pattern;
2729

2830
import static com.google.common.collect.FluentIterable.from;
31+
import static org.apache.commons.lang.StringUtils.isNotBlank;
32+
import static org.apache.http.HttpHeaders.CONTENT_LENGTH;
33+
import static org.apache.http.HttpHeaders.HOST;
2934

3035
public class PrerenderSeoService {
31-
public static final int HTTP_OK = 200;
3236
private final static Logger log = LoggerFactory.getLogger(PrerenderSeoService.class);
3337
/**
3438
* These are the "hop-by-hop" headers that should not be copied.
@@ -37,6 +41,7 @@ public class PrerenderSeoService {
3741
* approach does case insensitive lookup faster.
3842
*/
3943
private static final HeaderGroup hopByHopHeaders;
44+
public static final String ESCAPED_FRAGMENT_KEY = "_escaped_fragment_";
4045
private CloseableHttpClient httpClient;
4146
private PrerenderConfig prerenderConfig;
4247
private PreRenderEventHandler preRenderEventHandler;
@@ -90,7 +95,7 @@ private boolean shouldShowPrerenderedPage(HttpServletRequest request) throws URI
9095
final String url = getRequestURL(request);
9196
final String referer = request.getHeader("Referer");
9297

93-
log.trace("checking request for " + url + " from User-Agent " + userAgent + " and referer " + referer);
98+
log.trace(String.format("checking request for %s from User-Agent %s and referer %s", url, userAgent, referer));
9499

95100
if (!HttpGet.METHOD_NAME.equals(request.getMethod())) {
96101
log.trace("Request is not HTTP GET; intercept: no");
@@ -153,14 +158,14 @@ private void copyRequestHeaders(HttpServletRequest servletRequest, HttpRequest p
153158
while (enumerationOfHeaderNames.hasMoreElements()) {
154159
String headerName = (String) enumerationOfHeaderNames.nextElement();
155160
//Instead the content-length is effectively set via InputStreamEntity
156-
if (!headerName.equalsIgnoreCase(HttpHeaders.CONTENT_LENGTH) && !hopByHopHeaders.containsHeader(headerName)) {
161+
if (!headerName.equalsIgnoreCase(CONTENT_LENGTH) && !hopByHopHeaders.containsHeader(headerName)) {
157162
Enumeration<?> headers = servletRequest.getHeaders(headerName);
158163
while (headers.hasMoreElements()) {//sometimes more than one value
159164
String headerValue = (String) headers.nextElement();
160165
// In case the proxy host is running multiple virtual servers,
161166
// rewrite the Host header to ensure that we get content from
162167
// the correct virtual server
163-
if (headerName.equalsIgnoreCase(HttpHeaders.HOST)) {
168+
if (headerName.equalsIgnoreCase(HOST)) {
164169
HttpHost host = URIUtils.extractHost(new URI(prerenderConfig.getPrerenderServiceUrl()));
165170
headerValue = host.getHostName();
166171
if (host.getPort() != -1) {
@@ -194,12 +199,19 @@ private String getApiUrl(String url) {
194199
/**
195200
* Copy proxied response headers back to the servlet client.
196201
*/
197-
private void copyResponseHeaders(HttpResponse proxyResponse, HttpServletResponse servletResponse) {
198-
for (Header header : proxyResponse.getAllHeaders()) {
199-
if (!hopByHopHeaders.containsHeader(header.getName())) {
202+
private void copyResponseHeaders(HttpResponse proxyResponse, final HttpServletResponse servletResponse) {
203+
from(Arrays.asList(proxyResponse.getAllHeaders())).filter(new Predicate<Header>() {
204+
@Override
205+
public boolean apply(Header header) {
206+
return !hopByHopHeaders.containsHeader(header.getName());
207+
}
208+
}).transform(new Function<Header, Boolean>() {
209+
@Override
210+
public Boolean apply(Header header) {
200211
servletResponse.addHeader(header.getName(), header.getValue());
212+
return true;
201213
}
202-
}
214+
}).toList();
203215
}
204216

205217
private String getResponseHtml(HttpResponse proxyResponse)
@@ -234,7 +246,7 @@ protected void closeQuietly(Closeable closeable) {
234246
}
235247

236248
private boolean hasEscapedFragment(HttpServletRequest request) {
237-
return request.getParameterMap().containsKey("_escaped_fragment_");
249+
return request.getParameterMap().containsKey(ESCAPED_FRAGMENT_KEY);
238250
}
239251

240252
private boolean isInBlackList(final String url, final String referer, List<String> blacklist) {
@@ -280,7 +292,7 @@ public boolean apply(String regex) {
280292
private boolean beforeRender(HttpServletRequest request, HttpServletResponse response) throws IOException {
281293
if (preRenderEventHandler != null) {
282294
final String html = preRenderEventHandler.beforeRender(request);
283-
if (StringUtils.isNotBlank(html)) {
295+
if (isNotBlank(html)) {
284296
final PrintWriter writer = response.getWriter();
285297
writer.write(html);
286298
writer.flush();
@@ -323,17 +335,14 @@ private String afterRender(HttpServletRequest clientRequest, HttpServletResponse
323335
private void withPrerenderToken(HttpRequest proxyRequest) {
324336
final String token = prerenderConfig.getPrerenderToken();
325337
//for new version prerender with token.
326-
if (StringUtils.isNotBlank(token)) {
338+
if (isNotBlank(token)) {
327339
proxyRequest.addHeader("X-Prerender-Token", token);
328340
}
329341
}
330342

331343
private String getFullUrl(HttpServletRequest request) {
332344
final String url = getRequestURL(request);
333345
final String queryString = request.getQueryString();
334-
if (queryString != null) {
335-
return url + '?' + queryString;
336-
}
337-
return url;
346+
return isNotBlank(queryString) ? String.format("%s?%s", url, queryString) : url;
338347
}
339348
}

‎src/test/java/com/github/greengerong/PreRenderSEOFilterTest.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,11 @@
1919
import javax.servlet.http.HttpServletRequest;
2020
import javax.servlet.http.HttpServletResponse;
2121
import java.io.PrintWriter;
22-
import java.io.StringWriter;
2322
import java.util.Enumeration;
2423
import java.util.HashMap;
2524

25+
import static com.github.greengerong.PrerenderSeoService.ESCAPED_FRAGMENT_KEY;
26+
import static org.apache.http.HttpStatus.SC_OK;
2627
import static org.mockito.Mockito.*;
2728

2829
@RunWith(MockitoJUnitRunner.class)
@@ -99,9 +100,9 @@ public void should_handle_when_url_with_escaped_fragment_() throws Exception {
99100
when(httpClient.execute(httpGet)).thenReturn(httpResponse);
100101
when(httpResponse.getStatusLine()).thenReturn(statusLine);
101102
final HashMap<String, String> map = Maps.newHashMap();
102-
map.put("_escaped_fragment_", "");
103+
map.put(ESCAPED_FRAGMENT_KEY, "");
103104
when(servletRequest.getParameterMap()).thenReturn(map);
104-
when(statusLine.getStatusCode()).thenReturn(PrerenderSeoService.HTTP_OK);
105+
when(statusLine.getStatusCode()).thenReturn(SC_OK);
105106
when(httpResponse.getAllHeaders()).thenReturn(new Header[0]);
106107
when(servletResponse.getWriter()).thenReturn(printWriter);
107108

@@ -203,9 +204,9 @@ public void should_handle_when_user_agent_is_crawler_and_url_is_not_resource_and
203204
when(httpClient.execute(httpGet)).thenReturn(httpResponse);
204205
when(httpResponse.getStatusLine()).thenReturn(statusLine);
205206
final HashMap<String, String> map = Maps.newHashMap();
206-
map.put("_escaped_fragment_", "");
207+
map.put(ESCAPED_FRAGMENT_KEY, "");
207208
when(servletRequest.getParameterMap()).thenReturn(map);
208-
when(statusLine.getStatusCode()).thenReturn(PrerenderSeoService.HTTP_OK);
209+
when(statusLine.getStatusCode()).thenReturn(SC_OK);
209210
when(httpResponse.getAllHeaders()).thenReturn(new Header[0]);
210211
when(servletResponse.getWriter()).thenReturn(printWriter);
211212
//when
@@ -233,7 +234,7 @@ public void should_handle_when_every_thing_is_ok_but_prerender_server_response_i
233234
when(httpClient.execute(httpGet)).thenReturn(httpResponse);
234235
when(httpResponse.getStatusLine()).thenReturn(statusLine);
235236
final HashMap<String, String> map = Maps.newHashMap();
236-
map.put("_escaped_fragment_", "");
237+
map.put(ESCAPED_FRAGMENT_KEY, "");
237238
when(servletRequest.getParameterMap()).thenReturn(map);
238239
when(statusLine.getStatusCode()).thenReturn(NOT_FOUND);
239240
when(httpResponse.getAllHeaders()).thenReturn(new Header[0]);
@@ -270,9 +271,9 @@ public void should_handle_when_user_agent_is_crawler_and_url_is_not_resource_and
270271
when(httpClient.execute(httpGet)).thenReturn(httpResponse);
271272
when(httpResponse.getStatusLine()).thenReturn(statusLine);
272273
final HashMap<String, String> map = Maps.newHashMap();
273-
map.put("_escaped_fragment_", "");
274+
map.put(ESCAPED_FRAGMENT_KEY, "");
274275
when(servletRequest.getParameterMap()).thenReturn(map);
275-
when(statusLine.getStatusCode()).thenReturn(PrerenderSeoService.HTTP_OK);
276+
when(statusLine.getStatusCode()).thenReturn(SC_OK);
276277
when(httpResponse.getAllHeaders()).thenReturn(new Header[0]);
277278
when(servletResponse.getWriter()).thenReturn(printWriter);
278279

@@ -304,9 +305,9 @@ public void should_use_request_url_from_custom_header_if_available() throws Exce
304305
when(httpClient.execute(httpGet)).thenReturn(httpResponse);
305306
when(httpResponse.getStatusLine()).thenReturn(statusLine);
306307
final HashMap<String, String> map = Maps.newHashMap();
307-
map.put("_escaped_fragment_", "");
308+
map.put(ESCAPED_FRAGMENT_KEY, "");
308309
when(servletRequest.getParameterMap()).thenReturn(map);
309-
when(statusLine.getStatusCode()).thenReturn(PrerenderSeoService.HTTP_OK);
310+
when(statusLine.getStatusCode()).thenReturn(SC_OK);
310311
when(httpResponse.getAllHeaders()).thenReturn(new Header[0]);
311312
when(servletResponse.getWriter()).thenReturn(printWriter);
312313

0 commit comments

Comments
 (0)