Merge pull request #150 from microsoftgraph/feature/spotbugs

feature/spotbugs

Author: baywet

build.gradle

@@ -14,6 +14,7 @@ plugins {
     id 'maven-publish'
     id 'signing'
     id 'jacoco'
+    id 'com.github.spotbugs' version '4.6.0'
 }
 
 java {
@@ -32,6 +33,28 @@ jacoco {
     toolVersion = "0.8.7-SNAPSHOT" //https://github.com/gradle/gradle/issues/15038
 }
 
+spotbugsMain {
+    excludeFilter = file("spotBugsExcludeFilter.xml")
+    reports {
+        html {
+            enabled = true
+            destination = file("$buildDir/reports/spotbugs/main/spotbugs.html")
+            stylesheet = 'fancy-hist.xsl'
+        }
+    }
+}
+
+spotbugsTest {
+    excludeFilter = file("spotBugsExcludeFilter.xml")
+    reports {
+        html {
+            enabled = true
+            destination = file("$buildDir/reports/spotbugs/test/spotbugs.html")
+            stylesheet = 'fancy-hist.xsl'
+        }
+    }
+}
+
 jacocoTestReport {
     reports {
         xml.enabled true

spotBugsExcludeFilter.xml

@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<FindBugsFilter
+xmlns="https://github.com/spotbugs/filter/3.0.0"
+xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0 https://raw.githubusercontent.com/spotbugs/spotbugs/3.1.0/spotbugs/etc/findbugsfilter.xsd">
+	<Match>
+		<Class name="com.microsoft.graph.authentication.BaseAuthenticationProviderTest" />
+		<Method name="providerDoesNotAddTokenOnNullUrls" />
+		<Bug code="NP" />
+	</Match>
+	<Match>
+		<Class name="com.microsoft.graph.concurrency.ChunkedUploadRequest" />
+		<Bug code="Dm" />
+	</Match>
+	<Match>
+		<Class name="com.microsoft.graph.concurrency.ChunkedUploadResponseHandler" />
+		<Method name="generateResult" />
+		<Bug code="RCN,NP" />
+	</Match>
+</FindBugsFilter>

src/main/java/com/microsoft/graph/concurrency/ChunkedUploadProvider.java

@@ -30,6 +30,7 @@
 import java.io.InputStream;
 import java.security.InvalidParameterException;
 import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 
@@ -103,25 +104,15 @@ public ChunkedUploadProvider(@Nonnull final IUploadSession uploadSession,
                                  @Nonnull final InputStream inputStream,
                                  final long streamSize,
                                  @Nonnull final Class<UploadType> uploadTypeClass) {
-        if (uploadSession == null) {
-            throw new InvalidParameterException("Upload session is null.");
-        }
-
-        if (client == null) {
-            throw new InvalidParameterException("OneDrive client is null.");
-        }
-
-        if (inputStream == null) {
-            throw new InvalidParameterException("Input stream is null.");
-        }
+        Objects.requireNonNull(uploadSession, "Upload session is null.");
 
         if (streamSize <= 0) {
             throw new InvalidParameterException("Stream size should larger than 0.");
         }
 
-        this.client = client;
+        this.client = Objects.requireNonNull(client, "Graph client is null.");
         this.readSoFar = 0;
-        this.inputStream = inputStream;
+        this.inputStream = Objects.requireNonNull(inputStream, "Input stream is null.");
         this.streamSize = streamSize;
         this.uploadUrl = uploadSession.getUploadUrl();
         this.responseHandler = new ChunkedUploadResponseHandler<UploadType>(uploadTypeClass, uploadSession.getClass());

src/main/java/com/microsoft/graph/concurrency/ChunkedUploadRequest.java

@@ -6,6 +6,7 @@
 
 import java.util.List;
 import java.util.Locale;
+import java.util.Objects;
 
 import javax.annotation.Nullable;
 import javax.annotation.Nonnull;
@@ -61,9 +62,12 @@ protected ChunkedUploadRequest(@Nonnull final String requestUrl,
                                 final int chunkSize,
                                 final long beginIndex,
                                 final long totalLength) {
+        Objects.requireNonNull(requestUrl, "parameter requestUrl cannot be null");
+        Objects.requireNonNull(client, "parameter client cannot be null");
+        Objects.requireNonNull(chunk, "parameter chunk cannot be null");
         this.data = new byte[chunkSize];
         System.arraycopy(chunk, 0, this.data, 0, chunkSize);
-        this.baseRequest = new BaseRequest<ChunkedUploadResult<UploadType>>(requestUrl, client, options, (Class<? extends ChunkedUploadResult<UploadType>>)(new ChunkedUploadResult<UploadType>((UploadType)null)).getClass()) {
+        this.baseRequest = new BaseRequest<ChunkedUploadResult<UploadType>>(requestUrl, client, options, (Class<? extends ChunkedUploadResult<UploadType>>)(new ChunkedUploadResult<>((UploadType)null)).getClass()) {
         };
         this.baseRequest.setHttpMethod(HttpMethod.PUT);
         this.baseRequest.addHeader(CONTENT_RANGE_HEADER_NAME,
@@ -84,6 +88,7 @@ protected ChunkedUploadRequest(@Nonnull final String requestUrl,
     @Nonnull
     public ChunkedUploadResult<UploadType> upload(
             @Nonnull final ChunkedUploadResponseHandler<UploadType> responseHandler) {
+        Objects.requireNonNull(responseHandler, "parameter responseHandler cannot be null");
         ChunkedUploadResult<UploadType> result = null;
 
         try {

src/main/java/com/microsoft/graph/concurrency/ChunkedUploadResponseHandler.java

@@ -35,10 +35,12 @@
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Objects;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import okhttp3.MediaType;
 import okhttp3.Response;
 import okhttp3.ResponseBody;
 
@@ -64,8 +66,8 @@ public class ChunkedUploadResponseHandler<UploadType>
      * @param uploadSessionType the type of the upload session
 	 */
 	protected ChunkedUploadResponseHandler(@Nonnull final Class<UploadType> uploadType, @Nonnull final Class<? extends IUploadSession>  uploadSessionType) {
-        this.deserializeTypeClass = uploadType;
-        this.uploadSessionClass = uploadSessionType;
+        this.deserializeTypeClass = Objects.requireNonNull(uploadType, "parameter uploadType cannot be null");
+        this.uploadSessionClass = Objects.requireNonNull(uploadSessionType, "parameter uploadSessionType cannot be null");
 	}
 
 	/**
@@ -95,6 +97,10 @@ public ChunkedUploadResult<UploadType> generateResult(
 			@Nonnull final Response response,
 			@Nonnull final ISerializer serializer,
 			@Nonnull final ILogger logger) throws Exception {
+        Objects.requireNonNull(request, "parameter request cannot be null");
+        Objects.requireNonNull(response, "parameter response cannot be null");
+        Objects.requireNonNull(serializer, "parameter serializer cannot be null");
+        Objects.requireNonNull(logger, "parameter logger cannot be null");
 		if (response.code() >= HttpResponseCode.HTTP_CLIENT_ERROR) {
 			logger.logDebug("Receiving error during upload, see detail on result error");
 
@@ -105,7 +111,9 @@ public ChunkedUploadResult<UploadType> generateResult(
 				&& response.code() < HttpResponseCode.HTTP_MULTIPLE_CHOICES) {
             try(final ResponseBody body = response.body()) {
                 final String location = response.headers().get("Location");
-                if (body.contentType() != null && body.contentType().subtype().contains("json")) {
+                final MediaType contentType = body.contentType();
+                final String subType = contentType == null ? null : contentType.subtype();
+                if (subType != null && subType.contains("json")) {
                     return parseJsonUploadResult(body, serializer, logger);
                 } else if (location != null) {
                     logger.logDebug("Upload session is completed (Outlook), uploaded item returned.");

src/main/java/com/microsoft/graph/concurrency/ChunkedUploadResult.java

@@ -8,6 +8,9 @@
 import com.microsoft.graph.http.GraphServiceException;
 
 import javax.annotation.Nullable;
+
+import java.util.Objects;
+
 import javax.annotation.Nonnull;
 
 /**
@@ -68,7 +71,10 @@ protected ChunkedUploadResult(@Nullable final ClientException error) {
      * @param exception The exception received from server.
      */
     protected ChunkedUploadResult(@Nonnull final GraphServiceException exception) {
-        this(new ClientException(exception.getMessage(/* verbose */ true), exception));
+        this(new ClientException(Objects
+                                .requireNonNull(exception, "parameter exception cannot be null")
+                                .getMessage(/* verbose */ true),
+                                exception));
     }
 
     /**

src/main/java/com/microsoft/graph/content/BatchRequest.java

@@ -43,7 +43,7 @@ public class BatchRequest extends BaseRequest<BatchResponseContent> {
      * @param client the client to use to execute the request
      * @param options the options to apply to the request
      */
-    public BatchRequest(@Nonnull final String requestUrl, @Nonnull final IBaseClient client, @Nonnull final List<? extends Option> options) {
+    public BatchRequest(@Nonnull final String requestUrl, @Nonnull final IBaseClient<?> client, @Nonnull final List<? extends Option> options) {
         super(requestUrl, client, options, BatchResponseContent.class);
     }
 
@@ -55,7 +55,7 @@ public BatchRequest(@Nonnull final String requestUrl, @Nonnull final IBaseClient
      * @throws ClientException an exception occurs if there was an error while the request was sent
      */
     @Nullable
-    public BatchResponseContent post(@Nonnull final BatchRequestContent content) throws ClientException {
+    public BatchResponseContent post(@Nullable final BatchRequestContent content) throws ClientException {
         this.setHttpMethod(HttpMethod.POST);
         final BatchResponseContent response = this.getClient().getHttpProvider().send(this, BatchResponseContent.class, content);
         setSerializerOnSteps(response);
@@ -69,7 +69,7 @@ public BatchResponseContent post(@Nonnull final BatchRequestContent content) thr
      * @throws ClientException an exception occurs if there was an error while the request was sent
      */
     @Nullable
-    public java.util.concurrent.CompletableFuture<BatchResponseContent> postAsync(@Nonnull final BatchRequestContent content) throws ClientException {
+    public java.util.concurrent.CompletableFuture<BatchResponseContent> postAsync(@Nullable final BatchRequestContent content) throws ClientException {
         this.setHttpMethod(HttpMethod.POST);
         return this.getClient().getHttpProvider().sendAsync(this, BatchResponseContent.class, content).thenApply(response -> {
             setSerializerOnSteps(response);

src/main/java/com/microsoft/graph/content/BatchRequestBuilder.java

@@ -39,7 +39,7 @@ public class BatchRequestBuilder extends BaseRequestBuilder<BatchResponseContent
      * @param client the client to use to execute the request
      * @param options the request options
      */
-    public BatchRequestBuilder(@Nonnull final String requestUrl, @Nonnull final IBaseClient client, @Nonnull final List<? extends Option> options) {
+    public BatchRequestBuilder(@Nonnull final String requestUrl, @Nonnull final IBaseClient<?> client, @Nonnull final List<? extends Option> options) {
         super(requestUrl, client, options);
     }
     /**

src/main/java/com/microsoft/graph/content/BatchRequestContent.java

@@ -138,10 +138,10 @@ public void removeBatchRequestStepWithId(@Nonnull final String ...stepIds) {
 
         for(final String stepId : stepIds) {
             Objects.requireNonNull(stepId, "parameter stepIds cannot contain null values");
-            requests.removeIf(x -> x.id == stepId);
+            requests.removeIf(x -> stepId.equals(x.id));
             for(final BatchRequestStep<?> step : requests) {
                 if(step.dependsOn != null) {
-                    step.dependsOn.removeIf(x -> x == stepId);
+                    step.dependsOn.removeIf(x -> stepId.equals(x));
                     if(step.dependsOn.isEmpty())
                         step.dependsOn = null; // so we don't send dependsOn: [] over the wire
                 }

src/main/java/com/microsoft/graph/core/BaseClient.java

@@ -35,6 +35,7 @@
 
 import javax.annotation.Nullable;
 
+import java.util.Collections;
 import java.util.Objects;
 
 import javax.annotation.Nonnull;
@@ -74,7 +75,7 @@ public String getServiceRoot() {
 
     @Override
     public void setServiceRoot(@Nonnull final String value) {
-        endpoint = value;
+        endpoint = Objects.requireNonNull(value, "value parameter cannot be null");
     }
 
 	/**
@@ -88,7 +89,9 @@ public void setServiceRoot(@Nonnull final String value) {
 	 */
 	@Nonnull
 	public <T> CustomRequestBuilder<T> customRequest(@Nonnull final String url, @Nonnull final Class<T> responseType) {
-		return new CustomRequestBuilder<>(getServiceRoot() + url, this, null, responseType);
+		Objects.requireNonNull(url, "url parameter cannot be null");
+        Objects.requireNonNull(responseType, "responseType parameter cannot be null");
+        return new CustomRequestBuilder<>(getServiceRoot() + url, this, null, responseType);
 	}
 
 	/**
@@ -100,8 +103,7 @@ public <T> CustomRequestBuilder<T> customRequest(@Nonnull final String url, @Non
 	 */
 	@Nonnull
 	public CustomRequestBuilder<JsonElement> customRequest(@Nonnull final String url) {
-		return new CustomRequestBuilder<>(getServiceRoot() + url, this, null,
-				JsonElement.class);
+		return this.customRequest(url, JsonElement.class);
     }
 
     /**
@@ -110,7 +112,7 @@ public CustomRequestBuilder<JsonElement> customRequest(@Nonnull final String url
      */
     @Nonnull
     public BatchRequestBuilder batch() {
-        return new BatchRequestBuilder(getServiceRoot() + "/$batch", this, null);
+        return new BatchRequestBuilder(getServiceRoot() + "/$batch", this, Collections.emptyList());
     }
 
     /**
@@ -196,7 +198,7 @@ private IHttpProvider<nativeRequestType> getHttpProvider() {
 		 */
 		@Nonnull
 		public Builder<httpClientType, nativeRequestType> serializer(@Nonnull final ISerializer serializer) {
-			checkNotNull(serializer, "serializer");
+			Objects.requireNonNull(serializer, "parameter serializer cannot be null");
 			this.serializer = serializer;
 			return this;
 		}
@@ -210,7 +212,7 @@ public Builder<httpClientType, nativeRequestType> serializer(@Nonnull final ISer
 		 */
 		@Nonnull
 		public Builder<httpClientType, nativeRequestType> httpProvider(@Nonnull final IHttpProvider<nativeRequestType> httpProvider) {
-			checkNotNull(httpProvider, "httpProvider");
+			Objects.requireNonNull(httpProvider, "parameter httpProvider cannot be null");
 			this.httpProvider = httpProvider;
 			return this;
 		}
@@ -224,7 +226,7 @@ public Builder<httpClientType, nativeRequestType> httpProvider(@Nonnull final IH
 		 */
 		@Nonnull
 		public Builder<httpClientType, nativeRequestType> logger(@Nonnull final ILogger logger) {
-			checkNotNull(logger, "logger");
+			Objects.requireNonNull(logger, "parameter logger cannot be null");
 			this.logger = logger;
 			return this;
 		}
@@ -238,7 +240,7 @@ public Builder<httpClientType, nativeRequestType> logger(@Nonnull final ILogger
 		 */
 		@Nonnull
 		public Builder<httpClientType, nativeRequestType> httpClient(@Nonnull final httpClientType client) {
-			checkNotNull(client, "client");
+			Objects.requireNonNull(client, "parameter client cannot be null");
 			this.httpClient = client;
 			return this;
 		}
@@ -251,7 +253,7 @@ public Builder<httpClientType, nativeRequestType> httpClient(@Nonnull final http
 		 */
 		@Nonnull
 		public Builder<httpClientType, nativeRequestType> authenticationProvider(@Nonnull final IAuthenticationProvider auth) {
-			checkNotNull(auth, "auth");
+			Objects.requireNonNull(auth, "parameter auth cannot be null");
 			this.auth = auth;
 			return this;
 		}
@@ -287,18 +289,6 @@ public IBaseClient<nativeRequestType> buildClient() throws ClientException {
 		}
 	}
 
-	/**
-	 * Checks whether the provided object is null or not and throws an exception if it is
-	 *
-	 * @param o object to check
-	 * @param name name to use in the exception message
-	 */
-	protected static void checkNotNull(@Nullable final Object o, @Nonnull final String name) {
-		if (o==null) {
-			throw new NullPointerException(name + " cannot be null");
-		}
-    }
-
     /**
      * The HTTP provider instance
      */
@@ -352,7 +342,7 @@ public ISerializer getSerializer() {
      * @param logger The logger
      */
     protected void setLogger(@Nonnull final ILogger logger) {
-        checkNotNull(logger, "logger");
+        Objects.requireNonNull(logger, "parameter logger cannot be null");
         this.logger = logger;
     }
 
@@ -362,7 +352,7 @@ protected void setLogger(@Nonnull final ILogger logger) {
      * @param httpProvider The HTTP provider
      */
     protected void setHttpProvider(@Nonnull final IHttpProvider<nativeRequestType> httpProvider) {
-        checkNotNull(httpProvider, "httpProvider");
+        Objects.requireNonNull(httpProvider, "parameter httpProvider cannot be null");
         this.httpProvider = httpProvider;
     }
 
@@ -372,7 +362,7 @@ protected void setHttpProvider(@Nonnull final IHttpProvider<nativeRequestType> h
      * @param serializer The serializer
      */
     public void setSerializer(@Nonnull final ISerializer serializer) {
-        checkNotNull(serializer, "serializer");
+        Objects.requireNonNull(serializer, "parameter serializer cannot be null");
         this.serializer = serializer;
     }