From 98fa39a1d39002acf0fc2a56abe956478906837e Mon Sep 17 00:00:00 2001
From: Connie Yau <conniey@microsoft.com>
Date: Tue, 26 Feb 2019 19:49:06 -0800
Subject: [PATCH] SpotBug P3 Fixes (#2973)

* Adding suppression for having named static inner classes from anonymous.

* Add suppression for redundant null check.

* Move SIC_INNER_SHOULD_BE_STATIC suppression

* Make anonymous class in CachingKeyResolver a named inner static.

* Include missing impl classes for auto-rest when it generates a variable with known null value

* Suppressing unconfirmed casts because we know the cast is correct but the APIs weren't designed for it
---
 .../resources/spotbugs/spotbugs-exclude.xml   | 22 ++++++++++++++++++-
 .../extensions/CachingKeyResolver.java        | 22 +++++++++++++------
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml b/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml
index 906a06dd54c64..3560ab12958a9 100755
--- a/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml
+++ b/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml
@@ -1,6 +1,12 @@
 <?xml version="1.0" encoding="UTF-8"?>
 
 <FindBugsFilter>
+    <!-- These existing KeyVault Attribute APIs return their super class rather than the actual type. -->
+    <Match>
+        <Class name="~com\.microsoft\.azure\.keyvault\.requests\.\w+Request"/>
+        <Bug pattern="BC_UNCONFIRMED_CAST_OF_RETURN_VALUE"/>
+    </Match>
+
     <!-- These keyvault models are already publicly released APIs with name
     matching the simple name of superclass -->
     <Match>
@@ -33,6 +39,13 @@
         <Bug pattern="UUF_UNUSED_PUBLIC_OR_PROTECTED_FIELD"/>
     </Match>
 
+    <!-- Suppress non-null warning in the case that we change the code and it is possible for
+    KeyVaultCredentials.getAuthenticationCredentials to return null. -->
+    <Match>
+        <Class name="com.microsoft.azure.keyvault.authentication.KeyVaultCredentials"/>
+        <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"/>
+    </Match>
+
     <!-- These KeyVaultClientBase methods are made synchronous by blocking and
     waiting for a result. They do not return anything, so it can be ignored. -->
     <Match>
@@ -41,6 +54,13 @@
         <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT"/>
     </Match>
 
+    <!-- These autorest generated APIs for existing Azure SDK components supports Java 7.
+    Suppressing error for using anonymous inner classes. -->
+    <Match>
+        <Class name="~com\.microsoft\.azure\.(.+)Impl"/>
+        <Bug pattern="SIC_INNER_SHOULD_BE_STATIC_ANON"/>
+    </Match>
+
     <!-- KeyVaultCredential values manipulated do not need to be localised. They
     are base64 encoded or URL encoded.-->
     <Match>
@@ -58,7 +78,7 @@
     <!-- Known issue in autorest where it will generate a variable with a value of null.
     https://github.com/Azure/autorest.java/issues/324 -->
     <Match>
-        <Class name="com.microsoft.azure.keyvault.KeyVaultClientImpl"/>
+        <Class name="~com\.microsoft\.azure\.(.+)Impl"/>
         <Bug pattern="NP_LOAD_OF_KNOWN_NULL_VALUE"/>
     </Match>
 </FindBugsFilter>
\ No newline at end of file
diff --git a/keyvault/data-plane/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/CachingKeyResolver.java b/keyvault/data-plane/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/CachingKeyResolver.java
index 5a6afb43c2ea5..a0d3a66bd7f28 100644
--- a/keyvault/data-plane/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/CachingKeyResolver.java
+++ b/keyvault/data-plane/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/CachingKeyResolver.java
@@ -28,13 +28,7 @@ public class CachingKeyResolver implements IKeyResolver {
     public CachingKeyResolver(int capacity, final IKeyResolver keyResolver) {
         this.keyResolver = keyResolver;
         cache = CacheBuilder.newBuilder().maximumSize(capacity)
-                .build(new CacheLoader<String, ListenableFuture<IKey>>() {
-
-                    @Override
-                    public ListenableFuture<IKey> load(String kid) {
-                        return keyResolver.resolveKeyAsync(kid);
-                    }
-                });
+                .build(new CachingKeyResolverCacheLoader(keyResolver));
     }
 
     @Override
@@ -58,4 +52,18 @@ public void run() {
             return cache.getUnchecked(kid);
         }
     }
+
+    private static class CachingKeyResolverCacheLoader extends CacheLoader<String, ListenableFuture<IKey>> {
+
+        private final IKeyResolver keyResolver;
+
+        CachingKeyResolverCacheLoader(IKeyResolver keyResolver) {
+            this.keyResolver = keyResolver;
+        }
+
+        @Override
+        public ListenableFuture<IKey> load(String kid) {
+            return keyResolver.resolveKeyAsync(kid);
+        }
+    }
 }