Skip to content

Commit

Permalink
Apply comments
Browse files Browse the repository at this point in the history
  • Loading branch information
seonWKim committed May 18, 2024
1 parent 93a37e9 commit a15ef81
Show file tree
Hide file tree
Showing 11 changed files with 190 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import com.linecorp.armeria.internal.server.annotation.AnnotatedValueResolver.ResolverContext;
import com.linecorp.armeria.server.HttpService;
import com.linecorp.armeria.server.HttpServiceOptions;
import com.linecorp.armeria.server.HttpServiceOptionsBuilder;
import com.linecorp.armeria.server.Route;
import com.linecorp.armeria.server.RoutingContext;
import com.linecorp.armeria.server.ServiceRequestContext;
Expand Down Expand Up @@ -113,7 +114,6 @@ final class DefaultAnnotatedService implements AnnotatedService {
@Nullable
private final String name;

@Nullable
private final HttpServiceOptions options;

DefaultAnnotatedService(Object object, Method method,
Expand Down Expand Up @@ -182,16 +182,14 @@ final class DefaultAnnotatedService implements AnnotatedService {
// following must be called only after method.setAccessible(true)
methodHandle = asMethodHandle(method, object);

final HttpServiceOption httpServiceOption = AnnotationUtil.findFirst(method, HttpServiceOption.class);
HttpServiceOption httpServiceOption = AnnotationUtil.findFirst(method, HttpServiceOption.class);
if (httpServiceOption == null) {
httpServiceOption = AnnotationUtil.findFirst(object.getClass(), HttpServiceOption.class);
}
if (httpServiceOption != null) {
options = HttpServiceOptions.builder()
.requestTimeoutMillis(httpServiceOption.requestTimeoutMillis())
.maxRequestLength(httpServiceOption.maxRequestLength())
.requestAutoAbortDelayMillis(
httpServiceOption.requestAutoAbortDelayMillis())
.build();
options = buildHttpServiceOptions(httpServiceOption);
} else {
options = null;
options = HttpServiceOptions.of();
}
}

Expand Down Expand Up @@ -238,6 +236,20 @@ private static void warnIfHttpResponseArgumentExists(Type returnType,
}
}

private HttpServiceOptions buildHttpServiceOptions(HttpServiceOption httpServiceOption) {
final HttpServiceOptionsBuilder builder = HttpServiceOptions.builder();
if (httpServiceOption.requestTimeoutMillis() >= 0) {
builder.requestTimeoutMillis(httpServiceOption.requestTimeoutMillis());
}
if (httpServiceOption.maxRequestLength() >= 0) {
builder.maxRequestLength(httpServiceOption.maxRequestLength());
}
if (httpServiceOption.requestAutoAbortDelayMillis() >= 0) {
builder.requestAutoAbortDelayMillis(httpServiceOption.requestAutoAbortDelayMillis());
}
return builder.build();
}

@Override
public String name() {
return name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.Request;
import com.linecorp.armeria.common.Response;
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.annotation.UnstableApi;

/**
Expand Down Expand Up @@ -76,7 +75,6 @@ default ExchangeType exchangeType(RoutingContext routingContext) {
/**
* Returns the {@link HttpServiceOptions} of this {@link HttpService}.
*/
@Nullable
@UnstableApi
default HttpServiceOptions options() {
return HttpServiceOptions.of();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,15 @@ public long requestTimeoutMillis() {
}

/**
* Returns the server-side maximum length of a request.
* Returns the server-side maximum length of a request. {@code -1} if not set.
*/
public long maxRequestLength() {
return maxRequestLength;
}

/**
* Returns the amount of time to wait before aborting an {@link HttpRequest} when its corresponding
* {@link HttpResponse} is complete.
* {@link HttpResponse} is complete. {@code -1} if not set.
*/
public long requestAutoAbortDelayMillis() {
return requestAutoAbortDelayMillis;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.linecorp.armeria.server;

import static com.google.common.base.Preconditions.checkArgument;

import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.HttpResponse;

Expand All @@ -33,6 +35,8 @@ public final class HttpServiceOptionsBuilder {
* Returns the server-side timeout of a request in milliseconds.
*/
public HttpServiceOptionsBuilder requestTimeoutMillis(long requestTimeoutMillis) {
checkArgument(requestTimeoutMillis >= 0, "requestTimeoutMillis: %s (expected: >= 0)",
requestTimeoutMillis);
this.requestTimeoutMillis = requestTimeoutMillis;
return this;
}
Expand All @@ -41,6 +45,7 @@ public HttpServiceOptionsBuilder requestTimeoutMillis(long requestTimeoutMillis)
* Returns the server-side maximum length of a request.
*/
public HttpServiceOptionsBuilder maxRequestLength(long maxRequestLength) {
checkArgument(maxRequestLength >= 0, "maxRequestLength: %s (expected: >= 0)", maxRequestLength);
this.maxRequestLength = maxRequestLength;
return this;
}
Expand All @@ -50,6 +55,8 @@ public HttpServiceOptionsBuilder maxRequestLength(long maxRequestLength) {
* {@link HttpResponse} is complete.
*/
public HttpServiceOptionsBuilder requestAutoAbortDelayMillis(long requestAutoAbortDelayMillis) {
checkArgument(requestAutoAbortDelayMillis >= 0, "requestAutoAbortDelayMillis: %s (expected: >= 0)",
requestAutoAbortDelayMillis);
this.requestAutoAbortDelayMillis = requestAutoAbortDelayMillis;
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,6 @@ final class ServiceConfigBuilder implements ServiceConfigSetters {
this.service = requireNonNull(service, "service");

final HttpServiceOptions options = service.options();
if (options == null) {
return;
}

if (options.requestTimeoutMillis() != -1) {
requestTimeoutMillis = options.requestTimeoutMillis();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import com.linecorp.armeria.common.HttpResponse;

/**
* Decorates a {@link HttpService} to be treated as {@link TransientService} without inheritance.
* Decorates an {@link HttpService} to be treated as {@link TransientService} without inheritance.
*/
final class WrappingTransientHttpService extends SimpleDecoratingHttpService implements TransientHttpService {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
* An annotation used to configure {@link HttpServiceOptions} of an {@link HttpService}.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target({ ElementType.METHOD })
@Target({ ElementType.METHOD, ElementType.TYPE })
public @interface HttpServiceOption {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@

import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.server.annotation.Get;
import com.linecorp.armeria.server.annotation.HttpServiceOption;

/**
* The priority of configurations from highest to lowest:
Expand Down Expand Up @@ -132,53 +130,4 @@ public HttpServiceOptions options() {
httpServiceOptions.requestAutoAbortDelayMillis());
}
}

@Test
void httpServiceOptionAnnotationShouldBeAppliedWhenConfigured() {
final class TestAnnotatedService {
@HttpServiceOption(
requestTimeoutMillis = 11111,
maxRequestLength = 1111,
requestAutoAbortDelayMillis = 111
)
@Get("/test1")
public HttpResponse test1() {
return HttpResponse.of("OK");
}

@Get("/test2")
public HttpResponse test2() {
return HttpResponse.of("OK");
}
}

final long DEFAULT_REQUEST_TIMEOUT_MILLIS = 30001;
final long DEFAULT_MAX_REQUEST_LENGTH = 30002;
final long DEFAULT_REQUEST_AUTO_ABORT_DELAY_MILLIS = 30003;

try (Server server = Server.builder().annotatedService(new TestAnnotatedService())
.requestTimeoutMillis(DEFAULT_REQUEST_TIMEOUT_MILLIS)
.maxRequestLength(DEFAULT_MAX_REQUEST_LENGTH)
.requestAutoAbortDelayMillis(DEFAULT_REQUEST_AUTO_ABORT_DELAY_MILLIS)
.build()) {
final ServiceConfig sc1 = server.serviceConfigs()
.stream()
.filter(s -> s.route().paths().contains("/test1"))
.findFirst().orElse(null);

assertThat(sc1).isNotNull();
assertThat(sc1.requestTimeoutMillis()).isEqualTo(11111);
assertThat(sc1.maxRequestLength()).isEqualTo(1111);
assertThat(sc1.requestAutoAbortDelayMillis()).isEqualTo(111);

// default values should be applied
final ServiceConfig sc2 = server.serviceConfigs()
.stream()
.filter(s -> s.route().paths().contains("/test2"))
.findFirst().get();
assertThat(sc2.requestTimeoutMillis()).isEqualTo(DEFAULT_REQUEST_TIMEOUT_MILLIS);
assertThat(sc2.maxRequestLength()).isEqualTo(DEFAULT_MAX_REQUEST_LENGTH);
assertThat(sc2.requestAutoAbortDelayMillis()).isEqualTo(DEFAULT_REQUEST_AUTO_ABORT_DELAY_MILLIS);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.util.List;

Expand Down Expand Up @@ -141,6 +142,7 @@ static RoutingContext create(VirtualHost virtualHost, String path, @Nullable Str

static VirtualHost virtualHost() {
final HttpService service = mock(HttpService.class);
when(service.options()).thenReturn(HttpServiceOptions.of());
final Server server = Server.builder()
.virtualHost("example.com")
.serviceUnder("/", service)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/*
* Copyright 2024 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.annotation;

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;

import org.junit.jupiter.api.Test;

import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.server.Server;
import com.linecorp.armeria.server.ServiceConfig;

public class HttpServiceOptionTest {
@Test
void httpServiceOptionAnnotationShouldBeAppliedWhenConfiguredAtMethodLevel() {
final class TestAnnotatedService {
@HttpServiceOption(
requestTimeoutMillis = 11111,
maxRequestLength = 1111
)
@Get("/test1")
public HttpResponse test1() {
return HttpResponse.of("OK");
}

@Get("/test2")
public HttpResponse test2() {
return HttpResponse.of("OK");
}
}

final long DEFAULT_REQUEST_TIMEOUT_MILLIS = 30001;
final long DEFAULT_MAX_REQUEST_LENGTH = 30002;
final long DEFAULT_REQUEST_AUTO_ABORT_DELAY_MILLIS = 30003;

try (Server server = Server.builder().annotatedService(new TestAnnotatedService())
.requestTimeoutMillis(DEFAULT_REQUEST_TIMEOUT_MILLIS)
.maxRequestLength(DEFAULT_MAX_REQUEST_LENGTH)
.requestAutoAbortDelayMillis(DEFAULT_REQUEST_AUTO_ABORT_DELAY_MILLIS)
.build()) {
final ServiceConfig sc1 = server.serviceConfigs()
.stream()
.filter(s -> s.route().paths().contains("/test1"))
.findFirst().orElse(null);

assertThat(sc1).isNotNull();
assertThat(sc1.requestTimeoutMillis()).isEqualTo(11111);
assertThat(sc1.maxRequestLength()).isEqualTo(1111);
assertThat(sc1.requestAutoAbortDelayMillis()).isEqualTo(DEFAULT_REQUEST_AUTO_ABORT_DELAY_MILLIS);

// default values should be applied
final ServiceConfig sc2 = server.serviceConfigs()
.stream()
.filter(s -> s.route().paths().contains("/test2"))
.findFirst().get();
assertThat(sc2.requestTimeoutMillis()).isEqualTo(DEFAULT_REQUEST_TIMEOUT_MILLIS);
assertThat(sc2.maxRequestLength()).isEqualTo(DEFAULT_MAX_REQUEST_LENGTH);
assertThat(sc2.requestAutoAbortDelayMillis()).isEqualTo(DEFAULT_REQUEST_AUTO_ABORT_DELAY_MILLIS);
}
}

@Test
void httpServiceOptionAnnotationShouldBeAppliedWhenConfiguredAtClassLevel() {
@HttpServiceOption(
requestTimeoutMillis = 11111,
maxRequestLength = 1111
)
final class TestAnnotatedService {
@Get("/test")
public HttpResponse test() {
return HttpResponse.of("OK");
}
}

final long DEFAULT_REQUEST_TIMEOUT_MILLIS = 30001;
final long DEFAULT_MAX_REQUEST_LENGTH = 30002;
final long DEFAULT_REQUEST_AUTO_ABORT_DELAY_MILLIS = 30003;

try (Server server = Server.builder().annotatedService(new TestAnnotatedService())
.requestTimeoutMillis(DEFAULT_REQUEST_TIMEOUT_MILLIS)
.maxRequestLength(DEFAULT_MAX_REQUEST_LENGTH)
.requestAutoAbortDelayMillis(DEFAULT_REQUEST_AUTO_ABORT_DELAY_MILLIS)
.build()) {
final ServiceConfig sc = server.serviceConfigs()
.stream()
.filter(s -> s.route().paths().contains("/test"))
.findFirst().orElse(null);

assertThat(sc).isNotNull();
assertThat(sc.requestTimeoutMillis()).isEqualTo(11111);
assertThat(sc.maxRequestLength()).isEqualTo(1111);
assertThat(sc.requestAutoAbortDelayMillis()).isEqualTo(DEFAULT_REQUEST_AUTO_ABORT_DELAY_MILLIS);
}
}

@Test
void httpServiceOptionAnnotationAtMethodLevelShouldOverrideHttpServiceOptionAtClassLevel() {
@HttpServiceOption(
requestTimeoutMillis = 11111,
maxRequestLength = 1111,
requestAutoAbortDelayMillis = 111
)
final class TestAnnotatedService {
@HttpServiceOption(
requestTimeoutMillis = 22222,
maxRequestLength = 2222,
requestAutoAbortDelayMillis = 222
)
@Get("/test")
public HttpResponse test() {
return HttpResponse.of("OK");
}
}

try (Server server = Server.builder()
.annotatedService(new TestAnnotatedService())
.build()) {
final ServiceConfig sc = server.serviceConfigs()
.stream()
.filter(s -> s.route().paths().contains("/test"))
.findFirst().orElse(null);

assertThat(sc).isNotNull();
assertThat(sc.requestTimeoutMillis()).isEqualTo(22222);
assertThat(sc.maxRequestLength()).isEqualTo(2222);
assertThat(sc.requestAutoAbortDelayMillis()).isEqualTo(222);
}
}
}

0 comments on commit a15ef81

Please sign in to comment.