Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Portability to Java 6 support #74

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>io.cdap.http</groupId>
<artifactId>netty-http</artifactId>
<version>1.3.0</version>
<version>1.3.1</version>
<packaging>bundle</packaging>
<name>Netty based path router</name>
<description>Netty based path router</description>
Expand Down Expand Up @@ -62,14 +62,14 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<surefire.redirectTestOutputToFile>true</surefire.redirectTestOutputToFile>

<rs-api.version>2.0</rs-api.version>
<netty.version>4.1.16.Final</netty.version>
<slf4j.version>1.7.5</slf4j.version>
<rs-api.version>2.1.1</rs-api.version>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this related? Does Java6 support require changing the dependency versions?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java6 does not required it, but it is the last supported version and Java6 still compatible.

<netty.version>4.1.36.Final</netty.version>
<slf4j.version>1.7.26</slf4j.version>
<findbugs.version>2.0.1</findbugs.version>
<beanutils.version>1.8.3</beanutils.version>
<junit.version>4.11</junit.version>
<logback.version>1.0.9</logback.version>
<gson.version>2.2.4</gson.version>
<junit.version>4.12</junit.version>
<logback.version>1.2.3</logback.version>
<gson.version>2.8.5</gson.version>
</properties>

<dependencies>
Expand Down Expand Up @@ -135,10 +135,27 @@
<version>${gson.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.cdap.http</groupId>
<artifactId>netty-http</artifactId>
<version>1.2.0</version>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>animal-sniffer-maven-plugin</artifactId>
<version>1.16</version>
<configuration>
<signature>
<groupId>org.codehaus.mojo.signature</groupId>
<artifactId>java16</artifactId>
<version>1.0</version>
</signature>
</configuration>
</plugin>
<!-- Compiler -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
Expand Down Expand Up @@ -174,7 +191,6 @@
</execution>
</executions>
</plugin>

<!-- Checkstyle -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
Expand Down
7 changes: 3 additions & 4 deletions src/main/java/io/cdap/http/AbstractHttpResponder.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package io.cdap.http;

import io.cdap.http.internal.InternalUtil;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.handler.codec.http.DefaultHttpHeaders;
Expand All @@ -27,15 +28,13 @@
import java.io.File;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;

/**
* Base implementation of {@link HttpResponder} to simplify child implementations.
*/
public abstract class AbstractHttpResponder implements HttpResponder {

protected static final String OCTET_STREAM_TYPE = "application/octet-stream";

@Override
public void sendJson(HttpResponseStatus status, String jsonString) {
sendString(status, jsonString, new DefaultHttpHeaders().add(HttpHeaderNames.CONTENT_TYPE.toString(),
Expand All @@ -53,7 +52,7 @@ public void sendString(HttpResponseStatus status, String data, HttpHeaders heade
sendStatus(status, headers);
return;
}
ByteBuf buffer = Unpooled.wrappedBuffer(StandardCharsets.UTF_8.encode(data));
ByteBuf buffer = Unpooled.wrappedBuffer(InternalUtil.UTF_8.encode(data));
sendContent(status, buffer, addContentTypeIfMissing(new DefaultHttpHeaders().add(headers),
"text/plain; charset=utf-8"));
}
Expand All @@ -80,7 +79,7 @@ public void sendBytes(HttpResponseStatus status, ByteBuffer buffer, HttpHeaders
}

@Override
public void sendFile(File file) throws IOException {
public void sendFile(File file) throws Throwable {
sendFile(file, EmptyHttpHeaders.INSTANCE);
}

Expand Down
6 changes: 4 additions & 2 deletions src/main/java/io/cdap/http/HttpResponder.java
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,9 @@ public interface HttpResponder {
*
* @param file The file to send
* @throws IOException if failed to open and read the file
* @throws Throwable
*/
void sendFile(File file) throws IOException;
void sendFile(File file) throws IOException, Throwable;

/**
* Sends a file content back to client with response status 200. Default content type is "application/octet-stream",
Expand All @@ -134,8 +135,9 @@ public interface HttpResponder {
* @param file The file to send
* @param headers additional headers to send with the response.
* @throws IOException if failed to open and read the file
* @throws Throwable
*/
void sendFile(File file, HttpHeaders headers) throws IOException;
void sendFile(File file, HttpHeaders headers) throws IOException, Throwable;

/**
* Sends response back to client. The response body is produced by the given {@link BodyProducer}. This method
Expand Down
27 changes: 11 additions & 16 deletions src/main/java/io/cdap/http/NettyHttpService.java
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ private NettyHttpService(String serviceName,
this.workerThreadPoolSize = workerThreadPoolSize;
this.execThreadPoolSize = execThreadPoolSize;
this.execThreadKeepAliveSecs = execThreadKeepAliveSecs;
this.channelConfigs = new HashMap<>(channelConfigs);
this.childChannelConfigs = new HashMap<>(childChannelConfigs);
this.channelConfigs = new HashMap<ChannelOption, Object>(channelConfigs);
this.childChannelConfigs = new HashMap<ChannelOption, Object>(childChannelConfigs);
this.rejectedExecutionHandler = rejectedExecutionHandler;
this.resourceHandler = new HttpResourceHandler(httpHandlers, handlerHooks, urlRewriter, exceptionHandler);
this.handlerContext = new BasicHandlerContext(this.resourceHandler);
Expand All @@ -152,10 +152,9 @@ public static Builder builder(String serviceName) {

/**
* Starts the HTTP service.
*
* @throws Exception if the service failed to started
* @throws Throwable
*/
public synchronized void start() throws Exception {
public synchronized void start() throws Throwable {
if (state == State.RUNNING) {
LOG.debug("Ignore start() call on HTTP service {} since it has already been started.", serviceName);
return;
Expand Down Expand Up @@ -193,7 +192,6 @@ public synchronized void start() throws Exception {
shutdownExecutorGroups(0, 5, TimeUnit.SECONDS, eventExecutorGroup);
}
} catch (Throwable t2) {
t.addSuppressed(t2);
}
state = State.FAILED;
throw t;
Expand Down Expand Up @@ -224,10 +222,9 @@ public boolean isSSLEnabled() {
/**
* Stops the HTTP service gracefully and release all resources. Same as calling {@link #stop(long, long, TimeUnit)}
* with {@code 0} second quiet period and {@code 5} seconds timeout.
*
* @throws Exception if there is exception raised during shutdown.
* @throws Throwable
*/
public void stop() throws Exception {
public void stop() throws Throwable {
stop(0, 5, TimeUnit.SECONDS);
}

Expand All @@ -239,9 +236,9 @@ public void stop() throws Exception {
* {@linkplain EventExecutorGroup#shutdown()}
* regardless if a task was submitted during the quiet period
* @param unit the unit of {@code quietPeriod} and {@code timeout}
* @throws Exception if there is exception raised during shutdown.
* @throws Throwable
*/
public synchronized void stop(long quietPeriod, long timeout, TimeUnit unit) throws Exception {
public synchronized void stop(long quietPeriod, long timeout, TimeUnit unit) throws Throwable {
if (state == State.STOPPED) {
LOG.debug("Ignore stop() call on HTTP service {} since it has already been stopped.", serviceName);
return;
Expand Down Expand Up @@ -377,7 +374,7 @@ protected void initChannel(SocketChannel ch) throws Exception {
*/
private void shutdownExecutorGroups(long quietPeriod, long timeout, TimeUnit unit, EventExecutorGroup...groups) {
Exception ex = null;
List<Future<?>> futures = new ArrayList<>();
List<Future<?>> futures = new ArrayList<Future<?>>();
for (EventExecutorGroup group : groups) {
if (group == null) {
continue;
Expand All @@ -391,8 +388,6 @@ private void shutdownExecutorGroups(long quietPeriod, long timeout, TimeUnit uni
} catch (Exception e) {
if (ex == null) {
ex = e;
} else {
ex.addSuppressed(e);
}
}
}
Expand Down Expand Up @@ -449,8 +444,8 @@ protected Builder(String serviceName) {
rejectedExecutionHandler = DEFAULT_REJECTED_EXECUTION_HANDLER;
httpChunkLimit = DEFAULT_HTTP_CHUNK_LIMIT;
port = 0;
channelConfigs = new HashMap<>();
childChannelConfigs = new HashMap<>();
channelConfigs = new HashMap<ChannelOption, Object>();
childChannelConfigs = new HashMap<ChannelOption, Object>();
channelConfigs.put(ChannelOption.SO_BACKLOG, DEFAULT_CONNECTION_BACKLOG);
sslHandlerFactory = null;
exceptionHandler = new ExceptionHandler();
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/io/cdap/http/SSLHandlerFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,16 @@ public SSLHandlerFactory(SslContext sslContext) {
}

private static KeyStore getKeyStore(File keyStore, String keyStorePassword) throws Exception {
try (InputStream is = new FileInputStream(keyStore)) {
InputStream is = null;
try {
is = new FileInputStream(keyStore);
KeyStore ks = KeyStore.getInstance("JKS");
ks.load(is, keyStorePassword.toCharArray());
return ks;
} finally {
if (is != null) {
is.close();
}
}
}

Expand Down
8 changes: 3 additions & 5 deletions src/main/java/io/cdap/http/internal/BasicHttpResponder.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
import java.io.File;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -105,7 +104,7 @@ public void sendContent(HttpResponseStatus status, ByteBuf content, HttpHeaders
}

@Override
public void sendFile(File file, HttpHeaders headers) throws IOException {
public void sendFile(File file, HttpHeaders headers) throws Throwable {
HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
addContentTypeIfMissing(response.headers().add(headers), OCTET_STREAM_TYPE);

Expand Down Expand Up @@ -136,7 +135,6 @@ public void sendFile(File file, HttpHeaders headers) throws IOException {
try {
raf.close();
} catch (IOException ex) {
t.addSuppressed(ex);
}
throw t;
}
Expand All @@ -152,7 +150,7 @@ public void sendContent(HttpResponseStatus status, final BodyProducer bodyProduc
// Response with error and close the connection
sendContent(
HttpResponseStatus.INTERNAL_SERVER_ERROR,
Unpooled.copiedBuffer("Failed to determined content length. Cause: " + t.getMessage(), StandardCharsets.UTF_8),
Unpooled.copiedBuffer("Failed to determined content length. Cause: " + t.getMessage(), InternalUtil.UTF_8),
new DefaultHttpHeaders()
.set(HttpHeaderNames.CONNECTION, HttpHeaderValues.CLOSE)
.set(HttpHeaderNames.CONTENT_TYPE, "text/plain; charset=utf-8"));
Expand Down Expand Up @@ -197,7 +195,7 @@ boolean isResponded() {
private ChannelFutureListener createBodyProducerCompletionListener(final BodyProducer bodyProducer) {
return new ChannelFutureListener() {
@Override
public void operationComplete(ChannelFuture future) throws Exception {
public void operationComplete(ChannelFuture future) {
if (!future.isSuccess()) {
callBodyProducerHandleError(bodyProducer, future.cause());
channel.close();
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/io/cdap/http/internal/HandlerException.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import io.netty.handler.codec.http.HttpUtil;
import io.netty.handler.codec.http.HttpVersion;

import java.nio.charset.StandardCharsets;

/**
*Creating Http Response for Exception messages.
*/
Expand All @@ -48,7 +46,7 @@ final class HandlerException extends Exception {

HttpResponse createFailureResponse() {
FullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, failureStatus,
Unpooled.copiedBuffer(message, StandardCharsets.UTF_8));
Unpooled.copiedBuffer(message, InternalUtil.UTF_8));
HttpUtil.setContentLength(response, response.content().readableBytes());
return response;
}
Expand Down
11 changes: 8 additions & 3 deletions src/main/java/io/cdap/http/internal/HttpResourceHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import javax.annotation.Nullable;
import javax.ws.rs.DELETE;
import javax.ws.rs.GET;
import javax.ws.rs.OPTIONS;
import javax.ws.rs.POST;
import javax.ws.rs.PUT;
import javax.ws.rs.Path;
Expand Down Expand Up @@ -135,7 +136,7 @@ public HttpResourceHandler(Iterable<? extends HttpHandler> handlers, Iterable<?
* @return String representation of HttpMethod from annotations or emptyString as a default.
*/
private Set<HttpMethod> getHttpMethods(Method method) {
Set<HttpMethod> httpMethods = new HashSet<>();
Set<HttpMethod> httpMethods = new HashSet<HttpMethod>();

if (method.isAnnotationPresent(GET.class)) {
httpMethods.add(HttpMethod.GET);
Expand All @@ -149,6 +150,9 @@ private Set<HttpMethod> getHttpMethods(Method method) {
if (method.isAnnotationPresent(DELETE.class)) {
httpMethods.add(HttpMethod.DELETE);
}
if (method.isAnnotationPresent(OPTIONS.class)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems unrelated to Java6

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes indeed, but we need to have OPTIONS support.

httpMethods.add(HttpMethod.OPTIONS);
}

return Collections.unmodifiableSet(httpMethods);
}
Expand Down Expand Up @@ -310,7 +314,8 @@ public HttpMethodInfo getDestinationMethod(HttpRequest request, HttpResponder re

LOG.trace("Routable destinations for request {}: {}", requestUri, routableDestinations);
Iterable<String> requestUriParts = splitAndOmitEmpty(requestUri, '/');
List<PatternPathRouterWithGroups.RoutableDestination<HttpResourceModel>> matchedDestinations = new ArrayList<>();
List<PatternPathRouterWithGroups.RoutableDestination<HttpResourceModel>> matchedDestinations =
new ArrayList<PatternPathRouterWithGroups.RoutableDestination<HttpResourceModel>>();
long maxScore = 0;

for (PatternPathRouterWithGroups.RoutableDestination<HttpResourceModel> destination : routableDestinations) {
Expand Down Expand Up @@ -390,7 +395,7 @@ public void destroy(HandlerContext context) {
}

private static <T> List<T> copyOf(Iterable<? extends T> iterable) {
List<T> list = new ArrayList<>();
List<T> list = new ArrayList<T>();
for (T item : iterable) {
list.add(item);
}
Expand Down
11 changes: 7 additions & 4 deletions src/main/java/io/cdap/http/internal/HttpResourceModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@
public final class HttpResourceModel {

private static final Set<Class<? extends Annotation>> SUPPORTED_PARAM_ANNOTATIONS =
Collections.unmodifiableSet(new HashSet<>(Arrays.asList(PathParam.class, QueryParam.class, HeaderParam.class)));
Collections.unmodifiableSet(new HashSet<Class<? extends Annotation>>
(Arrays.asList(PathParam.class, QueryParam.class, HeaderParam.class)));

private final Set<HttpMethod> httpMethods;
private final String path;
Expand Down Expand Up @@ -211,13 +212,15 @@ private List<Map<Class<? extends Annotation>, ParameterInfo<?>>> createParameter
return Collections.emptyList();
}

List<Map<Class<? extends Annotation>, ParameterInfo<?>>> result = new ArrayList<>();
List<Map<Class<? extends Annotation>, ParameterInfo<?>>> result =
new ArrayList<Map<Class<? extends Annotation>, ParameterInfo<?>>>();
Type[] parameterTypes = method.getGenericParameterTypes();
Annotation[][] parameterAnnotations = method.getParameterAnnotations();

for (int i = 2; i < parameterAnnotations.length; i++) {
Annotation[] annotations = parameterAnnotations[i];
Map<Class<? extends Annotation>, ParameterInfo<?>> paramAnnotations = new IdentityHashMap<>();
Map<Class<? extends Annotation>, ParameterInfo<?>> paramAnnotations =
new IdentityHashMap<Class<? extends Annotation>, ParameterInfo<?>>();

for (Annotation annotation : annotations) {
Class<? extends Annotation> annotationType = annotation.annotationType();
Expand Down Expand Up @@ -266,7 +269,7 @@ private static final class ParameterInfo<T> {
private final Converter<T, Object> converter;

static <V> ParameterInfo<V> create(Annotation annotation, @Nullable Converter<V, Object> converter) {
return new ParameterInfo<>(annotation, converter);
return new ParameterInfo<V>(annotation, converter);
}

private ParameterInfo(Annotation annotation, @Nullable Converter<T, Object> converter) {
Expand Down
Loading