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

Adding AnnotatedServiceBuilder to add service level settings to annotated service #2180

Closed
wants to merge 57 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
c0a2b95
adding source files.added annotatedServiceBuilder and all the related…
sivaalli Oct 8, 2019
61d2e63
adding minor changes and adding few more tests
sivaalli Oct 9, 2019
310f3aa
adding java docs and finishing up the test cases.
sivaalli Oct 10, 2019
f6ac6e3
making requested changes.
sivaalli Oct 18, 2019
41ee119
making non-overridden methods to package-private in AbstractServiceBu…
sivaalli Oct 18, 2019
a41ebbf
implementing suggestions. renaming *serviceBuilder to *serviceConfigS…
sivaalli Oct 21, 2019
bc8f636
fixing a method name issue
sivaalli Oct 21, 2019
c760adf
Support multiple Path annotations
jrhee17 Oct 23, 2019
e8c424a
Allow configuring a thread name pattern to treat as non-request threa…
anuraaga Oct 23, 2019
287fecb
adding java docs, adding VirtualHostAnnotatedServiceBindingBuilder, m…
sivaalli Oct 23, 2019
8974da9
Some annotated service methods are run from blocking task execu… (#2187)
heowc Oct 24, 2019
00c0430
Fix NPE in Http1ClientCodec (#2210)
minwoox Oct 24, 2019
f696b08
Implement ArmeriaServerHttpRequest.getRemoteAddress() (#2208)
eunchan-kim Oct 24, 2019
610e29e
Have yarn task default to frozen lockfile to not update yarn file wit…
anuraaga Oct 24, 2019
c97554f
changing http client from apache to armeria
sivaalli Oct 24, 2019
3d579d4
Make sure spring configuration runs if serverbuilder consumers present
anuraaga Oct 25, 2019
393a8e1
Fix document
hirakida Oct 25, 2019
d29bfc1
Fix a wrong usage in documentation (#2206)
minwoox Oct 25, 2019
ec9e09f
Add hamcrest 2.2 dependency and upgrade json-unit 2.8.1 to 2.10… (#2207)
ikhoon Oct 25, 2019
5eb5ac3
Set useOpenSsl in Flags lazily to reduce server startup time (#2184)
mumgmangmange Oct 25, 2019
e8716d6
Update Dependencies (#2205)
minwoox Oct 25, 2019
8d0de0f
Update the project version to 0.95.1-SNAPSHOT
minwoox Oct 25, 2019
cc13492
adding tests for VirtualHostAnnotatedServiceBindingBuilder
sivaalli Oct 25, 2019
cb87c4c
some minor modifications
sivaalli Oct 25, 2019
8b930dc
using armeria http client in tests
sivaalli Oct 26, 2019
c82c8a6
Support multiple Path annotations
jrhee17 Oct 23, 2019
22c2a1b
Allow configuring a thread name pattern to treat as non-request threa…
anuraaga Oct 23, 2019
2e01ae9
Some annotated service methods are run from blocking task execu… (#2187)
heowc Oct 24, 2019
9d8be35
Fix NPE in Http1ClientCodec (#2210)
minwoox Oct 24, 2019
c17c5a9
Implement ArmeriaServerHttpRequest.getRemoteAddress() (#2208)
eunchan-kim Oct 24, 2019
128a718
Have yarn task default to frozen lockfile to not update yarn file wit…
anuraaga Oct 24, 2019
7463e45
Make sure spring configuration runs if serverbuilder consumers present
anuraaga Oct 25, 2019
c41c90a
Fix document
hirakida Oct 25, 2019
32823fc
Fix a wrong usage in documentation (#2206)
minwoox Oct 25, 2019
bfbefc0
Add hamcrest 2.2 dependency and upgrade json-unit 2.8.1 to 2.10… (#2207)
ikhoon Oct 25, 2019
a9f30cb
Set useOpenSsl in Flags lazily to reduce server startup time (#2184)
mumgmangmange Oct 25, 2019
b5ca885
Update Dependencies (#2205)
minwoox Oct 25, 2019
b03c589
Update the project version to 0.95.1-SNAPSHOT
minwoox Oct 25, 2019
65143a4
addressing comments. implemented all requested changes.
sivaalli Oct 30, 2019
fd7139c
adding clean to appveyor.build.sh
sivaalli Oct 30, 2019
036acd3
fixing compilation error from upstream changes
sivaalli Oct 30, 2019
96c842e
adding source files.added annotatedServiceBuilder and all the related…
sivaalli Oct 8, 2019
039ce56
adding minor changes and adding few more tests
sivaalli Oct 9, 2019
adf9455
adding java docs and finishing up the test cases.
sivaalli Oct 10, 2019
a044a62
making requested changes.
sivaalli Oct 18, 2019
73a341c
making non-overridden methods to package-private in AbstractServiceBu…
sivaalli Oct 18, 2019
d5200d5
implementing suggestions. renaming *serviceBuilder to *serviceConfigS…
sivaalli Oct 21, 2019
c344c9d
fixing a method name issue
sivaalli Oct 21, 2019
f83747e
adding java docs, adding VirtualHostAnnotatedServiceBindingBuilder, m…
sivaalli Oct 23, 2019
a07e45d
changing http client from apache to armeria
sivaalli Oct 24, 2019
aa20842
adding tests for VirtualHostAnnotatedServiceBindingBuilder
sivaalli Oct 25, 2019
3b949fc
some minor modifications
sivaalli Oct 25, 2019
3ca055c
using armeria http client in tests
sivaalli Oct 26, 2019
dd5937a
addressing comments. implemented all requested changes.
sivaalli Oct 30, 2019
a6c1c5e
adding clean to appveyor.build.sh
sivaalli Oct 30, 2019
ef1fc1f
fixing compilation error from upstream changes
sivaalli Oct 30, 2019
ecd8aed
Merge branch 'master' of https://github.com/sivaalli/armeria
sivaalli Oct 30, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -16,22 +16,16 @@

package com.linecorp.armeria.server;

import static com.linecorp.armeria.server.ServerConfig.validateMaxRequestLength;
import static com.linecorp.armeria.server.ServerConfig.validateRequestTimeoutMillis;
import static java.util.Objects.requireNonNull;

import java.nio.charset.Charset;
import java.time.Duration;
import java.util.List;
import java.util.function.Function;

import javax.annotation.Nullable;

import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.logging.ContentPreviewer;
import com.linecorp.armeria.common.logging.ContentPreviewerFactory;
import com.linecorp.armeria.internal.ArmeriaHttpUtil;
import com.linecorp.armeria.server.logging.AccessLogWriter;

/**
Expand All @@ -40,175 +34,81 @@
* @see ServiceBindingBuilder
* @see VirtualHostServiceBindingBuilder
*/
abstract class AbstractServiceBindingBuilder extends AbstractBindingBuilder {

@Nullable
private Long requestTimeoutMillis;
@Nullable
private Long maxRequestLength;
@Nullable
private Boolean verboseResponses;
@Nullable
private ContentPreviewerFactory requestContentPreviewerFactory;
@Nullable
private ContentPreviewerFactory responseContentPreviewerFactory;
@Nullable
private Function<Service<HttpRequest, HttpResponse>, Service<HttpRequest, HttpResponse>> decorator;
@Nullable
private AccessLogWriter accessLogWriter;
private boolean shutdownAccessLogWriterOnStop;

/**
* Sets the timeout of an HTTP request. If not set, {@link VirtualHost#requestTimeoutMillis()}
* is used.
*
* @param requestTimeout the timeout. {@code 0} disables the timeout.
*/
abstract class AbstractServiceBindingBuilder extends AbstractBindingBuilder implements
Copy link
Member

Choose a reason for hiding this comment

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

It'll be a lot much prettier if we break the line before implements.

ServiceConfigSetters {

private final DefaultServiceConfigSetters defaultServiceConfigSetters = new DefaultServiceConfigSetters();

@Override
public AbstractServiceBindingBuilder requestTimeout(Duration requestTimeout) {
return requestTimeoutMillis(requireNonNull(requestTimeout, "requestTimeout").toMillis());
}

/**
* Sets the timeout of an HTTP request in milliseconds. If not set,
* {@link VirtualHost#requestTimeoutMillis()} is used.
*
* @param requestTimeoutMillis the timeout in milliseconds. {@code 0} disables the timeout.
*/
@Override
public AbstractServiceBindingBuilder requestTimeoutMillis(long requestTimeoutMillis) {
this.requestTimeoutMillis = validateRequestTimeoutMillis(requestTimeoutMillis);
defaultServiceConfigSetters.requestTimeoutMillis(requestTimeoutMillis);
return this;
}

/**
* Sets the maximum allowed length of an HTTP request. If not set,
* {@link VirtualHost#maxRequestLength()} is used.
*
* @param maxRequestLength the maximum allowed length. {@code 0} disables the length limit.
*/
@Override
public AbstractServiceBindingBuilder maxRequestLength(long maxRequestLength) {
this.maxRequestLength = validateMaxRequestLength(maxRequestLength);
defaultServiceConfigSetters.maxRequestLength(maxRequestLength);
return this;
}

/**
* Sets whether the verbose response mode is enabled. When enabled, the service response will contain
* the exception type and its full stack trace, which may be useful for debugging while potentially
* insecure. When disabled, the service response will not expose such server-side details to the client.
* If not set, {@link VirtualHost#verboseResponses()} is used.
*/
@Override
public AbstractServiceBindingBuilder verboseResponses(boolean verboseResponses) {
this.verboseResponses = verboseResponses;
defaultServiceConfigSetters.verboseResponses(verboseResponses);
return this;
}

/**
* Sets the {@link ContentPreviewerFactory} for an HTTP request of a {@link Service}.
* If not set, {@link VirtualHost#requestContentPreviewerFactory()}
* is used.
*/
@Override
public AbstractServiceBindingBuilder requestContentPreviewerFactory(ContentPreviewerFactory factory) {
requestContentPreviewerFactory = requireNonNull(factory, "factory");
defaultServiceConfigSetters.requestContentPreviewerFactory(factory);
return this;
}

/**
* Sets the {@link ContentPreviewerFactory} for an HTTP response of a {@link Service}.
* If not set, {@link VirtualHost#responseContentPreviewerFactory()}
* is used.
*/
@Override
public AbstractServiceBindingBuilder responseContentPreviewerFactory(ContentPreviewerFactory factory) {
responseContentPreviewerFactory = requireNonNull(factory, "factory");
defaultServiceConfigSetters.responseContentPreviewerFactory(factory);
return this;
}

/**
* Sets the {@link ContentPreviewerFactory} for creating a {@link ContentPreviewer} which produces the
* preview with the maximum {@code length} limit for an HTTP request/response of the {@link Service}.
* The previewer is enabled only if the content type of an HTTP request/response meets
* any of the following conditions:
* <ul>
* <li>when it matches {@code text/*} or {@code application/x-www-form-urlencoded}</li>
* <li>when its charset has been specified</li>
* <li>when its subtype is {@code "xml"} or {@code "json"}</li>
* <li>when its subtype ends with {@code "+xml"} or {@code "+json"}</li>
* </ul>
* @param length the maximum length of the preview.
*/
@Override
public AbstractServiceBindingBuilder contentPreview(int length) {
return contentPreview(length, ArmeriaHttpUtil.HTTP_DEFAULT_CONTENT_CHARSET);
defaultServiceConfigSetters.contentPreview(length);
return this;
}

/**
* Sets the {@link ContentPreviewerFactory} for creating a {@link ContentPreviewer} which produces the
* preview with the maximum {@code length} limit for an HTTP request/response of a {@link Service}.
* The previewer is enabled only if the content type of an HTTP request/response meets any of
* the following conditions:
* <ul>
* <li>when it matches {@code text/*} or {@code application/x-www-form-urlencoded}</li>
* <li>when its charset has been specified</li>
* <li>when its subtype is {@code "xml"} or {@code "json"}</li>
* <li>when its subtype ends with {@code "+xml"} or {@code "+json"}</li>
* </ul>
* @param length the maximum length of the preview
* @param defaultCharset the default charset used when a charset is not specified in the
* {@code "content-type"} header
*/
@Override
public AbstractServiceBindingBuilder contentPreview(int length, Charset defaultCharset) {
return contentPreviewerFactory(ContentPreviewerFactory.ofText(length, defaultCharset));
}

/**
* Sets the {@link ContentPreviewerFactory} for an HTTP request/response of a {@link Service}.
*/
@Override
public AbstractServiceBindingBuilder contentPreviewerFactory(ContentPreviewerFactory factory) {
requestContentPreviewerFactory(factory);
responseContentPreviewerFactory(factory);
return this;
}

/**
* Sets the format of this {@link Service}'s access log. The specified {@code accessLogFormat} would be
* parsed by {@link AccessLogWriter#custom(String)}.
*/
@Override
public AbstractServiceBindingBuilder accessLogFormat(String accessLogFormat) {
return accessLogWriter(AccessLogWriter.custom(requireNonNull(accessLogFormat, "accessLogFormat")),
true);
}

/**
* Sets the access log writer of this {@link Service}. If not set, {@link ServerConfig#accessLogWriter()}
* is used.
*
* @param shutdownOnStop whether to shut down the {@link AccessLogWriter} when the {@link Server} stops
*/
@Override
public AbstractServiceBindingBuilder accessLogWriter(AccessLogWriter accessLogWriter,
boolean shutdownOnStop) {
this.accessLogWriter = requireNonNull(accessLogWriter, "accessLogWriter");
shutdownAccessLogWriterOnStop = shutdownOnStop;
defaultServiceConfigSetters.accessLogWriter(accessLogWriter, shutdownOnStop);
return this;
}

/**
* Decorates a {@link Service} with the specified {@code decorator}.
*
* @param decorator the {@link Function} that decorates the {@link Service}
* @param <T> the type of the {@link Service} being decorated
* @param <R> the type of the {@link Service} {@code decorator} will produce
*/
@Override
public <T extends Service<HttpRequest, HttpResponse>, R extends Service<HttpRequest, HttpResponse>>
AbstractServiceBindingBuilder decorator(Function<T, R> decorator) {
requireNonNull(decorator, "decorator");

@SuppressWarnings("unchecked")
final Function<Service<HttpRequest, HttpResponse>, Service<HttpRequest, HttpResponse>> castDecorator =
(Function<Service<HttpRequest, HttpResponse>, Service<HttpRequest, HttpResponse>>) decorator;

if (this.decorator != null) {
this.decorator = this.decorator.andThen(castDecorator);
} else {
this.decorator = castDecorator;
}

defaultServiceConfigSetters.decorator(decorator);
return this;
}

Expand All @@ -218,35 +118,10 @@ final void build0(Service<HttpRequest, HttpResponse> service) {
final List<Route> routes = buildRouteList();

for (Route route : routes) {
final ServiceConfigBuilder serviceConfigBuilder =
new ServiceConfigBuilder(route, decorate(service));
if (requestTimeoutMillis != null) {
serviceConfigBuilder.requestTimeoutMillis(requestTimeoutMillis);
}
if (maxRequestLength != null) {
serviceConfigBuilder.maxRequestLength(maxRequestLength);
}
if (verboseResponses != null) {
serviceConfigBuilder.verboseResponses(verboseResponses);
}
if (requestContentPreviewerFactory != null) {
serviceConfigBuilder.requestContentPreviewerFactory(requestContentPreviewerFactory);
}
if (responseContentPreviewerFactory != null) {
serviceConfigBuilder.responseContentPreviewerFactory(responseContentPreviewerFactory);
}
if (accessLogWriter != null) {
serviceConfigBuilder.accessLogWriter(accessLogWriter, shutdownAccessLogWriterOnStop);
}
service = defaultServiceConfigSetters.decorate(service);
final ServiceConfigBuilder serviceConfigBuilder = defaultServiceConfigSetters
.toServiceConfigBuilder(route, service);
Copy link
Member

Choose a reason for hiding this comment

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

nit: indentation

Copy link
Contributor

@ikhoon ikhoon Oct 22, 2019

Choose a reason for hiding this comment

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

I think we need a new local variable here to avoid decorating a service multiple times.

final Service<HttpRequest, HttpResponse> decoratedService = defaultServiceConfigSetters.decorate(service);
final ServiceConfigBuilder serviceConfigBuilder = defaultServiceConfigSetters                                                                    
        .toServiceConfigBuilder(route, decoratedService);

serviceConfigBuilder(serviceConfigBuilder);
}
}

private Service<HttpRequest, HttpResponse> decorate(Service<HttpRequest, HttpResponse> service) {
if (decorator == null) {
return service;
}

return service.decorate(decorator);
}
}
@@ -0,0 +1,124 @@
/*
* Copyright 2019 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

package com.linecorp.armeria.server;

import static java.util.Objects.requireNonNull;

import java.nio.charset.Charset;
import java.time.Duration;
import java.util.function.Function;

import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.logging.ContentPreviewerFactory;
import com.linecorp.armeria.server.logging.AccessLogWriter;

/**
* A builder that implements {@link ServiceConfigSetters} by delegating all calls
* to {@link DefaultServiceConfigSetters}.
*/
abstract class AbstractServiceConfigSetters implements ServiceConfigSetters {

private final DefaultServiceConfigSetters defaultServiceConfigSetters = new DefaultServiceConfigSetters();

@Override
public AbstractServiceConfigSetters requestTimeout(Duration requestTimeout) {
defaultServiceConfigSetters.requestTimeout(requestTimeout);
return this;
}

@Override
public AbstractServiceConfigSetters requestTimeoutMillis(long requestTimeoutMillis) {
defaultServiceConfigSetters.requestTimeoutMillis(requestTimeoutMillis);
return this;
}

@Override
public AbstractServiceConfigSetters maxRequestLength(long maxRequestLength) {
defaultServiceConfigSetters.maxRequestLength(maxRequestLength);
return this;
}

@Override
public AbstractServiceConfigSetters verboseResponses(boolean verboseResponses) {
defaultServiceConfigSetters.verboseResponses(verboseResponses);
return this;
}

@Override
public AbstractServiceConfigSetters requestContentPreviewerFactory(ContentPreviewerFactory factory) {
defaultServiceConfigSetters.requestContentPreviewerFactory(factory);
return this;
}

@Override
public AbstractServiceConfigSetters responseContentPreviewerFactory(ContentPreviewerFactory factory) {
defaultServiceConfigSetters.responseContentPreviewerFactory(factory);
return this;
}

@Override
public AbstractServiceConfigSetters contentPreview(int length) {
defaultServiceConfigSetters.contentPreview(length);
return this;
}

@Override
public AbstractServiceConfigSetters contentPreview(int length, Charset defaultCharset) {
contentPreviewerFactory(ContentPreviewerFactory.ofText(length, defaultCharset));
return this;
}

@Override
public AbstractServiceConfigSetters contentPreviewerFactory(ContentPreviewerFactory factory) {
requestContentPreviewerFactory(factory);
responseContentPreviewerFactory(factory);
return this;
}

@Override
public AbstractServiceConfigSetters accessLogFormat(String accessLogFormat) {
return accessLogWriter(AccessLogWriter.custom(requireNonNull(accessLogFormat, "accessLogFormat")),
true);
}

@Override
public AbstractServiceConfigSetters accessLogWriter(AccessLogWriter accessLogWriter,
boolean shutdownOnStop) {
defaultServiceConfigSetters.accessLogWriter(accessLogWriter, shutdownOnStop);
return this;
}

@Override
public <T extends Service<HttpRequest, HttpResponse>, R extends Service<HttpRequest, HttpResponse>>
AbstractServiceConfigSetters decorator(Function<T, R> decorator) {
defaultServiceConfigSetters.decorator(decorator);
return this;
}

Service<HttpRequest, HttpResponse> decorate(Service<HttpRequest, HttpResponse> service) {
return defaultServiceConfigSetters.decorate(service);
}

final void build0(Route route, Service<HttpRequest, HttpResponse> service) {
final ServiceConfigBuilder serviceConfigBuilder = defaultServiceConfigSetters
.toServiceConfigBuilder(route, service);
serviceConfigBuilder(serviceConfigBuilder);
}

abstract void serviceConfigBuilder(ServiceConfigBuilder serviceConfigBuilder);
}