Skip to content

Commit

Permalink
Add nullability annotations to APIs commonly used by services
Browse files Browse the repository at this point in the history 
This significantly improves kotlin support.
  • Loading branch information
@nicmunroe
nicmunroe committed Mar 1, 2019
1 parent c5e2f7e commit 65e7b6a
Show file tree
Hide file tree
Showing 131 changed files with 2,719 additions and 1,230 deletions.
6 changes: 3 additions & 3 deletions build.gradle
View file
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
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
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
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
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
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
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
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
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
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

0 comments on commit 65e7b6a

Please sign in to comment.