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.

Sign up for GitHub

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

Jump to bottom

Add nullability annotations to APIs commonly used by services #119

Merged
merged 1 commit into from
Mar 11, 2019
Merged

Add nullability annotations to APIs commonly used by services #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ ext {
wingtipsVersion = '0.18.1'
backstopperVersion = '0.11.4'
fastbreakVersion = '0.10.0'
spockVersion = '0.7-groovy-2.0'
spockVersion = '1.2-groovy-2.5'
cgLibVersion = '3.1'
objenesisVersion = '2.1'
javassistVersion = '3.18.2-GA'
Expand All @@ -98,9 +98,9 @@ ext {
slf4jTestVersion = '1.1.0'
guiceVersion = '4.0'

jetbrainsAnnotationsVersion = '16.0.3'
jetbrainsAnnotationsVersion = '17.0.0'

groovyVersion = '2.4.6'
groovyVersion = '2.5.6'

restAssuredVersion = '3.0.2'

Expand Down
5 changes: 4 additions & 1 deletion riposte-archaius/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ dependencies {
project(":riposte-core"),
"com.netflix.archaius:archaius-core:$archaiusVersion"
)

compileOnly(
"org.jetbrains:annotations:$jetbrainsAnnotationsVersion",
)
testCompile (
"org.jetbrains:annotations:$jetbrainsAnnotationsVersion",
"junit:junit:$junitVersion",
"org.mockito:mockito-core:$mockitoVersion",
"io.rest-assured:rest-assured:$restAssuredVersion",
Expand Down
13 changes: 8 additions & 5 deletions riposte-archaius/src/test/java/com/nike/riposte/archaius/ArchaiusServerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import com.netflix.config.ConfigurationManager;

import org.jetbrains.annotations.NotNull;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -52,7 +53,7 @@ private ArchaiusServer generateArchaiusServer(int port) {
protected ServerConfig getServerConfig() {
return new ServerConfig() {
@Override
public Collection<Endpoint<?>> appEndpoints() {
public @NotNull Collection<@NotNull Endpoint<?>> appEndpoints() {
return Collections.singleton(new SomeEndpoint());
}

Expand Down Expand Up @@ -136,14 +137,16 @@ private static class SomeEndpoint extends StandardEndpoint<Void, String> {
static final String MATCHING_PATH = "/archaiusValue";

@Override
public Matcher requestMatcher() {
public @NotNull Matcher requestMatcher() {
return Matcher.match(MATCHING_PATH);
}

@Override
public CompletableFuture<ResponseInfo<String>> execute(RequestInfo<Void> request,
Executor longRunningTaskExecutor,
ChannelHandlerContext ctx) {
public @NotNull CompletableFuture<ResponseInfo<String>> execute(
@NotNull RequestInfo<Void> request,
@NotNull Executor longRunningTaskExecutor,
@NotNull ChannelHandlerContext ctx
) {
String value = ConfigurationManager.getConfigInstance().getString("archaiusServer.foo");
return CompletableFuture.completedFuture(
ResponseInfo.newBuilder(value).build()
Expand Down
24 changes: 17 additions & 7 deletions riposte-async-http-client/src/main/java/com/nike/riposte/util/AwsUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.ning.http.client.Response;

import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -82,7 +83,9 @@ protected AwsUtil() { /* do nothing */ }
* completes successfully). If an error occurs retrieving the region from AWS then the error will be logged and
* {@link AppInfo#UNKNOWN_VALUE} returned as the value.
*/
public static CompletableFuture<String> getAwsRegion(AsyncHttpClientHelper asyncHttpClientHelper) {
public static @NotNull CompletableFuture<@NotNull String> getAwsRegion(
@NotNull AsyncHttpClientHelper asyncHttpClientHelper
) {
return asyncHttpClientHelper.executeAsyncHttpRequest(
asyncHttpClientHelper.getRequestBuilder(AMAZON_METADATA_DOCUMENT_URL, HttpMethod.GET),
response -> {
Expand Down Expand Up @@ -127,7 +130,9 @@ public static CompletableFuture<String> getAwsRegion(AsyncHttpClientHelper async
* completes successfully). If an error occurs retrieving the instance ID from AWS then the error will be logged and
* {@link AppInfo#UNKNOWN_VALUE} returned as the value.
*/
public static CompletableFuture<String> getAwsInstanceId(AsyncHttpClientHelper asyncHttpClientHelper) {
public static @NotNull CompletableFuture<@NotNull String> getAwsInstanceId(
@NotNull AsyncHttpClientHelper asyncHttpClientHelper
) {
return asyncHttpClientHelper.executeAsyncHttpRequest(
asyncHttpClientHelper.getRequestBuilder(AMAZON_METADATA_INSTANCE_ID_URL, HttpMethod.GET),
Response::getResponseBody
Expand Down Expand Up @@ -157,7 +162,9 @@ public static CompletableFuture<String> getAwsInstanceId(AsyncHttpClientHelper a
* See {@link #getAppInfoFutureWithAwsInfo(String, String, AsyncHttpClientHelper)} for more details on how the
* {@link AppInfo} returned by the {@link CompletableFuture} will be structured.
*/
public static CompletableFuture<AppInfo> getAppInfoFutureWithAwsInfo(AsyncHttpClientHelper asyncHttpClientHelper) {
public static @NotNull CompletableFuture<@NotNull AppInfo> getAppInfoFutureWithAwsInfo(
@NotNull AsyncHttpClientHelper asyncHttpClientHelper
) {
String appId = AppInfoImpl.detectAppId();
if (appId == null)
throw new IllegalStateException(
Expand Down Expand Up @@ -187,8 +194,11 @@ public static CompletableFuture<AppInfo> getAppInfoFutureWithAwsInfo(AsyncHttpCl
* services will be used to determine {@link AppInfo#dataCenter()} and {@link AppInfo#instanceId()}. If those AWS
* metadata calls fail for any reason then {@link AppInfo#UNKNOWN_VALUE} will be used instead.
*/
public static CompletableFuture<AppInfo> getAppInfoFutureWithAwsInfo(String appId, String environment,
AsyncHttpClientHelper asyncHttpClientHelper) {
public static @NotNull CompletableFuture<@NotNull AppInfo> getAppInfoFutureWithAwsInfo(
@NotNull String appId,
@NotNull String environment,
@NotNull AsyncHttpClientHelper asyncHttpClientHelper
) {
if ("local".equalsIgnoreCase(environment) || "compiletimetest".equalsIgnoreCase(environment)) {
AppInfo localAppInfo = AppInfoImpl.createLocalInstance(appId);

Expand All @@ -202,8 +212,8 @@ public static CompletableFuture<AppInfo> getAppInfoFutureWithAwsInfo(String appI
}

// Not local, so assume AWS.
CompletableFuture<String> dataCenterFuture = getAwsRegion(asyncHttpClientHelper);
CompletableFuture<String> instanceIdFuture = getAwsInstanceId(asyncHttpClientHelper);
CompletableFuture<@NotNull String> dataCenterFuture = getAwsRegion(asyncHttpClientHelper);
CompletableFuture<@NotNull String> instanceIdFuture = getAwsInstanceId(asyncHttpClientHelper);

return CompletableFuture.allOf(dataCenterFuture, instanceIdFuture).thenApply((aVoid) -> {

Expand Down
11 changes: 7 additions & 4 deletions ...rc/test/java/com/nike/riposte/componenttest/VerifyAsyncHttpClientHelperComponentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.tngtech.java.junit.dataprovider.DataProvider;
import com.tngtech.java.junit.dataprovider.DataProviderRunner;

import org.jetbrains.annotations.NotNull;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
Expand Down Expand Up @@ -285,13 +286,15 @@ private static class TestEndpoint extends StandardEndpoint<String, String> {
new ApiErrorBase("MISSING_EXPECTED_HEADER", 42, "Missing expected header", 400);

@Override
public Matcher requestMatcher() {
public @NotNull Matcher requestMatcher() {
return Matcher.match(MATCHING_PATH);
}

@Override
public CompletableFuture<ResponseInfo<String>> execute(
RequestInfo<String> request, Executor longRunningTaskExecutor, ChannelHandlerContext ctx
public @NotNull CompletableFuture<ResponseInfo<String>> execute(
@NotNull RequestInfo<String> request,
@NotNull Executor longRunningTaskExecutor,
@NotNull ChannelHandlerContext ctx
) {
if (!EXPECTED_REQUEST_PAYLOAD.equals(request.getContent()))
throw new ApiException(MISSING_EXPECTED_REQ_PAYLOAD);
Expand Down Expand Up @@ -329,7 +332,7 @@ private AppServerConfigForTesting(Collection<Endpoint<?>> endpoints, int port) {
}

@Override
public Collection<Endpoint<?>> appEndpoints() {
public @NotNull Collection<@NotNull Endpoint<?>> appEndpoints() {
return endpoints;
}

Expand Down
5 changes: 4 additions & 1 deletion riposte-auth/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ dependencies {
project(":riposte-spi"),
project(":riposte-core")
)

compileOnly(
"org.jetbrains:annotations:$jetbrainsAnnotationsVersion",
)
testCompile (
"org.jetbrains:annotations:$jetbrainsAnnotationsVersion",
"org.assertj:assertj-core:$assertJVersion",
"junit:junit:$junitVersion",
"org.mockito:mockito-core:$mockitoVersion",
Expand Down
16 changes: 12 additions & 4 deletions ...th/src/main/java/com/nike/riposte/server/error/validation/BasicAuthSecurityValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
import com.nike.riposte.server.http.Endpoint;
import com.nike.riposte.server.http.RequestInfo;

import org.jetbrains.annotations.NotNull;

import java.util.Base64;
import java.util.Collection;
import java.util.Collections;

/**
* A {@link RequestSecurityValidator} for validating that the incoming request for any of the given collection
Expand All @@ -14,7 +17,7 @@
@SuppressWarnings("WeakerAccess")
public class BasicAuthSecurityValidator implements RequestSecurityValidator {

protected final Collection<Endpoint<?>> basicAuthValidatedEndpoints;
protected final @NotNull Collection<Endpoint<?>> basicAuthValidatedEndpoints;

protected final String expectedUsername;
protected final String expectedPassword;
Expand All @@ -28,13 +31,18 @@ public class BasicAuthSecurityValidator implements RequestSecurityValidator {
public BasicAuthSecurityValidator(Collection<Endpoint<?>> basicAuthValidatedEndpoints,
String expectedUsername,
String expectedPassword) {
this.basicAuthValidatedEndpoints = basicAuthValidatedEndpoints;
this.basicAuthValidatedEndpoints = (basicAuthValidatedEndpoints == null)
? Collections.emptySet()
: basicAuthValidatedEndpoints;
this.expectedUsername = expectedUsername;
this.expectedPassword = expectedPassword;
}

@Override
public void validateSecureRequestForEndpoint(RequestInfo<?> requestInfo, Endpoint<?> endpoint) {
public void validateSecureRequestForEndpoint(
@NotNull RequestInfo<?> requestInfo,
@NotNull Endpoint<?> endpoint
) {
String authorizationHeader = requestInfo.getHeaders().get("Authorization");

if (authorizationHeader == null) {
Expand Down Expand Up @@ -74,7 +82,7 @@ public void validateSecureRequestForEndpoint(RequestInfo<?> requestInfo, Endpoin
}

@Override
public Collection<Endpoint<?>> endpointsToValidate() {
public @NotNull Collection<Endpoint<?>> endpointsToValidate() {
return basicAuthValidatedEndpoints;
}

Expand Down
16 changes: 11 additions & 5 deletions .../src/main/java/com/nike/riposte/server/error/validation/PolymorphicSecurityValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import com.nike.riposte.server.http.Endpoint;
import com.nike.riposte.server.http.RequestInfo;

import org.jetbrains.annotations.NotNull;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
Expand All @@ -18,7 +20,7 @@
@SuppressWarnings("WeakerAccess")
public class PolymorphicSecurityValidator implements RequestSecurityValidator {

protected Map<Endpoint<?>, List<RequestSecurityValidator>> validationMap;
protected @NotNull Map<Endpoint<?>, List<RequestSecurityValidator>> validationMap;
protected final boolean isFastEnoughToRunOnNettyWorkerThread;

/**
Expand All @@ -34,8 +36,9 @@ public PolymorphicSecurityValidator(List<RequestSecurityValidator> validators) {
isFastEnoughToRunOnNettyWorkerThread = !containsSlowValidator;
}

protected Map<Endpoint<?>, List<RequestSecurityValidator>> buildValidationMap(
List<RequestSecurityValidator> validators) {
protected @NotNull Map<Endpoint<?>, List<RequestSecurityValidator>> buildValidationMap(
List<RequestSecurityValidator> validators
) {
Map<Endpoint<?>, List<RequestSecurityValidator>> map = new HashMap<>();
if (validators == null) {
return map;
Expand All @@ -59,7 +62,10 @@ protected Map<Endpoint<?>, List<RequestSecurityValidator>> buildValidationMap(
* validators.
*/
@Override
public void validateSecureRequestForEndpoint(RequestInfo<?> requestInfo, Endpoint<?> endpoint) {
public void validateSecureRequestForEndpoint(
@NotNull RequestInfo<?> requestInfo,
@NotNull Endpoint<?> endpoint
) {
List<RequestSecurityValidator> validators = validationMap.get(endpoint);
if (validators == null || validators.isEmpty()) {
// if there are no validators for the endpoint, we don't need to validate
Expand All @@ -83,7 +89,7 @@ public void validateSecureRequestForEndpoint(RequestInfo<?> requestInfo, Endpoin
}

@Override
public Collection<Endpoint<?>> endpointsToValidate() {
public @NotNull Collection<Endpoint<?>> endpointsToValidate() {
return validationMap.keySet();
}

Expand Down
20 changes: 13 additions & 7 deletions ...rc/test/java/com/nike/riposte/server/error/validation/BasicAuthSecurityValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.nike.riposte.server.http.RequestInfo;

import org.apache.commons.codec.binary.Base64;
import org.assertj.core.api.Assertions;
import org.junit.Before;
import org.junit.Test;

Expand All @@ -14,7 +15,6 @@
import io.netty.handler.codec.http.HttpHeaders;

import static org.hamcrest.core.Is.is;
import static org.hamcrest.core.IsNull.nullValue;
import static org.junit.Assert.assertThat;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
Expand All @@ -39,6 +39,18 @@ public void setup() {
PASSWORD);
}

@Test
public void constructor_uses_empty_collection_if_passed_null_endpoints_collection() {
// when
BasicAuthSecurityValidator instance = new BasicAuthSecurityValidator(null, USERNAME, PASSWORD);

// then
Assertions.assertThat(instance.basicAuthValidatedEndpoints)
.isNotNull()
.isEmpty();
Assertions.assertThat(instance.endpointsToValidate()).isSameAs(instance.basicAuthValidatedEndpoints);
}

@Test
public void endpointsToValidateTest() {
assertThat(underTest.endpointsToValidate().size(), is(3));
Expand All @@ -47,12 +59,6 @@ public void endpointsToValidateTest() {
assertThat(underTest.endpointsToValidate().contains(mockEndpoint3), is(true));
}

@Test
public void endpointsToValidateNullTest() {
underTest = new BasicAuthSecurityValidator(null, USERNAME, PASSWORD);
assertThat(underTest.endpointsToValidate(), nullValue());
}

@Test
public void endpointsToValidateEmptyTest() {
underTest = new BasicAuthSecurityValidator(Collections.emptyList(), USERNAME, PASSWORD);
Expand Down
19 changes: 11 additions & 8 deletions riposte-core/src/main/java/com/nike/riposte/server/Server.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public class Server {

private final Logger logger = LoggerFactory.getLogger(this.getClass());

private final ServerConfig serverConfig;
private final @NotNull ServerConfig serverConfig;

private final List<EventLoopGroup> eventLoopGroups = new ArrayList<>();
private final List<Channel> channels = new ArrayList<>();
Expand All @@ -57,7 +57,7 @@ public class Server {
@SuppressWarnings("WeakerAccess")
public static final String SERVER_BOSS_CHANNEL_DEBUG_LOGGER_NAME = "ServerBossChannelDebugLogger";

public Server(ServerConfig serverConfig) {
public Server(@NotNull ServerConfig serverConfig) {
this.serverConfig = serverConfig;
}

Expand Down Expand Up @@ -155,8 +155,9 @@ public void startup() throws CertificateException, IOException, InterruptedExcep
.childHandler(channelInitializer);

// execute pre startup hooks
if (serverConfig.preServerStartupHooks() != null) {
for (PreServerStartupHook hook : serverConfig.preServerStartupHooks()) {
List<@NotNull PreServerStartupHook> preServerStartupHooks = serverConfig.preServerStartupHooks();
if (preServerStartupHooks != null) {
for (PreServerStartupHook hook : preServerStartupHooks) {
hook.executePreServerStartupHook(b);
}
}
Expand All @@ -170,8 +171,9 @@ public void startup() throws CertificateException, IOException, InterruptedExcep
.channel();

// execute post startup hooks
if (serverConfig.postServerStartupHooks() != null) {
for (PostServerStartupHook hook : serverConfig.postServerStartupHooks()) {
List<@NotNull PostServerStartupHook> postServerStartupHooks = serverConfig.postServerStartupHooks();
if (postServerStartupHooks != null) {
for (PostServerStartupHook hook : postServerStartupHooks) {
hook.executePostServerStartupHook(serverConfig, ch);
}
}
Expand Down Expand Up @@ -224,8 +226,9 @@ public synchronized void shutdown() throws InterruptedException {
List<ChannelFuture> channelCloseFutures = new ArrayList<>();
for (Channel ch : channels) {
// execute shutdown hooks
if (serverConfig.serverShutdownHooks() != null) {
for (ServerShutdownHook hook : serverConfig.serverShutdownHooks()) {
List<@NotNull ServerShutdownHook> serverShutdownHooks = serverConfig.serverShutdownHooks();
if (serverShutdownHooks != null) {
for (ServerShutdownHook hook : serverShutdownHooks) {
hook.executeServerShutdownHook(serverConfig, ch);
}
}
Expand Down
Loading