From eb8d89306fd569d7ef64298a99e970c653b79178 Mon Sep 17 00:00:00 2001 From: Mike Hardy Date: Tue, 18 May 2021 13:56:38 -0500 Subject: [PATCH] fix(android): correct lint issues for various API mis-use Highlights: - enforce CallSuper so TaskExecutors will always clean up correctly - enforce Locale translation in string manipulaton (Fixes #3917) --- .../common/ReactNativeFirebaseModule.java | 24 ++++++++++++------- .../firebase/common/SharedUtils.java | 2 +- .../auth/ReactNativeFirebaseAuthModule.java | 3 ++- .../UniversalFirebaseDatabaseCommon.java | 2 +- ...ReactNativeFirebaseDynamicLinksModule.java | 1 + ...iveFirebaseFirestoreTransactionModule.java | 1 + .../ReactNativeFirebaseStorageCommon.java | 3 ++- .../ReactNativeFirebaseStorageModule.java | 1 + tests/android/build.gradle | 1 + tests/android/lint.xml | 8 +++---- 10 files changed, 29 insertions(+), 17 deletions(-) diff --git a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java index f3f61983ec..45512f486c 100644 --- a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java +++ b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java @@ -19,15 +19,18 @@ import android.app.Activity; import android.content.Context; -import com.facebook.react.bridge.*; -import io.invertase.firebase.interfaces.ContextProvider; -import io.invertase.firebase.common.TaskExecutorService; +import androidx.annotation.CallSuper; +import androidx.annotation.NonNull; -import javax.annotation.Nonnull; import java.util.HashMap; import java.util.Map; import java.util.concurrent.ExecutorService; +import com.facebook.react.bridge.*; + +import io.invertase.firebase.interfaces.ContextProvider; +import io.invertase.firebase.common.TaskExecutorService; + public class ReactNativeFirebaseModule extends ReactContextBaseJavaModule implements ContextProvider { private final TaskExecutorService executorService; @@ -67,6 +70,7 @@ public static void rejectPromiseWithCodeAndMessage( } @Override + @CallSuper public void initialize() { super.initialize(); } @@ -75,24 +79,25 @@ public ReactContext getContext() { return getReactApplicationContext(); } - public ExecutorService getExecutor() { + public final ExecutorService getExecutor() { return executorService.getExecutor(); } - public ExecutorService getTransactionalExecutor() { + public final ExecutorService getTransactionalExecutor() { return executorService.getTransactionalExecutor(); } - public ExecutorService getTransactionalExecutor(String identifier) { + public final ExecutorService getTransactionalExecutor(String identifier) { return executorService.getTransactionalExecutor(identifier); } @Override + @CallSuper public void onCatalystInstanceDestroy() { executorService.shutdown(); } - public void removeEventListeningExecutor(String identifier) { + public final void removeEventListeningExecutor(String identifier) { String executorName = executorService.getExecutorName(true, identifier); executorService.removeExecutor(executorName); } @@ -105,12 +110,13 @@ public Activity getActivity() { return getCurrentActivity(); } - @Nonnull + @NonNull @Override public String getName() { return "RNFB" + moduleName + "Module"; } + @NonNull @Override public Map getConstants() { return new HashMap<>(); diff --git a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/SharedUtils.java b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/SharedUtils.java index 2764b201fc..19e9b3baec 100644 --- a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/SharedUtils.java +++ b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/SharedUtils.java @@ -99,7 +99,7 @@ public static WritableMap getExceptionMap(Exception exception) { public static String timestampToUTC(long timestamp) { long millisTimestamp = timestamp * 1000; - SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'"); + SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'", Locale.ROOT); format.setTimeZone(TimeZone.getTimeZone("UTC")); return format.format(millisTimestamp); } diff --git a/packages/auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java b/packages/auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java index 4e5b4f4e72..d30755e22a 100644 --- a/packages/auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java +++ b/packages/auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java @@ -61,6 +61,7 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.concurrent.TimeUnit; @@ -1885,7 +1886,7 @@ private WritableMap getJSError(Exception exception) { } code = code - .toLowerCase() + .toLowerCase(Locale.ROOT) .replace("error_", "") .replace('_', '-'); error.putString("code", code); diff --git a/packages/database/android/src/main/java/io/invertase/firebase/database/UniversalFirebaseDatabaseCommon.java b/packages/database/android/src/main/java/io/invertase/firebase/database/UniversalFirebaseDatabaseCommon.java index 42c9073c66..7b6b9bbe86 100644 --- a/packages/database/android/src/main/java/io/invertase/firebase/database/UniversalFirebaseDatabaseCommon.java +++ b/packages/database/android/src/main/java/io/invertase/firebase/database/UniversalFirebaseDatabaseCommon.java @@ -95,7 +95,7 @@ static void addEmulatorConfig(String appName, String dbURL, String host, int por String configKey = appName + dbURL; HashMap emulatorConfig = new HashMap<>(); emulatorConfig.put("host", host); - emulatorConfig.put("port", new Integer(port)); + emulatorConfig.put("port", Integer.valueOf(port)); emulatorConfigs.put(configKey, emulatorConfig); } diff --git a/packages/dynamic-links/android/src/main/java/io/invertase/firebase/dynamiclinks/ReactNativeFirebaseDynamicLinksModule.java b/packages/dynamic-links/android/src/main/java/io/invertase/firebase/dynamiclinks/ReactNativeFirebaseDynamicLinksModule.java index 3f2dab3c3e..7ea87e6916 100644 --- a/packages/dynamic-links/android/src/main/java/io/invertase/firebase/dynamiclinks/ReactNativeFirebaseDynamicLinksModule.java +++ b/packages/dynamic-links/android/src/main/java/io/invertase/firebase/dynamiclinks/ReactNativeFirebaseDynamicLinksModule.java @@ -72,6 +72,7 @@ public class ReactNativeFirebaseDynamicLinksModule extends ReactNativeFirebaseMo public void onCatalystInstanceDestroy() { getReactApplicationContext().removeActivityEventListener(this); getReactApplicationContext().addLifecycleEventListener(this); + super.onCatalystInstanceDestroy(); } @ReactMethod diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreTransactionModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreTransactionModule.java index 8b5f9a37c6..be91983718 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreTransactionModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreTransactionModule.java @@ -56,6 +56,7 @@ public void onCatalystInstanceDestroy() { } transactionHandlers.clear(); + super.onCatalystInstanceDestroy(); } @ReactMethod diff --git a/packages/storage/android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageCommon.java b/packages/storage/android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageCommon.java index 5ac6540256..c41ddc7c98 100644 --- a/packages/storage/android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageCommon.java +++ b/packages/storage/android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageCommon.java @@ -35,6 +35,7 @@ import com.google.firebase.storage.StorageReference; import com.google.firebase.storage.StorageTask; +import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -143,7 +144,7 @@ static StorageMetadata buildMetadataFromMap(ReadableMap metadataMap, @Nullable U if (mimeType == null) { String fileExt = MimeTypeMap.getFileExtensionFromUrl(file.toString()); - mimeType = MimeTypeMap.getSingleton().getMimeTypeFromExtension(fileExt.toLowerCase()); + mimeType = MimeTypeMap.getSingleton().getMimeTypeFromExtension(fileExt.toLowerCase(Locale.ROOT)); } if (mimeType != null) { diff --git a/packages/storage/android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageModule.java b/packages/storage/android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageModule.java index 8986b3963e..49127ddf3f 100644 --- a/packages/storage/android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageModule.java +++ b/packages/storage/android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageModule.java @@ -54,6 +54,7 @@ public class ReactNativeFirebaseStorageModule extends ReactNativeFirebaseModule @Override public void onCatalystInstanceDestroy() { ReactNativeFirebaseStorageTask.destroyAllTasks(); + super.onCatalystInstanceDestroy(); } /** diff --git a/tests/android/build.gradle b/tests/android/build.gradle index 9925a0a633..c24bcb419d 100644 --- a/tests/android/build.gradle +++ b/tests/android/build.gradle @@ -106,6 +106,7 @@ subprojects { abortOnError = true warningsAsErrors false lintConfig file('./lint.xml') + ignore 'UnknownNullness', 'SyntheticAccessor', 'LogConditional' checkReleaseBuilds = true checkAllWarnings true showAll true diff --git a/tests/android/lint.xml b/tests/android/lint.xml index ef7606f0cb..f0521da782 100644 --- a/tests/android/lint.xml +++ b/tests/android/lint.xml @@ -21,13 +21,15 @@ --> + + - + @@ -42,6 +44,7 @@ + @@ -91,7 +94,6 @@ - @@ -183,7 +185,6 @@ - @@ -191,7 +192,6 @@ -