Skip to content

Commit

Permalink
fix(android): correct lint issues for various API mis-use
Browse files Browse the repository at this point in the history
Highlights:

- enforce CallSuper so TaskExecutors will always clean up correctly
- enforce Locale translation in string manipulaton (Fixes #3917)
  • Loading branch information
mikehardy committed May 18, 2021
1 parent 70aba4b commit eb8d893
Show file tree
Hide file tree
Showing 10 changed files with 29 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -67,6 +70,7 @@ public static void rejectPromiseWithCodeAndMessage(
}

@Override
@CallSuper
public void initialize() {
super.initialize();
}
Expand All @@ -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);
}
Expand All @@ -105,12 +110,13 @@ public Activity getActivity() {
return getCurrentActivity();
}

@Nonnull
@NonNull
@Override
public String getName() {
return "RNFB" + moduleName + "Module";
}

@NonNull
@Override
public Map<String, Object> getConstants() {
return new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1885,7 +1886,7 @@ private WritableMap getJSError(Exception exception) {
}

code = code
.toLowerCase()
.toLowerCase(Locale.ROOT)
.replace("error_", "")
.replace('_', '-');
error.putString("code", code);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ static void addEmulatorConfig(String appName, String dbURL, String host, int por
String configKey = appName + dbURL;
HashMap<String, Object> emulatorConfig = new HashMap<>();
emulatorConfig.put("host", host);
emulatorConfig.put("port", new Integer(port));
emulatorConfig.put("port", Integer.valueOf(port));
emulatorConfigs.put(configKey, emulatorConfig);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public class ReactNativeFirebaseDynamicLinksModule extends ReactNativeFirebaseMo
public void onCatalystInstanceDestroy() {
getReactApplicationContext().removeActivityEventListener(this);
getReactApplicationContext().addLifecycleEventListener(this);
super.onCatalystInstanceDestroy();
}

@ReactMethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public void onCatalystInstanceDestroy() {
}

transactionHandlers.clear();
super.onCatalystInstanceDestroy();
}

@ReactMethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public class ReactNativeFirebaseStorageModule extends ReactNativeFirebaseModule
@Override
public void onCatalystInstanceDestroy() {
ReactNativeFirebaseStorageTask.destroyAllTasks();
super.onCatalystInstanceDestroy();
}

/**
Expand Down
1 change: 1 addition & 0 deletions tests/android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ subprojects {
abortOnError = true
warningsAsErrors false
lintConfig file('./lint.xml')
ignore 'UnknownNullness', 'SyntheticAccessor', 'LogConditional'
checkReleaseBuilds = true
checkAllWarnings true
showAll true
Expand Down
8 changes: 4 additions & 4 deletions tests/android/lint.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@
<issue id="NewIssueId" severity="ignore"/>
-->
<lint>
<issue id="DefaultLocale" severity="fatal" />
<issue id="NewApi" severity="fatal" />
<issue id="HardcodedText" severity="fatal" />
<issue id="InlinedApi" severity="fatal" />
<issue id="MissingSuperCall" severity="ignore" />
<issue id="StringFormatCount" severity="fatal" />
<issue id="StringFormatMatches" severity="fatal" />
<issue id="StringFormatInvalid" severity="fatal" />
<issue id="UnusedResources" severity="ignore" />
<issue id="UseValueOf" severity="fatal" />
<issue id="ViewConstructor" severity="fatal" />
<issue id="ViewHolder" severity="fatal" />
<issue id="ViewTag" severity="fatal" />
Expand All @@ -42,6 +44,7 @@
<issue id="RtlSymmetry" severity="ignore" />

<!-- <issue id="UseCompatTextViewDrawableXml" severity="ignore" /> -->
<issue id="UnusedResources" severity="ignore" />
<issue id="UnknownNullness" severity="ignore" />
<issue id="SyntheticAccessor" severity="ignore" />
<!-- FIXME need to remove WRITE_EXTERNAL_STORAGE from storage -->
Expand Down Expand Up @@ -91,7 +94,6 @@
<issue id="ButtonOrder" severity="ignore" />
<issue id="ButtonStyle" severity="ignore" />
<issue id="ByteOrderMark" severity="ignore" />
<issue id="MissingSuperCall" severity="ignore" />
<issue id="AdapterViewChildren" severity="ignore" />
<issue id="ScrollViewCount" severity="ignore" />
<issue id="PermissionImpliesUnsupportedChromeOsHardware" severity="ignore" />
Expand Down Expand Up @@ -183,15 +185,13 @@
<issue id="InvalidPackage" severity="ignore" />
<issue id="DrawAllocation" severity="ignore" />
<issue id="UseSparseArrays" severity="ignore" />
<issue id="UseValueOf" severity="ignore" />
<issue id="JavascriptInterface" severity="ignore" />
<issue id="JobSchedulerService" severity="ignore" />
<issue id="KeyboardInaccessibleWidget" severity="ignore" />
<issue id="LabelFor" severity="ignore" />
<issue id="InconsistentLayout" severity="ignore" />
<issue id="InflateParams" severity="ignore" />
<issue id="StaticFieldLeak" severity="ignore" />
<issue id="DefaultLocale" severity="ignore" />
<issue id="LocaleFolder" severity="ignore" />
<issue id="GetLocales" severity="ignore" />
<issue id="InvalidResourceFolder" severity="ignore" />
Expand Down

0 comments on commit eb8d893

Please sign in to comment.