From fe5b2f3ffd8025403277b2422788c056d8b22765 Mon Sep 17 00:00:00 2001 From: Vladimir Kryachko Date: Tue, 8 Nov 2022 11:23:12 -0500 Subject: [PATCH 1/5] Add qualifier support to firebase components. (#3180) * Add qualifier support to firebase components. Details: go/firebase-component-qualifiers * fix errorprone error. * change copyright year. --- firebase-common/firebase-common.gradle | 2 +- .../AbstractComponentContainer.java | 34 -------- .../google/firebase/components/Component.java | 63 +++++++++++++-- .../components/ComponentContainer.java | 38 +++++++-- .../firebase/components/ComponentRuntime.java | 20 ++--- .../firebase/components/CycleDetector.java | 6 +- .../firebase/components/Dependency.java | 80 ++++++++++++++++++- .../google/firebase/components/Qualified.java | 64 +++++++++++++++ .../RestrictedComponentContainer.java | 66 ++++++++++----- .../components/ComponentRuntimeTest.java | 46 ++++++++++- .../firebase/components/ComponentTest.java | 43 ++++++++-- .../components/CycleDetectorTest.java | 43 +++++++++- .../firebase/components/DependencyTest.java | 70 ++++++++++++++-- .../RestrictedComponentContainerTest.java | 24 ++++-- 14 files changed, 491 insertions(+), 108 deletions(-) delete mode 100644 firebase-components/src/main/java/com/google/firebase/components/AbstractComponentContainer.java create mode 100644 firebase-components/src/main/java/com/google/firebase/components/Qualified.java diff --git a/firebase-common/firebase-common.gradle b/firebase-common/firebase-common.gradle index ac5c0a9c218..47726702701 100644 --- a/firebase-common/firebase-common.gradle +++ b/firebase-common/firebase-common.gradle @@ -62,7 +62,7 @@ android { } dependencies { - // TODO(vkryachko): have sdks depend on components directly once components are released. + implementation project(':firebase-annotations') implementation project(':firebase-components') implementation 'com.google.android.gms:play-services-basement:18.1.0' implementation "com.google.android.gms:play-services-tasks:18.0.1" diff --git a/firebase-components/src/main/java/com/google/firebase/components/AbstractComponentContainer.java b/firebase-components/src/main/java/com/google/firebase/components/AbstractComponentContainer.java deleted file mode 100644 index 143adf0acc7..00000000000 --- a/firebase-components/src/main/java/com/google/firebase/components/AbstractComponentContainer.java +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2018 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.firebase.components; - -import com.google.firebase.inject.Provider; -import java.util.Set; - -abstract class AbstractComponentContainer implements ComponentContainer { - @Override - public T get(Class anInterface) { - Provider provider = getProvider(anInterface); - if (provider == null) { - return null; - } - return provider.get(); - } - - @Override - public Set setOf(Class anInterface) { - return setOfProvider(anInterface).get(); - } -} diff --git a/firebase-components/src/main/java/com/google/firebase/components/Component.java b/firebase-components/src/main/java/com/google/firebase/components/Component.java index 3e2590fde09..768baa48568 100644 --- a/firebase-components/src/main/java/com/google/firebase/components/Component.java +++ b/firebase-components/src/main/java/com/google/firebase/components/Component.java @@ -80,7 +80,7 @@ public final class Component { } private final String name; - private final Set> providedInterfaces; + private final Set> providedInterfaces; private final Set dependencies; private final @Instantiation int instantiation; private final @ComponentType int type; @@ -89,7 +89,7 @@ public final class Component { private Component( @Nullable String name, - Set> providedInterfaces, + Set> providedInterfaces, Set dependencies, @Instantiation int instantiation, @ComponentType int type, @@ -119,7 +119,7 @@ public String getName() { * *

Note: T conforms to all of these interfaces. */ - public Set> getProvidedInterfaces() { + public Set> getProvidedInterfaces() { return providedInterfaces; } @@ -202,6 +202,18 @@ public static Component.Builder builder( return new Builder<>(anInterface, additionalInterfaces); } + /** Returns a Component builder. */ + public static Component.Builder builder(Qualified anInterface) { + return new Builder<>(anInterface); + } + + /** Returns a Component builder. */ + @SafeVarargs + public static Component.Builder builder( + Qualified anInterface, Qualified... additionalInterfaces) { + return new Builder<>(anInterface, additionalInterfaces); + } + /** * Wraps a value in a {@link Component} with no dependencies. * @@ -219,6 +231,13 @@ public static Component of( return builder(anInterface, additionalInterfaces).factory((args) -> value).build(); } + /** Wraps a value in a {@link Component} with no dependencies. */ + @SafeVarargs + public static Component of( + T value, Qualified anInterface, Qualified... additionalInterfaces) { + return builder(anInterface, additionalInterfaces).factory((args) -> value).build(); + } + /** * Provides a builder for a {@link Set}-multibinding {@link Component}. * @@ -229,6 +248,16 @@ public static Component.Builder intoSetBuilder(Class anInterface) { return builder(anInterface).intoSet(); } + /** + * Provides a builder for a {@link Set}-multibinding {@link Component}. + * + *

Such components can be requested by dependents via {@link ComponentContainer#setOf(Class)} * + * or {@link ComponentContainer#setOfProvider(Class)}. + */ + public static Component.Builder intoSetBuilder(Qualified anInterface) { + return builder(anInterface).intoSet(); + } + /** * Wraps a value in a {@link Set}-multibinding {@link Component} with no dependencies. * * @@ -239,22 +268,42 @@ public static Component intoSet(T value, Class anInterface) { return intoSetBuilder(anInterface).factory(c -> value).build(); } + /** + * Wraps a value in a {@link Set}-multibinding {@link Component} with no dependencies. * + * + *

Such components can be requested by dependents via {@link ComponentContainer#setOf(Class)} * + * or {@link ComponentContainer#setOfProvider(Class)}. + */ + public static Component intoSet(T value, Qualified anInterface) { + return intoSetBuilder(anInterface).factory(c -> value).build(); + } + /** FirebaseComponent builder. */ public static class Builder { private String name = null; - private final Set> providedInterfaces = new HashSet<>(); + private final Set> providedInterfaces = new HashSet<>(); private final Set dependencies = new HashSet<>(); private @Instantiation int instantiation = Instantiation.LAZY; private @ComponentType int type = ComponentType.VALUE; private ComponentFactory factory; - private Set> publishedEvents = new HashSet<>(); + private final Set> publishedEvents = new HashSet<>(); @SafeVarargs private Builder(Class anInterface, Class... additionalInterfaces) { Preconditions.checkNotNull(anInterface, "Null interface"); - providedInterfaces.add(anInterface); + providedInterfaces.add(Qualified.unqualified(anInterface)); for (Class iface : additionalInterfaces) { Preconditions.checkNotNull(iface, "Null interface"); + providedInterfaces.add(Qualified.unqualified(iface)); + } + } + + @SafeVarargs + private Builder(Qualified anInterface, Qualified... additionalInterfaces) { + Preconditions.checkNotNull(anInterface, "Null interface"); + providedInterfaces.add(anInterface); + for (Qualified iface : additionalInterfaces) { + Preconditions.checkNotNull(iface, "Null interface"); } Collections.addAll(providedInterfaces, additionalInterfaces); } @@ -301,7 +350,7 @@ private Builder setInstantiation(@Instantiation int instantiation) { return this; } - private void validateInterface(Class anInterface) { + private void validateInterface(Qualified anInterface) { Preconditions.checkArgument( !providedInterfaces.contains(anInterface), "Components are not allowed to depend on interfaces they themselves provide."); diff --git a/firebase-components/src/main/java/com/google/firebase/components/ComponentContainer.java b/firebase-components/src/main/java/com/google/firebase/components/ComponentContainer.java index d5c349eea9c..0f301eb0e19 100644 --- a/firebase-components/src/main/java/com/google/firebase/components/ComponentContainer.java +++ b/firebase-components/src/main/java/com/google/firebase/components/ComponentContainer.java @@ -20,13 +20,41 @@ /** Provides a means to retrieve instances of requested classes/interfaces. */ public interface ComponentContainer { - T get(Class anInterface); + default T get(Class anInterface) { + return get(Qualified.unqualified(anInterface)); + } - Provider getProvider(Class anInterface); + default Provider getProvider(Class anInterface) { + return getProvider(Qualified.unqualified(anInterface)); + } - Deferred getDeferred(Class anInterface); + default Deferred getDeferred(Class anInterface) { + return getDeferred(Qualified.unqualified(anInterface)); + } - Set setOf(Class anInterface); + default Set setOf(Class anInterface) { + return setOf(Qualified.unqualified(anInterface)); + } - Provider> setOfProvider(Class anInterface); + default Provider> setOfProvider(Class anInterface) { + return setOfProvider(Qualified.unqualified(anInterface)); + } + + default T get(Qualified anInterface) { + Provider provider = getProvider(anInterface); + if (provider == null) { + return null; + } + return provider.get(); + } + + Provider getProvider(Qualified anInterface); + + Deferred getDeferred(Qualified anInterface); + + default Set setOf(Qualified anInterface) { + return setOfProvider(anInterface).get(); + } + + Provider> setOfProvider(Qualified anInterface); } diff --git a/firebase-components/src/main/java/com/google/firebase/components/ComponentRuntime.java b/firebase-components/src/main/java/com/google/firebase/components/ComponentRuntime.java index 895b19fff15..a560cf5b502 100644 --- a/firebase-components/src/main/java/com/google/firebase/components/ComponentRuntime.java +++ b/firebase-components/src/main/java/com/google/firebase/components/ComponentRuntime.java @@ -43,11 +43,11 @@ *

Does {@link Component} dependency resolution and provides access to resolved {@link * Component}s via {@link #get(Class)} method. */ -public class ComponentRuntime extends AbstractComponentContainer implements ComponentLoader { +public class ComponentRuntime implements ComponentContainer, ComponentLoader { private static final Provider> EMPTY_PROVIDER = Collections::emptySet; private final Map, Provider> components = new HashMap<>(); - private final Map, Provider> lazyInstanceMap = new HashMap<>(); - private final Map, LazySet> lazySetMap = new HashMap<>(); + private final Map, Provider> lazyInstanceMap = new HashMap<>(); + private final Map, LazySet> lazySetMap = new HashMap<>(); private final List> unprocessedRegistrarProviders; private final EventBus eventBus; private final AtomicReference eagerComponentsInitializedWith = new AtomicReference<>(); @@ -184,7 +184,7 @@ private List processInstanceComponents(List> componentsTo } Provider provider = components.get(component); - for (Class anInterface : component.getProvidedInterfaces()) { + for (Qualified anInterface : component.getProvidedInterfaces()) { if (!lazyInstanceMap.containsKey(anInterface)) { lazyInstanceMap.put(anInterface, provider); } else { @@ -203,7 +203,7 @@ private List processInstanceComponents(List> componentsTo /** Populates lazySetMap to make set components available for consumption via set dependencies. */ private List processSetComponents() { ArrayList runnables = new ArrayList<>(); - Map, Set>> setIndex = new HashMap<>(); + Map, Set>> setIndex = new HashMap<>(); for (Map.Entry, Provider> entry : components.entrySet()) { Component component = entry.getKey(); @@ -214,7 +214,7 @@ private List processSetComponents() { Provider provider = entry.getValue(); - for (Class anInterface : component.getProvidedInterfaces()) { + for (Qualified anInterface : component.getProvidedInterfaces()) { if (!setIndex.containsKey(anInterface)) { setIndex.put(anInterface, new HashSet<>()); } @@ -222,7 +222,7 @@ private List processSetComponents() { } } - for (Map.Entry, Set>> entry : setIndex.entrySet()) { + for (Map.Entry, Set>> entry : setIndex.entrySet()) { if (!lazySetMap.containsKey(entry.getKey())) { lazySetMap.put(entry.getKey(), LazySet.fromCollection(entry.getValue())); } else { @@ -240,13 +240,13 @@ private List processSetComponents() { @Override @SuppressWarnings("unchecked") - public synchronized Provider getProvider(Class anInterface) { + public synchronized Provider getProvider(Qualified anInterface) { Preconditions.checkNotNull(anInterface, "Null interface requested."); return (Provider) lazyInstanceMap.get(anInterface); } @Override - public Deferred getDeferred(Class anInterface) { + public Deferred getDeferred(Qualified anInterface) { Provider provider = getProvider(anInterface); if (provider == null) { return OptionalProvider.empty(); @@ -259,7 +259,7 @@ public Deferred getDeferred(Class anInterface) { @Override @SuppressWarnings("unchecked") - public synchronized Provider> setOfProvider(Class anInterface) { + public synchronized Provider> setOfProvider(Qualified anInterface) { LazySet provider = lazySetMap.get(anInterface); if (provider != null) { return (Provider>) (Provider) provider; diff --git a/firebase-components/src/main/java/com/google/firebase/components/CycleDetector.java b/firebase-components/src/main/java/com/google/firebase/components/CycleDetector.java index a435d6f3a8c..38d7124c11c 100644 --- a/firebase-components/src/main/java/com/google/firebase/components/CycleDetector.java +++ b/firebase-components/src/main/java/com/google/firebase/components/CycleDetector.java @@ -24,10 +24,10 @@ /** Cycle detector for the {@link Component} dependency graph. */ class CycleDetector { private static class Dep { - private final Class anInterface; + private final Qualified anInterface; private final boolean set; - private Dep(Class anInterface, boolean set) { + private Dep(Qualified anInterface, boolean set) { this.anInterface = anInterface; this.set = set; } @@ -135,7 +135,7 @@ private static Set toGraph(List> components) { Map> componentIndex = new HashMap<>(components.size()); for (Component component : components) { ComponentNode node = new ComponentNode(component); - for (Class anInterface : component.getProvidedInterfaces()) { + for (Qualified anInterface : component.getProvidedInterfaces()) { Dep cmp = new Dep(anInterface, !component.isValue()); if (!componentIndex.containsKey(cmp)) { componentIndex.put(cmp, new HashSet<>()); diff --git a/firebase-components/src/main/java/com/google/firebase/components/Dependency.java b/firebase-components/src/main/java/com/google/firebase/components/Dependency.java index ed8a3c3b66d..9c9ef676a75 100644 --- a/firebase-components/src/main/java/com/google/firebase/components/Dependency.java +++ b/firebase-components/src/main/java/com/google/firebase/components/Dependency.java @@ -37,11 +37,15 @@ public final class Dependency { int DEFERRED = 2; } - private final Class anInterface; - private final @Type int type; + private final Qualified anInterface; + @Type private final int type; private final @Injection int injection; private Dependency(Class anInterface, @Type int type, @Injection int injection) { + this(Qualified.unqualified(anInterface), type, injection); + } + + private Dependency(Qualified anInterface, @Type int type, @Injection int injection) { this.anInterface = Preconditions.checkNotNull(anInterface, "Null dependency anInterface."); this.type = type; this.injection = injection; @@ -71,6 +75,16 @@ public static Dependency deferred(Class anInterface) { return new Dependency(anInterface, Type.OPTIONAL, Injection.DEFERRED); } + /** + * Declares a deferred dependency. + * + *

Such dependencies are optional and may not be present by default. But they can become + * available if a dynamic module that contains them is installed. + */ + public static Dependency deferred(Qualified anInterface) { + return new Dependency(anInterface, Type.OPTIONAL, Injection.DEFERRED); + } + /** * Declares a required dependency. * @@ -83,6 +97,18 @@ public static Dependency required(Class anInterface) { return new Dependency(anInterface, Type.REQUIRED, Injection.DIRECT); } + /** + * Declares a required dependency. + * + *

Such dependencies must be present in order for the dependent component to function. Any + * component with a required dependency should also declare a Maven dependency on an SDK that + * provides it. Failing to do so will result in a {@link MissingDependencyException} to be thrown + * at runtime. + */ + public static Dependency required(Qualified anInterface) { + return new Dependency(anInterface, Type.REQUIRED, Injection.DIRECT); + } + /** * Declares a Set multi-binding dependency. * @@ -94,6 +120,17 @@ public static Dependency setOf(Class anInterface) { return new Dependency(anInterface, Type.SET, Injection.DIRECT); } + /** + * Declares a Set multi-binding dependency. + * + *

Such dependencies provide access to a {@code Set} to dependent components. Note that + * the set is only filled with components that explicitly declare the intent to be a "set" + * dependency via {@link Component#intoSet(Object, Class)}. + */ + public static Dependency setOf(Qualified anInterface) { + return new Dependency(anInterface, Type.SET, Injection.DIRECT); + } + /** * Declares an optional dependency. * @@ -104,6 +141,16 @@ public static Dependency optionalProvider(Class anInterface) { return new Dependency(anInterface, Type.OPTIONAL, Injection.PROVIDER); } + /** + * Declares an optional dependency. + * + *

Optional dependencies can be missing at runtime(being {@code null}) and dependents must be + * ready to handle that. + */ + public static Dependency optionalProvider(Qualified anInterface) { + return new Dependency(anInterface, Type.OPTIONAL, Injection.PROVIDER); + } + /** * Declares a required dependency. * @@ -116,6 +163,18 @@ public static Dependency requiredProvider(Class anInterface) { return new Dependency(anInterface, Type.REQUIRED, Injection.PROVIDER); } + /** + * Declares a required dependency. + * + *

Such dependencies must be present in order for the dependent component to function. Any + * component with a required dependency should also declare a Maven dependency on an SDK that + * provides it. Failing to do so will result in a {@link MissingDependencyException} to be thrown + * at runtime. + */ + public static Dependency requiredProvider(Qualified anInterface) { + return new Dependency(anInterface, Type.REQUIRED, Injection.PROVIDER); + } + /** * Declares a Set multi-binding dependency. * @@ -127,7 +186,18 @@ public static Dependency setOfProvider(Class anInterface) { return new Dependency(anInterface, Type.SET, Injection.PROVIDER); } - public Class getInterface() { + /** + * Declares a Set multi-binding dependency. + * + *

Such dependencies provide access to a {@code Set} to dependent components. Note that + * the set is only filled with components that explicitly declare the intent to be a "set" + * dependency via {@link Component#intoSet(Object, Class)}. + */ + public static Dependency setOfProvider(Qualified anInterface) { + return new Dependency(anInterface, Type.SET, Injection.PROVIDER); + } + + public Qualified getInterface() { return anInterface; } @@ -151,7 +221,9 @@ public boolean isDeferred() { public boolean equals(Object o) { if (o instanceof Dependency) { Dependency other = (Dependency) o; - return anInterface == other.anInterface && type == other.type && injection == other.injection; + return anInterface.equals(other.anInterface) + && type == other.type + && injection == other.injection; } return false; } diff --git a/firebase-components/src/main/java/com/google/firebase/components/Qualified.java b/firebase-components/src/main/java/com/google/firebase/components/Qualified.java new file mode 100644 index 00000000000..a0b755ac583 --- /dev/null +++ b/firebase-components/src/main/java/com/google/firebase/components/Qualified.java @@ -0,0 +1,64 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.components; + +import java.lang.annotation.Annotation; + +/** Represents a qualified class object. */ +public final class Qualified { + private @interface Unqualified {} + + private final Class qualifier; + private final Class type; + + public Qualified(Class qualifier, Class type) { + this.qualifier = qualifier; + this.type = type; + } + + public static Qualified unqualified(Class type) { + return new Qualified<>(Unqualified.class, type); + } + + public static Qualified qualified(Class qualifier, Class type) { + return new Qualified<>(qualifier, type); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + Qualified qualified = (Qualified) o; + + if (!type.equals(qualified.type)) return false; + return qualifier.equals(qualified.qualifier); + } + + @Override + public int hashCode() { + int result = type.hashCode(); + result = 31 * result + qualifier.hashCode(); + return result; + } + + @Override + public String toString() { + if (qualifier == Unqualified.class) { + return type.getName(); + } + return "@" + qualifier.getName() + " " + type.getName(); + } +} diff --git a/firebase-components/src/main/java/com/google/firebase/components/RestrictedComponentContainer.java b/firebase-components/src/main/java/com/google/firebase/components/RestrictedComponentContainer.java index 295dc3eb7cc..0c051e83a5b 100644 --- a/firebase-components/src/main/java/com/google/firebase/components/RestrictedComponentContainer.java +++ b/firebase-components/src/main/java/com/google/firebase/components/RestrictedComponentContainer.java @@ -26,21 +26,21 @@ * An implementation of {@link ComponentContainer} that is backed by another delegate {@link * ComponentContainer} and restricts access to only declared {@link Dependency dependencies}. */ -final class RestrictedComponentContainer extends AbstractComponentContainer { - private final Set> allowedDirectInterfaces; - private final Set> allowedProviderInterfaces; - private final Set> allowedDeferredInterfaces; - private final Set> allowedSetDirectInterfaces; - private final Set> allowedSetProviderInterfaces; +final class RestrictedComponentContainer implements ComponentContainer { + private final Set> allowedDirectInterfaces; + private final Set> allowedProviderInterfaces; + private final Set> allowedDeferredInterfaces; + private final Set> allowedSetDirectInterfaces; + private final Set> allowedSetProviderInterfaces; private final Set> allowedPublishedEvents; private final ComponentContainer delegateContainer; RestrictedComponentContainer(Component component, ComponentContainer container) { - Set> directInterfaces = new HashSet<>(); - Set> providerInterfaces = new HashSet<>(); - Set> deferredInterfaces = new HashSet<>(); - Set> setDirectInterfaces = new HashSet<>(); - Set> setProviderInterfaces = new HashSet<>(); + Set> directInterfaces = new HashSet<>(); + Set> providerInterfaces = new HashSet<>(); + Set> deferredInterfaces = new HashSet<>(); + Set> setDirectInterfaces = new HashSet<>(); + Set> setProviderInterfaces = new HashSet<>(); for (Dependency dependency : component.getDependencies()) { if (dependency.isDirectInjection()) { if (dependency.isSet()) { @@ -59,7 +59,7 @@ final class RestrictedComponentContainer extends AbstractComponentContainer { } } if (!component.getPublishedEvents().isEmpty()) { - directInterfaces.add(Publisher.class); + directInterfaces.add(Qualified.unqualified(Publisher.class)); } allowedDirectInterfaces = Collections.unmodifiableSet(directInterfaces); allowedProviderInterfaces = Collections.unmodifiableSet(providerInterfaces); @@ -77,7 +77,7 @@ final class RestrictedComponentContainer extends AbstractComponentContainer { */ @Override public T get(Class anInterface) { - if (!allowedDirectInterfaces.contains(anInterface)) { + if (!allowedDirectInterfaces.contains(Qualified.unqualified(anInterface))) { throw new DependencyException( String.format("Attempting to request an undeclared dependency %s.", anInterface)); } @@ -96,6 +96,15 @@ public T get(Class anInterface) { return publisher; } + @Override + public T get(Qualified anInterface) { + if (!allowedDirectInterfaces.contains(anInterface)) { + throw new DependencyException( + String.format("Attempting to request an undeclared dependency %s.", anInterface)); + } + return delegateContainer.get(anInterface); + } + /** * Returns an instance of the provider for the requested class if it is allowed. * @@ -103,6 +112,26 @@ public T get(Class anInterface) { */ @Override public Provider getProvider(Class anInterface) { + return getProvider(Qualified.unqualified(anInterface)); + } + + @Override + public Deferred getDeferred(Class anInterface) { + return getDeferred(Qualified.unqualified(anInterface)); + } + + /** + * Returns an instance of the provider for the set of requested classes if it is allowed. + * + * @throws DependencyException otherwise. + */ + @Override + public Provider> setOfProvider(Class anInterface) { + return setOfProvider(Qualified.unqualified(anInterface)); + } + + @Override + public Provider getProvider(Qualified anInterface) { if (!allowedProviderInterfaces.contains(anInterface)) { throw new DependencyException( String.format( @@ -112,7 +141,7 @@ public Provider getProvider(Class anInterface) { } @Override - public Deferred getDeferred(Class anInterface) { + public Deferred getDeferred(Qualified anInterface) { if (!allowedDeferredInterfaces.contains(anInterface)) { throw new DependencyException( String.format( @@ -121,13 +150,8 @@ public Deferred getDeferred(Class anInterface) { return delegateContainer.getDeferred(anInterface); } - /** - * Returns an instance of the provider for the set of requested classes if it is allowed. - * - * @throws DependencyException otherwise. - */ @Override - public Provider> setOfProvider(Class anInterface) { + public Provider> setOfProvider(Qualified anInterface) { if (!allowedSetProviderInterfaces.contains(anInterface)) { throw new DependencyException( String.format( @@ -142,7 +166,7 @@ public Provider> setOfProvider(Class anInterface) { * @throws DependencyException otherwise. */ @Override - public Set setOf(Class anInterface) { + public Set setOf(Qualified anInterface) { if (!allowedSetDirectInterfaces.contains(anInterface)) { throw new DependencyException( String.format("Attempting to request an undeclared dependency Set<%s>.", anInterface)); diff --git a/firebase-components/src/test/java/com/google/firebase/components/ComponentRuntimeTest.java b/firebase-components/src/test/java/com/google/firebase/components/ComponentRuntimeTest.java index ed5087cbdc9..8e445aee84a 100644 --- a/firebase-components/src/test/java/com/google/firebase/components/ComponentRuntimeTest.java +++ b/firebase-components/src/test/java/com/google/firebase/components/ComponentRuntimeTest.java @@ -15,6 +15,8 @@ package com.google.firebase.components; import static com.google.common.truth.Truth.assertThat; +import static com.google.firebase.components.Qualified.qualified; +import static com.google.firebase.components.Qualified.unqualified; import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; @@ -33,6 +35,10 @@ @RunWith(JUnit4.class) public final class ComponentRuntimeTest { + private @interface Qualifier1 {} + + private @interface Qualifier2 {} + private static final Executor EXECUTOR = Runnable::run; interface ComponentOne { @@ -210,6 +216,21 @@ public void container_withMultipleComponentsRegisteredForSameInterface_shouldThr } } + @Test + public void + container_withMultipleComponentsRegisteredForSameInterfaceButQualified_shouldNotThrow() { + ComponentRuntime runtime = + ComponentRuntime.builder(EXECUTOR) + .addComponent(Component.of(1, Integer.class)) + .addComponent(Component.of(2, qualified(Qualifier1.class, Integer.class))) + .addComponent(Component.of(3, qualified(Qualifier2.class, Integer.class))) + .build(); + + assertThat(runtime.get(Integer.class)).isEqualTo(1); + assertThat(runtime.get(qualified(Qualifier1.class, Integer.class))).isEqualTo(2); + assertThat(runtime.get(qualified(Qualifier2.class, Integer.class))).isEqualTo(3); + } + @Test public void container_withMissingDependencies_shouldThrow() { try { @@ -265,7 +286,7 @@ public void container_withCyclicProviderDependency_shouldProperlyInitialize() { public void get_withNullInterface_shouldThrow() { ComponentRuntime runtime = ComponentRuntime.builder(EXECUTOR).build(); try { - runtime.get(null); + runtime.get((Qualified) null); fail("Expected exception not thrown."); } catch (NullPointerException ex) { // success. @@ -350,6 +371,25 @@ public void setComponents_shouldNotPreventValueComponentsFromBeingRegistered() { assertThat(runtime.get(Double.class)).isEqualTo(4d); } + @Test + public void setComponents_withQualifiers_shouldContributeToAppropriateSets() { + ComponentRuntime runtime = + ComponentRuntime.builder(EXECUTOR) + .addComponent(Component.of(5, Integer.class)) + .addComponent(Component.intoSet(1, Integer.class)) + .addComponent(Component.intoSet(3, Integer.class)) + .addComponent(Component.intoSet(1, qualified(Qualifier1.class, Integer.class))) + .addComponent(Component.intoSet(2, qualified(Qualifier1.class, Integer.class))) + .addComponent(Component.intoSet(3, qualified(Qualifier2.class, Integer.class))) + .addComponent(Component.intoSet(4, qualified(Qualifier2.class, Integer.class))) + .build(); + + assertThat(runtime.get(Integer.class)).isEqualTo(5); + assertThat(runtime.setOf(Integer.class)).containsExactly(1, 3); + assertThat(runtime.setOf(qualified(Qualifier1.class, Integer.class))).containsExactly(1, 2); + assertThat(runtime.setOf(qualified(Qualifier2.class, Integer.class))).containsExactly(3, 4); + } + private static class DependsOnString { private final Provider dep; @@ -607,8 +647,8 @@ public void container_withComponentProcessor_shouldDelegateToItForEachComponentR runtime.getAllComponentsForTest().stream() .filter( c -> - (c.getProvidedInterfaces().contains(ComponentOne.class) - || c.getProvidedInterfaces().contains(ComponentTwo.class))) + (c.getProvidedInterfaces().contains(unqualified(ComponentOne.class)) + || c.getProvidedInterfaces().contains(unqualified(ComponentTwo.class)))) .allMatch(c -> c.getFactory() == replacedFactory)) .isTrue(); } diff --git a/firebase-components/src/test/java/com/google/firebase/components/ComponentTest.java b/firebase-components/src/test/java/com/google/firebase/components/ComponentTest.java index 92cc6cf3588..13549495ef8 100644 --- a/firebase-components/src/test/java/com/google/firebase/components/ComponentTest.java +++ b/firebase-components/src/test/java/com/google/firebase/components/ComponentTest.java @@ -15,6 +15,8 @@ package com.google.firebase.components; import static com.google.common.truth.Truth.assertThat; +import static com.google.firebase.components.Qualified.qualified; +import static com.google.firebase.components.Qualified.unqualified; import static org.junit.Assert.fail; import java.math.BigDecimal; @@ -25,6 +27,8 @@ @RunWith(JUnit4.class) public class ComponentTest { + @interface TestQualifier {} + interface TestInterface {} private static class TestClass implements TestInterface {} @@ -36,7 +40,7 @@ public void of_withMultipleInterfaces_shouldSetCorrectDefaults() { TestClass testClass = new TestClass(); Component component = Component.of(testClass, TestClass.class, TestInterface.class); assertThat(component.getProvidedInterfaces()) - .containsExactly(TestClass.class, TestInterface.class); + .containsExactly(unqualified(TestClass.class), unqualified(TestInterface.class)); assertThat(component.isLazy()).isTrue(); assertThat(component.isValue()).isTrue(); assertThat(component.isAlwaysEager()).isFalse(); @@ -49,7 +53,22 @@ public void of_withMultipleInterfaces_shouldSetCorrectDefaults() { public void builder_shouldSetCorrectDefaults() { Component component = Component.builder(TestClass.class).factory(nullFactory).build(); - assertThat(component.getProvidedInterfaces()).containsExactly(TestClass.class); + assertThat(component.getProvidedInterfaces()).containsExactly(unqualified(TestClass.class)); + assertThat(component.isLazy()).isTrue(); + assertThat(component.isValue()).isTrue(); + assertThat(component.isAlwaysEager()).isFalse(); + assertThat(component.isEagerInDefaultApp()).isFalse(); + assertThat(component.getDependencies()).isEmpty(); + } + + @Test + public void qualifiedBuilder_shouldSetCorrectDefaults() { + Component component = + Component.builder(qualified(TestQualifier.class, TestClass.class)) + .factory(nullFactory) + .build(); + assertThat(component.getProvidedInterfaces()) + .containsExactly(qualified(TestQualifier.class, TestClass.class)); assertThat(component.isLazy()).isTrue(); assertThat(component.isValue()).isTrue(); assertThat(component.isAlwaysEager()).isFalse(); @@ -61,7 +80,7 @@ public void builder_shouldSetCorrectDefaults() { public void intoSetBuilder_shouldSetCorrectDefaults() { Component component = Component.intoSetBuilder(TestClass.class).factory(nullFactory).build(); - assertThat(component.getProvidedInterfaces()).containsExactly(TestClass.class); + assertThat(component.getProvidedInterfaces()).containsExactly(unqualified(TestClass.class)); assertThat(component.isLazy()).isTrue(); assertThat(component.isValue()).isFalse(); assertThat(component.isAlwaysEager()).isFalse(); @@ -73,7 +92,7 @@ public void intoSetBuilder_shouldSetCorrectDefaults() { public void intoSet_shouldSetCorrectDefaults() { TestClass testClass = new TestClass(); Component component = Component.intoSet(testClass, TestClass.class); - assertThat(component.getProvidedInterfaces()).containsExactly(TestClass.class); + assertThat(component.getProvidedInterfaces()).containsExactly(unqualified(TestClass.class)); assertThat(component.isLazy()).isTrue(); assertThat(component.isValue()).isFalse(); assertThat(component.isAlwaysEager()).isFalse(); @@ -174,7 +193,21 @@ public void builder_withMultipleInterfaces_shouldProperlySetInterfaces() { Component component = Component.builder(TestClass.class, TestInterface.class).factory(nullFactory).build(); assertThat(component.getProvidedInterfaces()) - .containsExactly(TestClass.class, TestInterface.class); + .containsExactly(unqualified(TestClass.class), unqualified(TestInterface.class)); + } + + @Test + public void builder_withMultipleQualifiedInterfaces_shouldProperlySetInterfaces() { + Component component = + Component.builder( + qualified(TestQualifier.class, TestClass.class), + qualified(TestQualifier.class, TestInterface.class)) + .factory(nullFactory) + .build(); + assertThat(component.getProvidedInterfaces()) + .containsExactly( + qualified(TestQualifier.class, TestClass.class), + qualified(TestQualifier.class, TestInterface.class)); } @Test diff --git a/firebase-components/src/test/java/com/google/firebase/components/CycleDetectorTest.java b/firebase-components/src/test/java/com/google/firebase/components/CycleDetectorTest.java index 091b7be8b08..8fc904741fe 100644 --- a/firebase-components/src/test/java/com/google/firebase/components/CycleDetectorTest.java +++ b/firebase-components/src/test/java/com/google/firebase/components/CycleDetectorTest.java @@ -15,11 +15,13 @@ package com.google.firebase.components; import static com.google.common.truth.Truth.assertThat; +import static com.google.firebase.components.Qualified.qualified; import static junit.framework.Assert.fail; import java.util.Arrays; import java.util.Collections; import java.util.List; +import javax.inject.Qualifier; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -246,7 +248,7 @@ public void detect_withProviderDependencyCycle_shouldNotThrow() { } @Test - public void detect_withMultipleComponentsImplementingSameIface_shouldThrow() { + public void detect_withMultipleComponentsImplementingSameInterface_shouldThrow() { List> components = Arrays.asList( Component.builder(TestInterface1.class).factory(nullFactory()).build(), @@ -260,6 +262,45 @@ public void detect_withMultipleComponentsImplementingSameIface_shouldThrow() { } } + @Qualifier + @interface Qualifier1 {} + + @Qualifier + @interface Qualifier2 {} + + @Test + public void detect_withMultipleComponentsImplementingSameQualifiedInterface_shouldNotThrow() { + List> components = + Arrays.asList( + Component.builder(TestInterface1.class).factory(nullFactory()).build(), + Component.builder(qualified(Qualifier1.class, TestInterface1.class)) + .factory(nullFactory()) + .build(), + Component.builder(qualified(Qualifier2.class, TestInterface1.class)) + .factory(nullFactory()) + .build()); + + CycleDetector.detect(components); + } + + @Test + public void + detect_withMultipleComponentsImplementingSameQualifiedInterfaceAndNoDepCycle_shouldNotThrow() { + List> components = + Arrays.asList( + Component.builder(TestInterface1.class).factory(nullFactory()).build(), + Component.builder(qualified(Qualifier1.class, TestInterface1.class)) + .add(Dependency.required(TestInterface1.class)) + .factory(nullFactory()) + .build(), + Component.builder(qualified(Qualifier2.class, TestInterface1.class)) + .add(Dependency.required(qualified(Qualifier1.class, TestInterface1.class))) + .factory(nullFactory()) + .build()); + + CycleDetector.detect(components); + } + private static void detect(List> components) { Collections.shuffle(components); try { diff --git a/firebase-components/src/test/java/com/google/firebase/components/DependencyTest.java b/firebase-components/src/test/java/com/google/firebase/components/DependencyTest.java index 0c3fdaca99c..9c13a0a9fd8 100644 --- a/firebase-components/src/test/java/com/google/firebase/components/DependencyTest.java +++ b/firebase-components/src/test/java/com/google/firebase/components/DependencyTest.java @@ -15,11 +15,17 @@ package com.google.firebase.components; import static com.google.common.truth.Truth.assertThat; +import static com.google.firebase.components.Qualified.qualified; +import static com.google.firebase.components.Qualified.unqualified; +import javax.inject.Qualifier; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +@Qualifier +@interface TestQualifier {} + @RunWith(JUnit4.class) public class DependencyTest { @Test @@ -29,7 +35,7 @@ public void optional_shouldHaveExpectedInvariants() { assertThat(dependency.isRequired()).isFalse(); assertThat(dependency.isSet()).isFalse(); assertThat(dependency.isDirectInjection()).isTrue(); - assertThat(dependency.getInterface()).isEqualTo(String.class); + assertThat(dependency.getInterface()).isEqualTo(unqualified(String.class)); } @Test @@ -39,7 +45,17 @@ public void required_shouldHaveExpectedInvariants() { assertThat(dependency.isRequired()).isTrue(); assertThat(dependency.isSet()).isFalse(); assertThat(dependency.isDirectInjection()).isTrue(); - assertThat(dependency.getInterface()).isEqualTo(String.class); + assertThat(dependency.getInterface()).isEqualTo(unqualified(String.class)); + } + + @Test + public void requiredQualified_shouldHaveExpectedInvariants() { + Dependency dependency = Dependency.required(qualified(TestQualifier.class, String.class)); + + assertThat(dependency.isRequired()).isTrue(); + assertThat(dependency.isSet()).isFalse(); + assertThat(dependency.isDirectInjection()).isTrue(); + assertThat(dependency.getInterface()).isEqualTo(qualified(TestQualifier.class, String.class)); } @Test @@ -49,7 +65,17 @@ public void setOf_shouldHaveExpectedInvariants() { assertThat(dependency.isRequired()).isFalse(); assertThat(dependency.isSet()).isTrue(); assertThat(dependency.isDirectInjection()).isTrue(); - assertThat(dependency.getInterface()).isEqualTo(String.class); + assertThat(dependency.getInterface()).isEqualTo(unqualified(String.class)); + } + + @Test + public void setOfQualified_shouldHaveExpectedInvariants() { + Dependency dependency = Dependency.setOf(qualified(TestQualifier.class, String.class)); + + assertThat(dependency.isRequired()).isFalse(); + assertThat(dependency.isSet()).isTrue(); + assertThat(dependency.isDirectInjection()).isTrue(); + assertThat(dependency.getInterface()).isEqualTo(qualified(TestQualifier.class, String.class)); } @Test @@ -59,7 +85,18 @@ public void optionalProvider_shouldHaveExpectedInvariants() { assertThat(dependency.isRequired()).isFalse(); assertThat(dependency.isSet()).isFalse(); assertThat(dependency.isDirectInjection()).isFalse(); - assertThat(dependency.getInterface()).isEqualTo(String.class); + assertThat(dependency.getInterface()).isEqualTo(unqualified(String.class)); + } + + @Test + public void optionalProviderQualified_shouldHaveExpectedInvariants() { + Dependency dependency = + Dependency.optionalProvider(qualified(TestQualifier.class, String.class)); + + assertThat(dependency.isRequired()).isFalse(); + assertThat(dependency.isSet()).isFalse(); + assertThat(dependency.isDirectInjection()).isFalse(); + assertThat(dependency.getInterface()).isEqualTo(qualified(TestQualifier.class, String.class)); } @Test @@ -69,7 +106,18 @@ public void requiredProvider_shouldHaveExpectedInvariants() { assertThat(dependency.isRequired()).isTrue(); assertThat(dependency.isSet()).isFalse(); assertThat(dependency.isDirectInjection()).isFalse(); - assertThat(dependency.getInterface()).isEqualTo(String.class); + assertThat(dependency.getInterface()).isEqualTo(unqualified(String.class)); + } + + @Test + public void requiredProviderQualified_shouldHaveExpectedInvariants() { + Dependency dependency = + Dependency.requiredProvider(qualified(TestQualifier.class, String.class)); + + assertThat(dependency.isRequired()).isTrue(); + assertThat(dependency.isSet()).isFalse(); + assertThat(dependency.isDirectInjection()).isFalse(); + assertThat(dependency.getInterface()).isEqualTo(qualified(TestQualifier.class, String.class)); } @Test @@ -79,6 +127,16 @@ public void setOfProvider_shouldHaveExpectedInvariants() { assertThat(dependency.isRequired()).isFalse(); assertThat(dependency.isSet()).isTrue(); assertThat(dependency.isDirectInjection()).isFalse(); - assertThat(dependency.getInterface()).isEqualTo(String.class); + assertThat(dependency.getInterface()).isEqualTo(unqualified(String.class)); + } + + @Test + public void setOfProviderQualified_shouldHaveExpectedInvariants() { + Dependency dependency = Dependency.setOfProvider(qualified(TestQualifier.class, String.class)); + + assertThat(dependency.isRequired()).isFalse(); + assertThat(dependency.isSet()).isTrue(); + assertThat(dependency.isDirectInjection()).isFalse(); + assertThat(dependency.getInterface()).isEqualTo(qualified(TestQualifier.class, String.class)); } } diff --git a/firebase-components/src/test/java/com/google/firebase/components/RestrictedComponentContainerTest.java b/firebase-components/src/test/java/com/google/firebase/components/RestrictedComponentContainerTest.java index bda8a135cd5..8367780f302 100644 --- a/firebase-components/src/test/java/com/google/firebase/components/RestrictedComponentContainerTest.java +++ b/firebase-components/src/test/java/com/google/firebase/components/RestrictedComponentContainerTest.java @@ -15,9 +15,11 @@ package com.google.firebase.components; import static com.google.common.truth.Truth.assertThat; +import static com.google.firebase.components.Qualified.unqualified; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -96,10 +98,10 @@ public void get_withPublisher_shouldThrow() { @Test public void getProvider_withAllowedClass_shouldReturnAnInstanceOfThatClass() { Double value = 3.0d; - when(delegate.getProvider(Double.class)).thenReturn(new Lazy<>(value)); + when(delegate.getProvider(unqualified(Double.class))).thenReturn(new Lazy<>(value)); assertThat(container.getProvider(Double.class).get()).isSameInstanceAs(value); - verify(delegate).getProvider(Double.class); + verify(delegate).getProvider(unqualified(Double.class)); } @Test @@ -125,12 +127,16 @@ public void getProvider_withDirectClass_shouldThrow() { @Test public void getDeferred_withAllowedClass_shouldReturnAnInstanceOfThatClass() { Integer value = 3; - when(delegate.getDeferred(Integer.class)).thenReturn(OptionalProvider.of(() -> value)); + when(delegate.getDeferred(unqualified(Integer.class))) + .thenReturn(OptionalProvider.of(() -> value)); AtomicReference returned = new AtomicReference<>(); container.getDeferred(Integer.class).whenAvailable(d -> returned.set(d.get())); assertThat(returned.get()).isSameInstanceAs(value); - verify(delegate).getDeferred(Integer.class); + + container.getDeferred(unqualified(Integer.class)).whenAvailable(d -> returned.set(d.get())); + assertThat(returned.get()).isSameInstanceAs(value); + verify(delegate, times(2)).getDeferred(unqualified(Integer.class)); } @Test @@ -166,10 +172,11 @@ public void getDeferred_withProviderClass_shouldThrow() { @Test public void setOf_withAllowedClass_shouldReturnExpectedSet() { Set set = Collections.emptySet(); - when(delegate.setOf(Long.class)).thenReturn(set); + when(delegate.setOf(unqualified(Long.class))).thenReturn(set); assertThat(container.setOf(Long.class)).isSameInstanceAs(set); - verify(delegate).setOf(Long.class); + assertThat(container.setOf(unqualified(Long.class))).isSameInstanceAs(set); + verify(delegate, times(2)).setOf(unqualified(Long.class)); } @Test @@ -185,10 +192,11 @@ public void setOf_withNotAllowedClass_shouldThrow() { @Test public void setOfProvider_withAllowedClass_shouldReturnExpectedSet() { Set set = Collections.emptySet(); - when(delegate.setOfProvider(Boolean.class)).thenReturn(new Lazy<>(set)); + when(delegate.setOfProvider(unqualified(Boolean.class))).thenReturn(new Lazy<>(set)); assertThat(container.setOfProvider(Boolean.class).get()).isSameInstanceAs(set); - verify(delegate).setOfProvider(Boolean.class); + assertThat(container.setOfProvider(unqualified(Boolean.class)).get()).isSameInstanceAs(set); + verify(delegate, times(2)).setOfProvider(unqualified(Boolean.class)); } @Test From 3cf020ab3e587d98c2e2c343312f26fa9c37a9be Mon Sep 17 00:00:00 2001 From: Vladimir Kryachko Date: Thu, 10 Nov 2022 12:36:43 -0500 Subject: [PATCH 2/5] Register executors as components. (#4288) * Register executors as components. The intent for those is to be used by all Firebase SDKs and forbid creating their own at will. * Add copyrights. * add more copyrights * ktlintformat * gJF * ktlint * Address review comments. --- .../firebase-annotations.gradle | 4 + .../annotations/concurrent/Background.java | 30 +++ .../annotations/concurrent/Blocking.java | 27 +++ .../annotations/concurrent/Lightweight.java | 26 +++ .../annotations/concurrent/UiThread.java | 24 +++ firebase-common/firebase-common.gradle | 1 + firebase-common/ktx/ktx.gradle | 1 + .../com/google/firebase/ktx/Firebase.kt | 36 +++- .../java/com/google/firebase/FirebaseApp.java | 23 +-- .../concurrent/CustomThreadFactory.java | 41 ++++ .../DelegatingScheduledExecutorService.java | 185 ++++++++++++++++++ .../concurrent/DelegatingScheduledFuture.java | 72 +++++++ .../concurrent/ExecutorsRegistrar.java | 102 ++++++++++ .../firebase/concurrent/UiExecutor.java | 31 +++ .../DefaultHeartBeatController.java | 20 +- firebase-common/src/test/AndroidManifest.xml | 25 +++ ...elegatingScheduledExecutorServiceTest.java | 82 ++++++++ .../concurrent/ExecutorComponent.java | 58 ++++++ .../concurrent/ExecutorComponentTest.java | 46 +++++ .../concurrent/ExecutorTestsRegistrar.java | 81 ++++++++ tools/lint/src/main/kotlin/CheckRegistry.kt | 2 + .../src/main/kotlin/ThreadPoolDetector.kt | 64 ++++++ 22 files changed, 947 insertions(+), 34 deletions(-) create mode 100644 firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/Background.java create mode 100644 firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/Blocking.java create mode 100644 firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/Lightweight.java create mode 100644 firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/UiThread.java create mode 100644 firebase-common/src/main/java/com/google/firebase/concurrent/CustomThreadFactory.java create mode 100644 firebase-common/src/main/java/com/google/firebase/concurrent/DelegatingScheduledExecutorService.java create mode 100644 firebase-common/src/main/java/com/google/firebase/concurrent/DelegatingScheduledFuture.java create mode 100644 firebase-common/src/main/java/com/google/firebase/concurrent/ExecutorsRegistrar.java create mode 100644 firebase-common/src/main/java/com/google/firebase/concurrent/UiExecutor.java create mode 100644 firebase-common/src/test/AndroidManifest.xml create mode 100644 firebase-common/src/test/java/com/google/firebase/concurrent/DelegatingScheduledExecutorServiceTest.java create mode 100644 firebase-common/src/test/java/com/google/firebase/concurrent/ExecutorComponent.java create mode 100644 firebase-common/src/test/java/com/google/firebase/concurrent/ExecutorComponentTest.java create mode 100644 firebase-common/src/test/java/com/google/firebase/concurrent/ExecutorTestsRegistrar.java create mode 100644 tools/lint/src/main/kotlin/ThreadPoolDetector.kt diff --git a/firebase-annotations/firebase-annotations.gradle b/firebase-annotations/firebase-annotations.gradle index e77114c5afb..b1762d0a129 100644 --- a/firebase-annotations/firebase-annotations.gradle +++ b/firebase-annotations/firebase-annotations.gradle @@ -29,3 +29,7 @@ java { tasks.withType(JavaCompile) { options.compilerArgs << "-Werror" } + +dependencies { + implementation 'javax.inject:javax.inject:1' +} diff --git a/firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/Background.java b/firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/Background.java new file mode 100644 index 00000000000..5626ea94a78 --- /dev/null +++ b/firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/Background.java @@ -0,0 +1,30 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.annotations.concurrent; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Target; +import javax.inject.Qualifier; + +/** + * An executor/coroutine dispatcher for long running tasks including disk IO, heavy CPU + * computations. + * + *

For operations that can block for long periods of time, like network requests, use the {@link + * Blocking} executor. + */ +@Qualifier +@Target({ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD}) +public @interface Background {} diff --git a/firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/Blocking.java b/firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/Blocking.java new file mode 100644 index 00000000000..d57513b57f1 --- /dev/null +++ b/firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/Blocking.java @@ -0,0 +1,27 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.annotations.concurrent; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Target; +import javax.inject.Qualifier; + +/** + * An executor/coroutine dispatcher for tasks that can block for long periods of time, e.g network + * IO. + */ +@Qualifier +@Target({ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD}) +public @interface Blocking {} diff --git a/firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/Lightweight.java b/firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/Lightweight.java new file mode 100644 index 00000000000..4d3b0828954 --- /dev/null +++ b/firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/Lightweight.java @@ -0,0 +1,26 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.annotations.concurrent; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Target; +import javax.inject.Qualifier; + +/** + * An executor/coroutine dispatcher for lightweight tasks that never block (on IO or other tasks). + */ +@Qualifier +@Target({ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD}) +public @interface Lightweight {} diff --git a/firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/UiThread.java b/firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/UiThread.java new file mode 100644 index 00000000000..bad10c164b8 --- /dev/null +++ b/firebase-annotations/src/main/java/com/google/firebase/annotations/concurrent/UiThread.java @@ -0,0 +1,24 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.annotations.concurrent; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Target; +import javax.inject.Qualifier; + +/** An executor/coroutine dispatcher for work that must run on the UI thread. */ +@Qualifier +@Target({ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD}) +public @interface UiThread {} diff --git a/firebase-common/firebase-common.gradle b/firebase-common/firebase-common.gradle index 47726702701..16c38ff1db6 100644 --- a/firebase-common/firebase-common.gradle +++ b/firebase-common/firebase-common.gradle @@ -66,6 +66,7 @@ dependencies { implementation project(':firebase-components') implementation 'com.google.android.gms:play-services-basement:18.1.0' implementation "com.google.android.gms:play-services-tasks:18.0.1" + implementation 'androidx.concurrent:concurrent-futures:1.1.0' // FirebaseApp references storage, so storage needs to be on classpath when dokka runs. javadocClasspath project(path: ':firebase-storage') diff --git a/firebase-common/ktx/ktx.gradle b/firebase-common/ktx/ktx.gradle index 2d15573ab83..f86c14a5e16 100644 --- a/firebase-common/ktx/ktx.gradle +++ b/firebase-common/ktx/ktx.gradle @@ -38,6 +38,7 @@ android { dependencies { implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlinVersion" + implementation project(':firebase-annotations') implementation project(':firebase-common') implementation project(':firebase-components') implementation 'androidx.annotation:annotation:1.1.0' diff --git a/firebase-common/ktx/src/main/kotlin/com/google/firebase/ktx/Firebase.kt b/firebase-common/ktx/src/main/kotlin/com/google/firebase/ktx/Firebase.kt index f939f218f0f..64ed1fa31d2 100644 --- a/firebase-common/ktx/src/main/kotlin/com/google/firebase/ktx/Firebase.kt +++ b/firebase-common/ktx/src/main/kotlin/com/google/firebase/ktx/Firebase.kt @@ -17,9 +17,18 @@ import android.content.Context import androidx.annotation.Keep import com.google.firebase.FirebaseApp import com.google.firebase.FirebaseOptions +import com.google.firebase.annotations.concurrent.Background +import com.google.firebase.annotations.concurrent.Blocking +import com.google.firebase.annotations.concurrent.Lightweight +import com.google.firebase.annotations.concurrent.UiThread import com.google.firebase.components.Component import com.google.firebase.components.ComponentRegistrar +import com.google.firebase.components.Dependency +import com.google.firebase.components.Qualified import com.google.firebase.platforminfo.LibraryVersionComponent +import java.util.concurrent.Executor +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.asCoroutineDispatcher /** * Single access point to all firebase SDKs from Kotlin. @@ -40,11 +49,11 @@ fun Firebase.initialize(context: Context): FirebaseApp? = FirebaseApp.initialize /** Initializes and returns a FirebaseApp. */ fun Firebase.initialize(context: Context, options: FirebaseOptions): FirebaseApp = - FirebaseApp.initializeApp(context, options) + FirebaseApp.initializeApp(context, options) /** Initializes and returns a FirebaseApp. */ fun Firebase.initialize(context: Context, options: FirebaseOptions, name: String): FirebaseApp = - FirebaseApp.initializeApp(context, options, name) + FirebaseApp.initializeApp(context, options, name) /** Returns options of default FirebaseApp */ val Firebase.options: FirebaseOptions @@ -57,6 +66,27 @@ internal const val LIBRARY_NAME: String = "fire-core-ktx" class FirebaseCommonKtxRegistrar : ComponentRegistrar { override fun getComponents(): List> { return listOf( - LibraryVersionComponent.create(LIBRARY_NAME, BuildConfig.VERSION_NAME)) + LibraryVersionComponent.create(LIBRARY_NAME, BuildConfig.VERSION_NAME), + coroutineDispatcher(), + coroutineDispatcher(), + coroutineDispatcher(), + coroutineDispatcher() + ) } } + +private inline fun coroutineDispatcher(): Component = + Component.builder( + Qualified.qualified(T::class.java, CoroutineDispatcher::class.java) + ).add( + Dependency.required( + Qualified.qualified( + T::class.java, + Executor::class.java + ) + ) + ).factory { c -> + c.get( + Qualified.qualified(T::class.java, Executor::class.java) + ).asCoroutineDispatcher() + }.build() diff --git a/firebase-common/src/main/java/com/google/firebase/FirebaseApp.java b/firebase-common/src/main/java/com/google/firebase/FirebaseApp.java index efc6c01d8ce..8fc7767388e 100644 --- a/firebase-common/src/main/java/com/google/firebase/FirebaseApp.java +++ b/firebase-common/src/main/java/com/google/firebase/FirebaseApp.java @@ -23,8 +23,6 @@ import android.content.Intent; import android.content.IntentFilter; import android.os.Build; -import android.os.Handler; -import android.os.Looper; import android.text.TextUtils; import android.util.Log; import androidx.annotation.GuardedBy; @@ -47,6 +45,7 @@ import com.google.firebase.components.ComponentRegistrar; import com.google.firebase.components.ComponentRuntime; import com.google.firebase.components.Lazy; +import com.google.firebase.concurrent.ExecutorsRegistrar; import com.google.firebase.events.Publisher; import com.google.firebase.heartbeatinfo.DefaultHeartBeatController; import com.google.firebase.inject.Provider; @@ -59,7 +58,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -97,16 +95,10 @@ public class FirebaseApp { private static final Object LOCK = new Object(); - private static final Executor UI_EXECUTOR = new UiExecutor(); - /** A map of (name, FirebaseApp) instances. */ @GuardedBy("LOCK") static final Map INSTANCES = new ArrayMap<>(); - private static final String FIREBASE_ANDROID = "fire-android"; - private static final String FIREBASE_COMMON = "fire-core"; - private static final String KOTLIN = "kotlin"; - private final Context applicationContext; private final String name; private final FirebaseOptions options; @@ -427,9 +419,10 @@ protected FirebaseApp(Context applicationContext, String name, FirebaseOptions o FirebaseTrace.pushTrace("Runtime"); componentRuntime = - ComponentRuntime.builder(UI_EXECUTOR) + ComponentRuntime.builder(com.google.firebase.concurrent.UiExecutor.INSTANCE) .addLazyComponentRegistrars(registrars) .addComponentRegistrar(new FirebaseCommonRegistrar()) + .addComponentRegistrar(new ExecutorsRegistrar()) .addComponent(Component.of(applicationContext, Context.class)) .addComponent(Component.of(this, FirebaseApp.class)) .addComponent(Component.of(options, FirebaseOptions.class)) @@ -712,14 +705,4 @@ public void onBackgroundStateChanged(boolean background) { } } } - - private static class UiExecutor implements Executor { - - private static final Handler HANDLER = new Handler(Looper.getMainLooper()); - - @Override - public void execute(@NonNull Runnable command) { - HANDLER.post(command); - } - } } diff --git a/firebase-common/src/main/java/com/google/firebase/concurrent/CustomThreadFactory.java b/firebase-common/src/main/java/com/google/firebase/concurrent/CustomThreadFactory.java new file mode 100644 index 00000000000..b3079db1d20 --- /dev/null +++ b/firebase-common/src/main/java/com/google/firebase/concurrent/CustomThreadFactory.java @@ -0,0 +1,41 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.concurrent; + +import java.util.Locale; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicLong; + +class CustomThreadFactory implements ThreadFactory { + private static final ThreadFactory DEFAULT = Executors.defaultThreadFactory(); + private final AtomicLong threadCount = new AtomicLong(); + private final String namePrefix; + private final int priority; + + CustomThreadFactory(String namePrefix, int priority) { + this.namePrefix = namePrefix; + this.priority = priority; + } + + @Override + public Thread newThread(Runnable r) { + Thread thread = DEFAULT.newThread(r); + thread.setPriority(priority); + thread.setName( + String.format(Locale.ROOT, "%s Thread #%d", namePrefix, threadCount.getAndIncrement())); + return thread; + } +} diff --git a/firebase-common/src/main/java/com/google/firebase/concurrent/DelegatingScheduledExecutorService.java b/firebase-common/src/main/java/com/google/firebase/concurrent/DelegatingScheduledExecutorService.java new file mode 100644 index 00000000000..23b9385c284 --- /dev/null +++ b/firebase-common/src/main/java/com/google/firebase/concurrent/DelegatingScheduledExecutorService.java @@ -0,0 +1,185 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.concurrent; + +import java.util.Collection; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +class DelegatingScheduledExecutorService implements ScheduledExecutorService { + private final ExecutorService delegate; + private final ScheduledExecutorService scheduler; + + DelegatingScheduledExecutorService(ExecutorService delegate, ScheduledExecutorService scheduler) { + this.delegate = delegate; + this.scheduler = scheduler; + } + + @Override + public void shutdown() { + throw new UnsupportedOperationException("Shutting down is not allowed."); + } + + @Override + public List shutdownNow() { + throw new UnsupportedOperationException("Shutting down is not allowed."); + } + + @Override + public boolean isShutdown() { + return delegate.isShutdown(); + } + + @Override + public boolean isTerminated() { + return delegate.isTerminated(); + } + + @Override + public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException { + return delegate.awaitTermination(timeout, unit); + } + + @Override + public Future submit(Callable task) { + return delegate.submit(task); + } + + @Override + public Future submit(Runnable task, T result) { + return delegate.submit(task, result); + } + + @Override + public Future submit(Runnable task) { + return delegate.submit(task); + } + + @Override + public List> invokeAll(Collection> tasks) + throws InterruptedException { + return delegate.invokeAll(tasks); + } + + @Override + public List> invokeAll( + Collection> tasks, long timeout, TimeUnit unit) + throws InterruptedException { + return delegate.invokeAll(tasks, timeout, unit); + } + + @Override + public T invokeAny(Collection> tasks) + throws ExecutionException, InterruptedException { + return delegate.invokeAny(tasks); + } + + @Override + public T invokeAny(Collection> tasks, long timeout, TimeUnit unit) + throws ExecutionException, InterruptedException, TimeoutException { + return delegate.invokeAny(tasks, timeout, unit); + } + + @Override + public void execute(Runnable command) { + delegate.execute(command); + } + + @Override + public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) { + return new DelegatingScheduledFuture( + completer -> + scheduler.schedule( + () -> + delegate.execute( + () -> { + try { + command.run(); + completer.set(null); + } catch (Exception ex) { + completer.setException(ex); + } + }), + delay, + unit)); + } + + @Override + public ScheduledFuture schedule(Callable callable, long delay, TimeUnit unit) { + return new DelegatingScheduledFuture<>( + completer -> + scheduler.schedule( + () -> + delegate.submit( + () -> { + try { + V result = callable.call(); + completer.set(result); + } catch (Exception ex) { + completer.setException(ex); + } + }), + delay, + unit)); + } + + @Override + public ScheduledFuture scheduleAtFixedRate( + Runnable command, long initialDelay, long period, TimeUnit unit) { + return new DelegatingScheduledFuture<>( + completer -> + scheduler.scheduleAtFixedRate( + () -> + delegate.execute( + () -> { + try { + command.run(); + } catch (Exception ex) { + completer.setException(ex); + throw ex; + } + }), + initialDelay, + period, + unit)); + } + + @Override + public ScheduledFuture scheduleWithFixedDelay( + Runnable command, long initialDelay, long delay, TimeUnit unit) { + return new DelegatingScheduledFuture<>( + completer -> + scheduler.scheduleWithFixedDelay( + () -> + delegate.execute( + () -> { + try { + command.run(); + } catch (Exception ex) { + completer.setException(ex); + } + }), + initialDelay, + delay, + unit)); + } +} diff --git a/firebase-common/src/main/java/com/google/firebase/concurrent/DelegatingScheduledFuture.java b/firebase-common/src/main/java/com/google/firebase/concurrent/DelegatingScheduledFuture.java new file mode 100644 index 00000000000..26ef258136a --- /dev/null +++ b/firebase-common/src/main/java/com/google/firebase/concurrent/DelegatingScheduledFuture.java @@ -0,0 +1,72 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.concurrent; + +import android.annotation.SuppressLint; +import androidx.concurrent.futures.AbstractResolvableFuture; +import java.util.concurrent.Delayed; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +// While direct use of AbstractResolvableFuture is not encouraged, it's stable for use and is not +// going to be removed. In this case it's required since we need to implement a ScheduledFuture so +// we can't use CallbackToFutureAdapter. +@SuppressLint("RestrictedApi") +class DelegatingScheduledFuture extends AbstractResolvableFuture + implements ScheduledFuture { + + interface Completer { + void set(T value); + + void setException(Throwable ex); + } + + interface Resolver { + ScheduledFuture addCompleter(Completer completer); + } + + DelegatingScheduledFuture(Resolver resolver) { + upstreamFuture = + resolver.addCompleter( + new Completer() { + @Override + public void set(V value) { + DelegatingScheduledFuture.this.set(value); + } + + @Override + public void setException(Throwable ex) { + DelegatingScheduledFuture.this.setException(ex); + } + }); + } + + private final ScheduledFuture upstreamFuture; + + @Override + protected void afterDone() { + upstreamFuture.cancel(wasInterrupted()); + } + + @Override + public long getDelay(TimeUnit unit) { + return upstreamFuture.getDelay(unit); + } + + @Override + public int compareTo(Delayed o) { + return upstreamFuture.compareTo(o); + } +} diff --git a/firebase-common/src/main/java/com/google/firebase/concurrent/ExecutorsRegistrar.java b/firebase-common/src/main/java/com/google/firebase/concurrent/ExecutorsRegistrar.java new file mode 100644 index 00000000000..1b28eb8aec9 --- /dev/null +++ b/firebase-common/src/main/java/com/google/firebase/concurrent/ExecutorsRegistrar.java @@ -0,0 +1,102 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.concurrent; + +import android.annotation.SuppressLint; +import android.os.Process; +import com.google.firebase.annotations.concurrent.Background; +import com.google.firebase.annotations.concurrent.Blocking; +import com.google.firebase.annotations.concurrent.Lightweight; +import com.google.firebase.annotations.concurrent.UiThread; +import com.google.firebase.components.Component; +import com.google.firebase.components.ComponentRegistrar; +import com.google.firebase.components.Lazy; +import com.google.firebase.components.Qualified; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadFactory; + +/** @hide */ +@SuppressLint("ThreadPoolCreation") +public class ExecutorsRegistrar implements ComponentRegistrar { + private static final Lazy BG_EXECUTOR = + new Lazy<>( + () -> + scheduled( + Executors.newFixedThreadPool( + 4, factory("Firebase Background", Process.THREAD_PRIORITY_BACKGROUND)))); + + private static final Lazy LITE_EXECUTOR = + new Lazy<>( + () -> + scheduled( + Executors.newFixedThreadPool( + Math.max(2, Runtime.getRuntime().availableProcessors()), + factory("Firebase Lite", Process.THREAD_PRIORITY_DEFAULT)))); + + private static final Lazy BLOCKING_EXECUTOR = + new Lazy<>( + () -> + scheduled( + Executors.newCachedThreadPool( + factory( + "Firebase Blocking", + Process.THREAD_PRIORITY_BACKGROUND + + Process.THREAD_PRIORITY_LESS_FAVORABLE)))); + + private static final Lazy SCHEDULER = + new Lazy<>( + () -> + Executors.newSingleThreadScheduledExecutor( + factory("Firebase Scheduler", Process.THREAD_PRIORITY_DEFAULT))); + + @Override + public List> getComponents() { + return Arrays.asList( + Component.builder( + Qualified.qualified(Background.class, ScheduledExecutorService.class), + Qualified.qualified(Background.class, ExecutorService.class), + Qualified.qualified(Background.class, Executor.class)) + .factory(c -> BG_EXECUTOR.get()) + .build(), + Component.builder( + Qualified.qualified(Blocking.class, ScheduledExecutorService.class), + Qualified.qualified(Blocking.class, ExecutorService.class), + Qualified.qualified(Blocking.class, Executor.class)) + .factory(c -> BLOCKING_EXECUTOR.get()) + .build(), + Component.builder( + Qualified.qualified(Lightweight.class, ScheduledExecutorService.class), + Qualified.qualified(Lightweight.class, ExecutorService.class), + Qualified.qualified(Lightweight.class, Executor.class)) + .factory(c -> LITE_EXECUTOR.get()) + .build(), + Component.builder(Qualified.qualified(UiThread.class, Executor.class)) + .factory(c -> UiExecutor.INSTANCE) + .build()); + } + + private static ScheduledExecutorService scheduled(ExecutorService delegate) { + return new DelegatingScheduledExecutorService(delegate, SCHEDULER.get()); + } + + private static ThreadFactory factory(String threadPrefix, int priority) { + return new CustomThreadFactory(threadPrefix, priority); + } +} diff --git a/firebase-common/src/main/java/com/google/firebase/concurrent/UiExecutor.java b/firebase-common/src/main/java/com/google/firebase/concurrent/UiExecutor.java new file mode 100644 index 00000000000..e06311c8a3a --- /dev/null +++ b/firebase-common/src/main/java/com/google/firebase/concurrent/UiExecutor.java @@ -0,0 +1,31 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.concurrent; + +import android.os.Handler; +import android.os.Looper; +import java.util.concurrent.Executor; + +/** @hide */ +public enum UiExecutor implements Executor { + INSTANCE; + + private static final Handler HANDLER = new Handler(Looper.getMainLooper()); + + @Override + public void execute(Runnable command) { + HANDLER.post(command); + } +} diff --git a/firebase-common/src/main/java/com/google/firebase/heartbeatinfo/DefaultHeartBeatController.java b/firebase-common/src/main/java/com/google/firebase/heartbeatinfo/DefaultHeartBeatController.java index cb7722c37bd..ff9ba5123fc 100644 --- a/firebase-common/src/main/java/com/google/firebase/heartbeatinfo/DefaultHeartBeatController.java +++ b/firebase-common/src/main/java/com/google/firebase/heartbeatinfo/DefaultHeartBeatController.java @@ -23,18 +23,16 @@ import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; import com.google.firebase.FirebaseApp; +import com.google.firebase.annotations.concurrent.Background; import com.google.firebase.components.Component; import com.google.firebase.components.Dependency; +import com.google.firebase.components.Qualified; import com.google.firebase.inject.Provider; import com.google.firebase.platforminfo.UserAgentPublisher; import java.io.ByteArrayOutputStream; import java.util.List; import java.util.Set; import java.util.concurrent.Executor; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; import java.util.zip.GZIPOutputStream; import org.json.JSONArray; import org.json.JSONObject; @@ -52,9 +50,6 @@ public class DefaultHeartBeatController implements HeartBeatController, HeartBea private final Executor backgroundExecutor; - private static final ThreadFactory THREAD_FACTORY = - r -> new Thread(r, "heartbeat-information-executor"); - public Task registerHeartBeat() { if (consumers.size() <= 0) { return Tasks.forResult(null); @@ -118,12 +113,12 @@ private DefaultHeartBeatController( Context context, String persistenceKey, Set consumers, - Provider userAgentProvider) { + Provider userAgentProvider, + Executor backgroundExecutor) { this( () -> new HeartBeatInfoStorage(context, persistenceKey), consumers, - new ThreadPoolExecutor( - 0, 1, 30, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), THREAD_FACTORY), + backgroundExecutor, userAgentProvider, context); } @@ -143,19 +138,22 @@ private DefaultHeartBeatController( } public static @NonNull Component component() { + Qualified backgroundExecutor = Qualified.qualified(Background.class, Executor.class); return Component.builder( DefaultHeartBeatController.class, HeartBeatController.class, HeartBeatInfo.class) .add(Dependency.required(Context.class)) .add(Dependency.required(FirebaseApp.class)) .add(Dependency.setOf(HeartBeatConsumer.class)) .add(Dependency.requiredProvider(UserAgentPublisher.class)) + .add(Dependency.required(backgroundExecutor)) .factory( c -> new DefaultHeartBeatController( c.get(Context.class), c.get(FirebaseApp.class).getPersistenceKey(), c.setOf(HeartBeatConsumer.class), - c.getProvider(UserAgentPublisher.class))) + c.getProvider(UserAgentPublisher.class), + c.get(backgroundExecutor))) .build(); } diff --git a/firebase-common/src/test/AndroidManifest.xml b/firebase-common/src/test/AndroidManifest.xml new file mode 100644 index 00000000000..b0baac40e37 --- /dev/null +++ b/firebase-common/src/test/AndroidManifest.xml @@ -0,0 +1,25 @@ + + + + + + + + + + + + + + + + + + + + + + + diff --git a/firebase-common/src/test/java/com/google/firebase/concurrent/DelegatingScheduledExecutorServiceTest.java b/firebase-common/src/test/java/com/google/firebase/concurrent/DelegatingScheduledExecutorServiceTest.java new file mode 100644 index 00000000000..0717ddf4ee8 --- /dev/null +++ b/firebase-common/src/test/java/com/google/firebase/concurrent/DelegatingScheduledExecutorServiceTest.java @@ -0,0 +1,82 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.concurrent; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +public class DelegatingScheduledExecutorServiceTest { + private final DelegatingScheduledExecutorService service = + new DelegatingScheduledExecutorService( + Executors.newCachedThreadPool(), Executors.newSingleThreadScheduledExecutor()); + + @Test + public void schedule_whenCancelled_shouldCancelUnderlyingFuture() { + CountDownLatch latch = new CountDownLatch(1); + AtomicBoolean ran = new AtomicBoolean(); + ScheduledFuture future = + service.schedule( + () -> { + latch.await(); + ran.set(true); + return null; + }, + 10, + TimeUnit.SECONDS); + future.cancel(true); + latch.countDown(); + assertThat(ran.get()).isFalse(); + } + + @Test + public void scheduleAtFixedRate_whenRunnableThrows_shouldCancelSchedule() + throws InterruptedException { + Semaphore semaphore = new Semaphore(0); + AtomicLong ran = new AtomicLong(); + + ScheduledFuture future = + service.scheduleAtFixedRate( + () -> { + ran.incrementAndGet(); + throw new RuntimeException("fail"); + }, + 1, + 1, + TimeUnit.SECONDS); + + semaphore.release(); + try { + future.get(); + fail("Expected exception not thrown"); + } catch (ExecutionException ex) { + assertThat(ex.getCause()).isInstanceOf(RuntimeException.class); + assertThat(ex.getCause().getMessage()).isEqualTo("fail"); + } + assertThat(ran.get()).isEqualTo(1); + } +} diff --git a/firebase-common/src/test/java/com/google/firebase/concurrent/ExecutorComponent.java b/firebase-common/src/test/java/com/google/firebase/concurrent/ExecutorComponent.java new file mode 100644 index 00000000000..0da8650a80f --- /dev/null +++ b/firebase-common/src/test/java/com/google/firebase/concurrent/ExecutorComponent.java @@ -0,0 +1,58 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.concurrent; + +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ScheduledExecutorService; + +class ExecutorComponent { + final ScheduledExecutorService bgScheduledService; + final ExecutorService bgService; + final Executor bgExecutor; + + final ScheduledExecutorService liteScheduledService; + final ExecutorService liteService; + final Executor liteExecutor; + + final ScheduledExecutorService blockingScheduledService; + final ExecutorService blockingService; + final Executor blockingExecutor; + + final Executor uiExecutor; + + public ExecutorComponent( + ScheduledExecutorService bgScheduledService, + ExecutorService bgService, + Executor bgExecutor, + ScheduledExecutorService liteScheduledService, + ExecutorService liteService, + Executor liteExecutor, + ScheduledExecutorService blockingScheduledService, + ExecutorService blockingService, + Executor blockingExecutor, + Executor uiExecutor) { + this.bgScheduledService = bgScheduledService; + this.bgService = bgService; + this.bgExecutor = bgExecutor; + this.liteScheduledService = liteScheduledService; + this.liteService = liteService; + this.liteExecutor = liteExecutor; + this.blockingScheduledService = blockingScheduledService; + this.blockingService = blockingService; + this.blockingExecutor = blockingExecutor; + this.uiExecutor = uiExecutor; + } +} diff --git a/firebase-common/src/test/java/com/google/firebase/concurrent/ExecutorComponentTest.java b/firebase-common/src/test/java/com/google/firebase/concurrent/ExecutorComponentTest.java new file mode 100644 index 00000000000..bf560c88e95 --- /dev/null +++ b/firebase-common/src/test/java/com/google/firebase/concurrent/ExecutorComponentTest.java @@ -0,0 +1,46 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.concurrent; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.firebase.FirebaseAppTestUtil.withApp; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.firebase.FirebaseOptions; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +public class ExecutorComponentTest { + private static final FirebaseOptions OPTIONS = + new FirebaseOptions.Builder() + .setApiKey("myKey") + .setApplicationId("123") + .setProjectId("456") + .build(); + + @Test + public void testThatAllExecutorsAreRegisteredByCommon() { + withApp( + "test", + OPTIONS, + app -> { + ExecutorComponent executorComponent = app.get(ExecutorComponent.class); + // If the component is not null, it means it was able to get all of its required + // dependencies, otherwise get() would throw. + assertThat(executorComponent).isNotNull(); + }); + } +} diff --git a/firebase-common/src/test/java/com/google/firebase/concurrent/ExecutorTestsRegistrar.java b/firebase-common/src/test/java/com/google/firebase/concurrent/ExecutorTestsRegistrar.java new file mode 100644 index 00000000000..9fdcf7d428f --- /dev/null +++ b/firebase-common/src/test/java/com/google/firebase/concurrent/ExecutorTestsRegistrar.java @@ -0,0 +1,81 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.concurrent; + +import com.google.firebase.annotations.concurrent.Background; +import com.google.firebase.annotations.concurrent.Blocking; +import com.google.firebase.annotations.concurrent.Lightweight; +import com.google.firebase.annotations.concurrent.UiThread; +import com.google.firebase.components.Component; +import com.google.firebase.components.ComponentRegistrar; +import com.google.firebase.components.Dependency; +import com.google.firebase.components.Qualified; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ScheduledExecutorService; + +public class ExecutorTestsRegistrar implements ComponentRegistrar { + @Override + public List> getComponents() { + Qualified bgScheduledService = + Qualified.qualified(Background.class, ScheduledExecutorService.class); + Qualified bgService = + Qualified.qualified(Background.class, ExecutorService.class); + Qualified bgExecutor = Qualified.qualified(Background.class, Executor.class); + + Qualified liteScheduledService = + Qualified.qualified(Lightweight.class, ScheduledExecutorService.class); + Qualified liteService = + Qualified.qualified(Lightweight.class, ExecutorService.class); + Qualified liteExecutor = Qualified.qualified(Lightweight.class, Executor.class); + + Qualified blockingScheduledService = + Qualified.qualified(Blocking.class, ScheduledExecutorService.class); + Qualified blockingService = + Qualified.qualified(Blocking.class, ExecutorService.class); + Qualified blockingExecutor = Qualified.qualified(Blocking.class, Executor.class); + + Qualified uiExecutor = Qualified.qualified(UiThread.class, Executor.class); + + return Collections.singletonList( + Component.builder(ExecutorComponent.class) + .add(Dependency.required(bgScheduledService)) + .add(Dependency.required(bgService)) + .add(Dependency.required(bgExecutor)) + .add(Dependency.required(liteScheduledService)) + .add(Dependency.required(liteService)) + .add(Dependency.required(liteExecutor)) + .add(Dependency.required(blockingScheduledService)) + .add(Dependency.required(blockingService)) + .add(Dependency.required(blockingExecutor)) + .add(Dependency.required(uiExecutor)) + .factory( + c -> + new ExecutorComponent( + c.get(bgScheduledService), + c.get(bgService), + c.get(bgExecutor), + c.get(liteScheduledService), + c.get(liteService), + c.get(liteExecutor), + c.get(blockingScheduledService), + c.get(blockingService), + c.get(blockingExecutor), + c.get(uiExecutor))) + .build()); + } +} diff --git a/tools/lint/src/main/kotlin/CheckRegistry.kt b/tools/lint/src/main/kotlin/CheckRegistry.kt index ebd68916a9f..4b995e55cb4 100644 --- a/tools/lint/src/main/kotlin/CheckRegistry.kt +++ b/tools/lint/src/main/kotlin/CheckRegistry.kt @@ -29,6 +29,8 @@ class CheckRegistry : IssueRegistry() { NonAndroidxNullabilityDetector.NON_ANDROIDX_NULLABILITY, DeferredApiDetector.INVALID_DEFERRED_API_USE, ProviderAssignmentDetector.INVALID_PROVIDER_ASSIGNMENT + // TODO(vkryachko): enable the check after suppressing current violations. + // ThreadPoolDetector.THREAD_POOL_CREATION ) override val api: Int diff --git a/tools/lint/src/main/kotlin/ThreadPoolDetector.kt b/tools/lint/src/main/kotlin/ThreadPoolDetector.kt new file mode 100644 index 00000000000..46148b5b0f7 --- /dev/null +++ b/tools/lint/src/main/kotlin/ThreadPoolDetector.kt @@ -0,0 +1,64 @@ +package com.google.firebase.lint.checks + +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.android.tools.lint.detector.api.SourceCodeScanner +import com.intellij.psi.PsiClass +import com.intellij.psi.PsiMethod +import org.jetbrains.uast.UCallExpression + +class ThreadPoolDetector : Detector(), SourceCodeScanner { + override fun getApplicableMethodNames(): List = listOf( + "newCachedThreadPool", + "newFixedThreadPool", + "newScheduledThreadPool", + "newSingleThreadExecutor", + "newSingleThreadScheduledExecutor", + "newWorkStealingPool" + ) + + override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) { + if (!isExecutorMethod(method)) { + return + } + + context.report( + THREAD_POOL_CREATION, + context.getCallLocation(node, includeReceiver = false, includeArguments = true), + "Creating thread pools is not allowed.") + } + + private fun isExecutorMethod(method: PsiMethod): Boolean { + (method.parent as? PsiClass)?.let { + return it.qualifiedName == "java.util.concurrent.Executors" + } + return false + } + + companion object { + private val IMPLEMENTATION = Implementation( + ThreadPoolDetector::class.java, + Scope.JAVA_FILE_SCOPE + ) + + /** Calling methods on the wrong thread */ + @JvmField + val THREAD_POOL_CREATION = Issue.create( + id = "ThreadPoolCreation", + briefDescription = "Creating thread pools is not allowed.", + + explanation = """ + Please use one of the executors provided by firebase-common + """, + category = Category.CORRECTNESS, + priority = 6, + severity = Severity.ERROR, + implementation = IMPLEMENTATION + ) + } +} From 6b7178ac0d37bafda94c030079fabc19bc4bbadf Mon Sep 17 00:00:00 2001 From: Vladimir Kryachko Date: Fri, 11 Nov 2022 13:08:58 -0500 Subject: [PATCH 3/5] Adds generally useful executors (#4305) Namely, SequentialExecutor and directExecutor. --- .../concurrent/ExecutorsRegistrar.java | 1 - .../concurrent/FirebaseExecutors.java | 49 ++++ .../concurrent/SequentialExecutor.java | 247 ++++++++++++++++++ .../firebase/concurrent/package-info.java | 16 ++ 4 files changed, 312 insertions(+), 1 deletion(-) create mode 100644 firebase-common/src/main/java/com/google/firebase/concurrent/FirebaseExecutors.java create mode 100644 firebase-common/src/main/java/com/google/firebase/concurrent/SequentialExecutor.java create mode 100644 firebase-common/src/main/java/com/google/firebase/concurrent/package-info.java diff --git a/firebase-common/src/main/java/com/google/firebase/concurrent/ExecutorsRegistrar.java b/firebase-common/src/main/java/com/google/firebase/concurrent/ExecutorsRegistrar.java index 1b28eb8aec9..c1a6aada792 100644 --- a/firebase-common/src/main/java/com/google/firebase/concurrent/ExecutorsRegistrar.java +++ b/firebase-common/src/main/java/com/google/firebase/concurrent/ExecutorsRegistrar.java @@ -32,7 +32,6 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ThreadFactory; -/** @hide */ @SuppressLint("ThreadPoolCreation") public class ExecutorsRegistrar implements ComponentRegistrar { private static final Lazy BG_EXECUTOR = diff --git a/firebase-common/src/main/java/com/google/firebase/concurrent/FirebaseExecutors.java b/firebase-common/src/main/java/com/google/firebase/concurrent/FirebaseExecutors.java new file mode 100644 index 00000000000..a31ff759fa6 --- /dev/null +++ b/firebase-common/src/main/java/com/google/firebase/concurrent/FirebaseExecutors.java @@ -0,0 +1,49 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.concurrent; + +import java.util.concurrent.Executor; + +/** Provides commonly useful executors. */ +public class FirebaseExecutors { + private FirebaseExecutors() {} + + /** + * Creates a sequential executor. + * + *

Executes tasks sequentially and provides memory synchronization guarantees for any mutations + * of shared state. + * + *

For details see: + * https://guava.dev/releases/31.1-jre/api/docs/com/google/common/util/concurrent/MoreExecutors.html#newSequentialExecutor(java.util.concurrent.Executor) + */ + public static Executor newSequentialExecutor(Executor delegate) { + return new SequentialExecutor(delegate); + } + + /** Returns a direct executor. */ + public static Executor directExecutor() { + return DirectExecutor.INSTANCE; + } + + private enum DirectExecutor implements Executor { + INSTANCE; + + @Override + public void execute(Runnable command) { + command.run(); + } + } +} diff --git a/firebase-common/src/main/java/com/google/firebase/concurrent/SequentialExecutor.java b/firebase-common/src/main/java/com/google/firebase/concurrent/SequentialExecutor.java new file mode 100644 index 00000000000..cd190a914c6 --- /dev/null +++ b/firebase-common/src/main/java/com/google/firebase/concurrent/SequentialExecutor.java @@ -0,0 +1,247 @@ +package com.google.firebase.concurrent; + +import static com.google.android.gms.common.internal.Preconditions.checkNotNull; +import static com.google.firebase.concurrent.SequentialExecutor.WorkerRunningState.IDLE; +import static com.google.firebase.concurrent.SequentialExecutor.WorkerRunningState.QUEUED; +import static com.google.firebase.concurrent.SequentialExecutor.WorkerRunningState.QUEUING; +import static com.google.firebase.concurrent.SequentialExecutor.WorkerRunningState.RUNNING; +import static java.lang.System.identityHashCode; + +import androidx.annotation.GuardedBy; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.concurrent.Executor; +import java.util.concurrent.RejectedExecutionException; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.annotation.CheckForNull; + +/** + * Executor ensuring that all Runnables submitted are executed in order, using the provided + * Executor, and sequentially such that no two will ever be running at the same time. + * + *

Tasks submitted to {@link #execute(Runnable)} are executed in FIFO order. + * + *

The execution of tasks is done by one thread as long as there are tasks left in the queue. + * When a task is {@linkplain Thread#interrupt interrupted}, execution of subsequent tasks + * continues. See {@link QueueWorker#workOnQueue} for details. + * + *

{@code RuntimeException}s thrown by tasks are simply logged and the executor keeps trucking. + * If an {@code Error} is thrown, the error will propagate and execution will stop until it is + * restarted by a call to {@link #execute}. + */ +final class SequentialExecutor implements Executor { + private static final Logger log = Logger.getLogger(SequentialExecutor.class.getName()); + + enum WorkerRunningState { + /** Runnable is not running and not queued for execution */ + IDLE, + /** Runnable is not running, but is being queued for execution */ + QUEUING, + /** runnable has been submitted but has not yet begun execution */ + QUEUED, + RUNNING, + } + + /** Underlying executor that all submitted Runnable objects are run on. */ + private final Executor executor; + + @GuardedBy("queue") + private final Deque queue = new ArrayDeque<>(); + + /** see {@link WorkerRunningState} */ + @GuardedBy("queue") + private WorkerRunningState workerRunningState = IDLE; + + /** + * This counter prevents an ABA issue where a thread may successfully schedule the worker, the + * worker runs and exhausts the queue, another thread enqueues a task and fails to schedule the + * worker, and then the first thread's call to delegate.execute() returns. Without this counter, + * it would observe the QUEUING state and set it to QUEUED, and the worker would never be + * scheduled again for future submissions. + */ + @GuardedBy("queue") + private long workerRunCount = 0; + + private final QueueWorker worker = new QueueWorker(); + + SequentialExecutor(Executor executor) { + this.executor = checkNotNull(executor); + } + + /** + * Adds a task to the queue and makes sure a worker thread is running. + * + *

If this method throws, e.g. a {@code RejectedExecutionException} from the delegate executor, + * execution of tasks will stop until a call to this method is made. + */ + @Override + public void execute(Runnable task) { + checkNotNull(task); + Runnable submittedTask; + long oldRunCount; + synchronized (queue) { + // If the worker is already running (or execute() on the delegate returned successfully, and + // the worker has yet to start) then we don't need to start the worker. + if (workerRunningState == RUNNING || workerRunningState == QUEUED) { + queue.add(task); + return; + } + + oldRunCount = workerRunCount; + + // If the worker is not yet running, the delegate Executor might reject our attempt to start + // it. To preserve FIFO order and failure atomicity of rejected execution when the same + // Runnable is executed more than once, allocate a wrapper that we know is safe to remove by + // object identity. + // A data structure that returned a removal handle from add() would allow eliminating this + // allocation. + submittedTask = + new Runnable() { + @Override + public void run() { + task.run(); + } + + @Override + public String toString() { + return task.toString(); + } + }; + queue.add(submittedTask); + workerRunningState = QUEUING; + } + + try { + executor.execute(worker); + } catch (RuntimeException | Error t) { + synchronized (queue) { + boolean removed = + (workerRunningState == IDLE || workerRunningState == QUEUING) + && queue.removeLastOccurrence(submittedTask); + // If the delegate is directExecutor(), the submitted runnable could have thrown a REE. But + // that's handled by the log check that catches RuntimeExceptions in the queue worker. + if (!(t instanceof RejectedExecutionException) || removed) { + throw t; + } + } + return; + } + + /* + * This is an unsynchronized read! After the read, the function returns immediately or acquires + * the lock to check again. Since an IDLE state was observed inside the preceding synchronized + * block, and reference field assignment is atomic, this may save reacquiring the lock when + * another thread or the worker task has cleared the count and set the state. + * + *

When {@link #executor} is a directExecutor(), the value written to + * {@code workerRunningState} will be available synchronously, and behaviour will be + * deterministic. + */ + @SuppressWarnings("GuardedBy") + boolean alreadyMarkedQueued = workerRunningState != QUEUING; + if (alreadyMarkedQueued) { + return; + } + synchronized (queue) { + if (workerRunCount == oldRunCount && workerRunningState == QUEUING) { + workerRunningState = QUEUED; + } + } + } + + /** Worker that runs tasks from {@link #queue} until it is empty. */ + private final class QueueWorker implements Runnable { + @CheckForNull Runnable task; + + @Override + public void run() { + try { + workOnQueue(); + } catch (Error e) { + synchronized (queue) { + workerRunningState = IDLE; + } + throw e; + // The execution of a task has ended abnormally. + // We could have tasks left in the queue, so should perhaps try to restart a worker, + // but then the Error will get delayed if we are using a direct (same thread) executor. + } + } + + /** + * Continues executing tasks from {@link #queue} until it is empty. + * + *

The thread's interrupt bit is cleared before execution of each task. + * + *

If the Thread in use is interrupted before or during execution of the tasks in {@link + * #queue}, the Executor will complete its tasks, and then restore the interruption. This means + * that once the Thread returns to the Executor that this Executor composes, the interruption + * will still be present. If the composed Executor is an ExecutorService, it can respond to + * shutdown() by returning tasks queued on that Thread after {@link #worker} drains the queue. + */ + private void workOnQueue() { + boolean interruptedDuringTask = false; + boolean hasSetRunning = false; + try { + while (true) { + synchronized (queue) { + // Choose whether this thread will run or not after acquiring the lock on the first + // iteration + if (!hasSetRunning) { + if (workerRunningState == RUNNING) { + // Don't want to have two workers pulling from the queue. + return; + } else { + // Increment the run counter to avoid the ABA problem of a submitter marking the + // thread as QUEUED after it already ran and exhausted the queue before returning + // from execute(). + workerRunCount++; + workerRunningState = RUNNING; + hasSetRunning = true; + } + } + task = queue.poll(); + if (task == null) { + workerRunningState = IDLE; + return; + } + } + // Remove the interrupt bit before each task. The interrupt is for the "current task" when + // it is sent, so subsequent tasks in the queue should not be caused to be interrupted + // by a previous one in the queue being interrupted. + interruptedDuringTask |= Thread.interrupted(); + try { + task.run(); + } catch (RuntimeException e) { + log.log(Level.SEVERE, "Exception while executing runnable " + task, e); + } finally { + task = null; + } + } + } finally { + // Ensure that if the thread was interrupted at all while processing the task queue, it + // is returned to the delegate Executor interrupted so that it may handle the + // interruption if it likes. + if (interruptedDuringTask) { + Thread.currentThread().interrupt(); + } + } + } + + @SuppressWarnings("GuardedBy") + @Override + public String toString() { + Runnable currentlyRunning = task; + if (currentlyRunning != null) { + return "SequentialExecutorWorker{running=" + currentlyRunning + "}"; + } + return "SequentialExecutorWorker{state=" + workerRunningState + "}"; + } + } + + @Override + public String toString() { + return "SequentialExecutor@" + identityHashCode(this) + "{" + executor + "}"; + } +} diff --git a/firebase-common/src/main/java/com/google/firebase/concurrent/package-info.java b/firebase-common/src/main/java/com/google/firebase/concurrent/package-info.java new file mode 100644 index 00000000000..9452eb485fd --- /dev/null +++ b/firebase-common/src/main/java/com/google/firebase/concurrent/package-info.java @@ -0,0 +1,16 @@ +// Copyright 2018 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** @hide */ +package com.google.firebase.concurrent; From 4173c8cf1da781273c46dbe74f440737e95803ce Mon Sep 17 00:00:00 2001 From: Vladimir Kryachko Date: Fri, 11 Nov 2022 13:09:21 -0500 Subject: [PATCH 4/5] Enable strict mode for executors. (#4303) Any violations would kill the app in debug builds of firebase-common, and log a warning in release builds. This is done to fail tests that incorrectly use executors while not affecting 3p apps in release builds. Additionally correctly set thread priorities in an Android specific way. --- .../concurrent/CustomThreadFactory.java | 18 +++++++-- .../concurrent/ExecutorsRegistrar.java | 38 +++++++++++++++++-- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/firebase-common/src/main/java/com/google/firebase/concurrent/CustomThreadFactory.java b/firebase-common/src/main/java/com/google/firebase/concurrent/CustomThreadFactory.java index b3079db1d20..417435c2abf 100644 --- a/firebase-common/src/main/java/com/google/firebase/concurrent/CustomThreadFactory.java +++ b/firebase-common/src/main/java/com/google/firebase/concurrent/CustomThreadFactory.java @@ -14,26 +14,38 @@ package com.google.firebase.concurrent; +import android.os.Process; +import android.os.StrictMode; import java.util.Locale; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicLong; +import javax.annotation.Nullable; class CustomThreadFactory implements ThreadFactory { private static final ThreadFactory DEFAULT = Executors.defaultThreadFactory(); private final AtomicLong threadCount = new AtomicLong(); private final String namePrefix; private final int priority; + private final StrictMode.ThreadPolicy policy; - CustomThreadFactory(String namePrefix, int priority) { + CustomThreadFactory(String namePrefix, int priority, @Nullable StrictMode.ThreadPolicy policy) { this.namePrefix = namePrefix; this.priority = priority; + this.policy = policy; } @Override public Thread newThread(Runnable r) { - Thread thread = DEFAULT.newThread(r); - thread.setPriority(priority); + Thread thread = + DEFAULT.newThread( + () -> { + Process.setThreadPriority(priority); + if (policy != null) { + StrictMode.setThreadPolicy(policy); + } + r.run(); + }); thread.setName( String.format(Locale.ROOT, "%s Thread #%d", namePrefix, threadCount.getAndIncrement())); return thread; diff --git a/firebase-common/src/main/java/com/google/firebase/concurrent/ExecutorsRegistrar.java b/firebase-common/src/main/java/com/google/firebase/concurrent/ExecutorsRegistrar.java index c1a6aada792..39063132e84 100644 --- a/firebase-common/src/main/java/com/google/firebase/concurrent/ExecutorsRegistrar.java +++ b/firebase-common/src/main/java/com/google/firebase/concurrent/ExecutorsRegistrar.java @@ -15,7 +15,10 @@ package com.google.firebase.concurrent; import android.annotation.SuppressLint; +import android.os.Build; import android.os.Process; +import android.os.StrictMode; +import com.google.firebase.BuildConfig; import com.google.firebase.annotations.concurrent.Background; import com.google.firebase.annotations.concurrent.Blocking; import com.google.firebase.annotations.concurrent.Lightweight; @@ -39,7 +42,9 @@ public class ExecutorsRegistrar implements ComponentRegistrar { () -> scheduled( Executors.newFixedThreadPool( - 4, factory("Firebase Background", Process.THREAD_PRIORITY_BACKGROUND)))); + 4, + factory( + "Firebase Background", Process.THREAD_PRIORITY_BACKGROUND, bgPolicy())))); private static final Lazy LITE_EXECUTOR = new Lazy<>( @@ -47,7 +52,7 @@ public class ExecutorsRegistrar implements ComponentRegistrar { scheduled( Executors.newFixedThreadPool( Math.max(2, Runtime.getRuntime().availableProcessors()), - factory("Firebase Lite", Process.THREAD_PRIORITY_DEFAULT)))); + factory("Firebase Lite", Process.THREAD_PRIORITY_DEFAULT, litePolicy())))); private static final Lazy BLOCKING_EXECUTOR = new Lazy<>( @@ -96,6 +101,33 @@ private static ScheduledExecutorService scheduled(ExecutorService delegate) { } private static ThreadFactory factory(String threadPrefix, int priority) { - return new CustomThreadFactory(threadPrefix, priority); + return new CustomThreadFactory(threadPrefix, priority, null); + } + + private static ThreadFactory factory( + String threadPrefix, int priority, StrictMode.ThreadPolicy policy) { + return new CustomThreadFactory(threadPrefix, priority, policy); + } + + private static StrictMode.ThreadPolicy bgPolicy() { + StrictMode.ThreadPolicy.Builder builder = new StrictMode.ThreadPolicy.Builder().detectNetwork(); + if (Build.VERSION.SDK_INT >= 23) { + builder.detectResourceMismatches(); + if (Build.VERSION.SDK_INT >= 26) { + builder.detectUnbufferedIo(); + } + } + if (BuildConfig.DEBUG) { + builder.penaltyDeath(); + } + return builder.penaltyLog().build(); + } + + private static StrictMode.ThreadPolicy litePolicy() { + StrictMode.ThreadPolicy.Builder builder = new StrictMode.ThreadPolicy.Builder().detectAll(); + if (BuildConfig.DEBUG) { + builder.penaltyDeath(); + } + return builder.penaltyLog().build(); } } From 57d3b15e868355d38e8f52caa50dede8107b9dd3 Mon Sep 17 00:00:00 2001 From: Vladimir Kryachko Date: Mon, 14 Nov 2022 12:23:38 -0500 Subject: [PATCH 5/5] Enable thread pool linter check. (#4297) * Enable thread pool linter check. All violations are now suppressed, bugs filed to fix each product. * ktlint * Remove init * Fix copyright --- .../debug/internal/DebugAppCheckProvider.java | 3 ++ .../PlayIntegrityAppCheckProvider.java | 3 ++ .../internal/SafetyNetAppCheckProvider.java | 3 ++ .../internal/DefaultFirebaseAppCheck.java | 3 ++ .../internal/DefaultTokenRefresher.java | 3 ++ .../appdistribution/impl/AabUpdater.java | 3 ++ .../appdistribution/impl/ApkUpdater.java | 3 ++ ...irebaseAppDistributionTesterApiClient.java | 4 ++ .../concurrent/SequentialExecutor.java | 14 +++++++ .../remoteconfig/RemoteConfigComponent.java | 5 +++ .../common/CrashlyticsController.java | 4 ++ .../internal/common/ExecutorUtils.java | 8 ++++ .../internal/send/ReportQueue.java | 5 ++- .../database/core/ThreadPoolEventTarget.java | 3 ++ .../firebase/firestore/core/Transaction.java | 4 ++ .../firebase/firestore/util/AsyncQueue.java | 3 ++ .../internal/AbtIntegrationHelper.java | 6 ++- .../internal/DeveloperListenerManager.java | 6 +++ .../installations/FirebaseInstallations.java | 7 ++++ .../firebase-messaging-directboot.gradle | 4 -- .../directboot/threads/PoolableExecutors.java | 15 +++++++ firebase-messaging/firebase-messaging.gradle | 5 --- .../firebase/messaging/FcmExecutors.java | 13 ++++++ .../firebase/messaging/FirebaseMessaging.java | 3 ++ .../google/firebase/messaging/SyncTask.java | 2 + .../messaging/WithinAppServiceConnection.java | 3 ++ .../messaging/threads/PoolableExecutors.java | 15 +++++++ .../FirebaseModelDownloader.java | 3 ++ .../internal/CustomModelDownloadService.java | 3 ++ .../perf/config/DeviceCacheManager.java | 3 ++ .../perf/config/RemoteConfigManager.java | 3 ++ .../firebase/perf/metrics/AppStartTrace.java | 3 ++ .../firebase/perf/session/SessionManager.java | 2 + .../session/gauges/CpuGaugeCollector.java | 3 ++ .../perf/session/gauges/GaugeManager.java | 5 ++- .../session/gauges/MemoryGaugeCollector.java | 3 ++ .../perf/transport/TransportManager.java | 3 ++ .../firebase-segmentation.gradle | 1 + .../segmentation/FirebaseSegmentation.java | 14 ++++--- .../FirebaseSegmentationRegistrar.java | 9 +++- .../FirebaseSegmentationTest.java | 42 +++++++++++++++---- .../storage/StorageTaskScheduler.java | 15 +++++++ tools/lint/src/main/kotlin/CheckRegistry.kt | 5 +-- .../src/main/kotlin/ThreadPoolDetector.kt | 22 +++++++++- .../runtime/ExecutionModule.java | 2 + 45 files changed, 262 insertions(+), 29 deletions(-) diff --git a/appcheck/firebase-appcheck-debug/src/main/java/com/google/firebase/appcheck/debug/internal/DebugAppCheckProvider.java b/appcheck/firebase-appcheck-debug/src/main/java/com/google/firebase/appcheck/debug/internal/DebugAppCheckProvider.java index 4f23d52951a..01bee5b36b4 100644 --- a/appcheck/firebase-appcheck-debug/src/main/java/com/google/firebase/appcheck/debug/internal/DebugAppCheckProvider.java +++ b/appcheck/firebase-appcheck-debug/src/main/java/com/google/firebase/appcheck/debug/internal/DebugAppCheckProvider.java @@ -16,6 +16,7 @@ import static com.google.android.gms.common.internal.Preconditions.checkNotNull; +import android.annotation.SuppressLint; import android.util.Log; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -45,6 +46,8 @@ public class DebugAppCheckProvider implements AppCheckProvider { private final RetryManager retryManager; private final Task debugSecretTask; + // TODO(b/258273630): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public DebugAppCheckProvider(@NonNull FirebaseApp firebaseApp, @Nullable String debugSecret) { checkNotNull(firebaseApp); this.networkClient = new NetworkClient(firebaseApp); diff --git a/appcheck/firebase-appcheck-playintegrity/src/main/java/com/google/firebase/appcheck/playintegrity/internal/PlayIntegrityAppCheckProvider.java b/appcheck/firebase-appcheck-playintegrity/src/main/java/com/google/firebase/appcheck/playintegrity/internal/PlayIntegrityAppCheckProvider.java index 13381af83a6..8a5e2622a9b 100644 --- a/appcheck/firebase-appcheck-playintegrity/src/main/java/com/google/firebase/appcheck/playintegrity/internal/PlayIntegrityAppCheckProvider.java +++ b/appcheck/firebase-appcheck-playintegrity/src/main/java/com/google/firebase/appcheck/playintegrity/internal/PlayIntegrityAppCheckProvider.java @@ -14,6 +14,7 @@ package com.google.firebase.appcheck.playintegrity.internal; +import android.annotation.SuppressLint; import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; import com.google.android.gms.tasks.Task; @@ -41,6 +42,8 @@ public class PlayIntegrityAppCheckProvider implements AppCheckProvider { private final ExecutorService backgroundExecutor; private final RetryManager retryManager; + // TODO(b/258273630): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public PlayIntegrityAppCheckProvider(@NonNull FirebaseApp firebaseApp) { this( firebaseApp.getOptions().getGcmSenderId(), diff --git a/appcheck/firebase-appcheck-safetynet/src/main/java/com/google/firebase/appcheck/safetynet/internal/SafetyNetAppCheckProvider.java b/appcheck/firebase-appcheck-safetynet/src/main/java/com/google/firebase/appcheck/safetynet/internal/SafetyNetAppCheckProvider.java index f44867c39ee..e88ee9c0e9b 100644 --- a/appcheck/firebase-appcheck-safetynet/src/main/java/com/google/firebase/appcheck/safetynet/internal/SafetyNetAppCheckProvider.java +++ b/appcheck/firebase-appcheck-safetynet/src/main/java/com/google/firebase/appcheck/safetynet/internal/SafetyNetAppCheckProvider.java @@ -17,6 +17,7 @@ import static com.google.android.gms.common.internal.Preconditions.checkNotEmpty; import static com.google.android.gms.common.internal.Preconditions.checkNotNull; +import android.annotation.SuppressLint; import android.content.Context; import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; @@ -56,6 +57,8 @@ public class SafetyNetAppCheckProvider implements AppCheckProvider { private final String apiKey; /** @param firebaseApp the FirebaseApp to which this Factory is tied. */ + // TODO(b/258273630): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public SafetyNetAppCheckProvider(@NonNull FirebaseApp firebaseApp) { this( firebaseApp, diff --git a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java index 769bd1048d3..3ed17999814 100644 --- a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java +++ b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java @@ -16,6 +16,7 @@ import static com.google.android.gms.common.internal.Preconditions.checkNotNull; +import android.annotation.SuppressLint; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; @@ -57,6 +58,8 @@ public class DefaultFirebaseAppCheck extends FirebaseAppCheck { private AppCheckProvider appCheckProvider; private AppCheckToken cachedToken; + // TODO(b/258273630): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public DefaultFirebaseAppCheck( @NonNull FirebaseApp firebaseApp, @NonNull Provider heartBeatController) { diff --git a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java index 8e7f6722046..8cec32c540a 100644 --- a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java +++ b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java @@ -18,6 +18,7 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; +import android.annotation.SuppressLint; import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; import com.google.android.gms.tasks.OnFailureListener; @@ -41,6 +42,8 @@ public class DefaultTokenRefresher { private volatile ScheduledFuture refreshFuture; private volatile long delayAfterFailureSeconds; + // TODO(b/258273630): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") DefaultTokenRefresher(@NonNull DefaultFirebaseAppCheck firebaseAppCheck) { this(checkNotNull(firebaseAppCheck), Executors.newScheduledThreadPool(/* corePoolSize= */ 1)); } diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AabUpdater.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AabUpdater.java index cc4122d7d0d..4d1ba164ca3 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AabUpdater.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AabUpdater.java @@ -19,6 +19,7 @@ import static com.google.firebase.appdistribution.impl.TaskUtils.runAsyncInTask; import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskException; +import android.annotation.SuppressLint; import android.app.Activity; import android.content.Intent; import android.net.Uri; @@ -52,6 +53,8 @@ class AabUpdater { @GuardedBy("updateAabLock") private boolean hasBeenSentToPlayForCurrentTask = false; + // TODO(b/258264924): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") AabUpdater() { this( FirebaseAppDistributionLifecycleNotifier.getInstance(), diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/ApkUpdater.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/ApkUpdater.java index 3945b4234e5..a54abd1a268 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/ApkUpdater.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/ApkUpdater.java @@ -19,6 +19,7 @@ import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskException; import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskResult; +import android.annotation.SuppressLint; import android.content.Context; import android.os.Build.VERSION; import android.os.Build.VERSION_CODES; @@ -62,6 +63,8 @@ class ApkUpdater { private final Object updateTaskLock = new Object(); + // TODO(b/258264924): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public ApkUpdater(@NonNull FirebaseApp firebaseApp, @NonNull ApkInstaller apkInstaller) { this( Executors.newSingleThreadExecutor(), diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionTesterApiClient.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionTesterApiClient.java index f5dfd8e4173..8d81833a1e4 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionTesterApiClient.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionTesterApiClient.java @@ -16,6 +16,7 @@ import static com.google.firebase.appdistribution.impl.TaskUtils.runAsyncInTask; +import android.annotation.SuppressLint; import androidx.annotation.NonNull; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; @@ -63,6 +64,9 @@ private interface FidDependentJob { private final FirebaseApp firebaseApp; private final Provider firebaseInstallationsApiProvider; private final TesterApiHttpClient testerApiHttpClient; + + // TODO(b/258264924): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private final Executor taskExecutor = Executors.newSingleThreadExecutor(); FirebaseAppDistributionTesterApiClient( diff --git a/firebase-common/src/main/java/com/google/firebase/concurrent/SequentialExecutor.java b/firebase-common/src/main/java/com/google/firebase/concurrent/SequentialExecutor.java index cd190a914c6..db5f4f3a374 100644 --- a/firebase-common/src/main/java/com/google/firebase/concurrent/SequentialExecutor.java +++ b/firebase-common/src/main/java/com/google/firebase/concurrent/SequentialExecutor.java @@ -1,3 +1,17 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package com.google.firebase.concurrent; import static com.google.android.gms.common.internal.Preconditions.checkNotNull; diff --git a/firebase-config/src/main/java/com/google/firebase/remoteconfig/RemoteConfigComponent.java b/firebase-config/src/main/java/com/google/firebase/remoteconfig/RemoteConfigComponent.java index 83b867a65f2..543b95cfec0 100644 --- a/firebase-config/src/main/java/com/google/firebase/remoteconfig/RemoteConfigComponent.java +++ b/firebase-config/src/main/java/com/google/firebase/remoteconfig/RemoteConfigComponent.java @@ -14,6 +14,7 @@ package com.google.firebase.remoteconfig; +import android.annotation.SuppressLint; import android.content.Context; import android.content.SharedPreferences; import androidx.annotation.GuardedBy; @@ -87,6 +88,8 @@ public class RemoteConfigComponent { private Map customHeaders = new HashMap<>(); /** Firebase Remote Config Component constructor. */ + // TODO(b/258275481): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") RemoteConfigComponent( Context context, FirebaseApp firebaseApp, @@ -211,6 +214,8 @@ public synchronized void setCustomHeaders(Map customHeaders) { this.customHeaders = customHeaders; } + // TODO(b/258275481): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private ConfigCacheClient getCacheClient(String namespace, String configStoreType) { String fileName = String.format( diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java index 4c4ecad1795..fd62712ef22 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.firebase.crashlytics.internal.common; +import android.annotation.SuppressLint; import android.app.ActivityManager; import android.app.ApplicationExitInfo; import android.content.Context; @@ -734,6 +735,9 @@ private Task logAnalyticsAppExceptionEvent(long timestamp) { return Tasks.forResult(null); } Logger.getLogger().d("Logging app exception event to Firebase Analytics"); + + // TODO(b/258263226): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") final ThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(1); return Tasks.call( executor, diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/ExecutorUtils.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/ExecutorUtils.java index 9147921af73..77fafe9adc5 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/ExecutorUtils.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/ExecutorUtils.java @@ -16,6 +16,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; +import android.annotation.SuppressLint; import com.google.firebase.crashlytics.internal.Logger; import java.util.Locale; import java.util.concurrent.ExecutorService; @@ -43,6 +44,8 @@ public static ExecutorService buildSingleThreadExecutorService(String name) { public static ScheduledExecutorService buildSingleThreadScheduledExecutorService(String name) { final ThreadFactory threadFactory = ExecutorUtils.getNamedThreadFactory(name); + // TODO(b/258263226): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") final ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor(threadFactory); ExecutorUtils.addDelayedShutdownHook(name, executor); @@ -70,6 +73,8 @@ public void onRun() { }; } + // TODO(b/258263226): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private static ExecutorService newSingleThreadExecutor( ThreadFactory threadFactory, RejectedExecutionHandler rejectedExecutionHandler) { return Executors.unconfigurableExecutorService( @@ -88,11 +93,14 @@ private static void addDelayedShutdownHook(String serviceName, ExecutorService s serviceName, service, DEFAULT_TERMINATION_TIMEOUT, SECONDS); } + // TODO(b/258263226): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private static void addDelayedShutdownHook( final String serviceName, final ExecutorService service, final long terminationTimeout, final TimeUnit timeUnit) { + Runtime.getRuntime() .addShutdownHook( new Thread( diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/send/ReportQueue.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/send/ReportQueue.java index d25c86c0eff..39c79d07c3c 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/send/ReportQueue.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/send/ReportQueue.java @@ -62,6 +62,8 @@ final class ReportQueue { onDemandCounter); } + // TODO(b/258263226): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") ReportQueue( double ratePerMinute, double base, @@ -120,7 +122,8 @@ TaskCompletionSource enqueueReport( } } - @SuppressLint("DiscouragedApi") // best effort only + // TODO(b/258263226): Migrate to go/firebase-android-executors + @SuppressLint({"DiscouragedApi", "ThreadPoolCreation"}) // best effort only public void flushScheduledReportsIfAble() { CountDownLatch latch = new CountDownLatch(1); new Thread( diff --git a/firebase-database/src/main/java/com/google/firebase/database/core/ThreadPoolEventTarget.java b/firebase-database/src/main/java/com/google/firebase/database/core/ThreadPoolEventTarget.java index 90bd199e496..f7c8b4e5df7 100644 --- a/firebase-database/src/main/java/com/google/firebase/database/core/ThreadPoolEventTarget.java +++ b/firebase-database/src/main/java/com/google/firebase/database/core/ThreadPoolEventTarget.java @@ -14,6 +14,7 @@ package com.google.firebase.database.core; +import android.annotation.SuppressLint; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadFactory; @@ -25,6 +26,8 @@ class ThreadPoolEventTarget implements EventTarget { private final ThreadPoolExecutor executor; + // TODO(b/258277572): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public ThreadPoolEventTarget( final ThreadFactory wrappedFactory, final ThreadInitializer threadInitializer) { int poolSize = 1; diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/Transaction.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/Transaction.java index d235fc8e7b0..42c62125dd1 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/Transaction.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/Transaction.java @@ -17,6 +17,7 @@ import static com.google.firebase.firestore.util.Assert.fail; import static com.google.firebase.firestore.util.Assert.hardAssert; +import android.annotation.SuppressLint; import androidx.annotation.Nullable; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; @@ -162,6 +163,9 @@ private static Executor createDefaultExecutor() { int maxPoolSize = corePoolSize; int keepAliveSeconds = 1; LinkedBlockingQueue queue = new LinkedBlockingQueue<>(); + + // TODO(b/258277574): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") ThreadPoolExecutor executor = new ThreadPoolExecutor( corePoolSize, maxPoolSize, keepAliveSeconds, TimeUnit.SECONDS, queue); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/AsyncQueue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/AsyncQueue.java index c368022f07a..532ea0dc342 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/AsyncQueue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/AsyncQueue.java @@ -17,6 +17,7 @@ import static com.google.firebase.firestore.util.Assert.fail; import static com.google.firebase.firestore.util.Assert.hardAssert; +import android.annotation.SuppressLint; import android.os.Handler; import android.os.Looper; import androidx.annotation.NonNull; @@ -243,6 +244,8 @@ public Thread newThread(@NonNull Runnable runnable) { } } + // TODO(b/258277574): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") SynchronizedShutdownAwareExecutor() { DelayedStartFactory threadFactory = new DelayedStartFactory(); diff --git a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/AbtIntegrationHelper.java b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/AbtIntegrationHelper.java index 8ea864b51f3..1927a4cd1de 100644 --- a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/AbtIntegrationHelper.java +++ b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/AbtIntegrationHelper.java @@ -14,6 +14,7 @@ package com.google.firebase.inappmessaging.internal; +import android.annotation.SuppressLint; import androidx.annotation.VisibleForTesting; import com.google.firebase.abt.AbtException; import com.google.firebase.abt.AbtExperimentInfo; @@ -33,7 +34,10 @@ public class AbtIntegrationHelper { private final FirebaseABTesting abTesting; - @VisibleForTesting Executor executor = Executors.newSingleThreadExecutor(); + // TODO(b/258280977): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") + @VisibleForTesting + Executor executor = Executors.newSingleThreadExecutor(); @Inject public AbtIntegrationHelper(FirebaseABTesting abTesting) { diff --git a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/DeveloperListenerManager.java b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/DeveloperListenerManager.java index ff1d3383849..bc6dc381445 100644 --- a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/DeveloperListenerManager.java +++ b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/DeveloperListenerManager.java @@ -14,6 +14,7 @@ package com.google.firebase.inappmessaging.internal; +import android.annotation.SuppressLint; import androidx.annotation.NonNull; import com.google.firebase.inappmessaging.FirebaseInAppMessagingClickListener; import com.google.firebase.inappmessaging.FirebaseInAppMessagingDismissListener; @@ -56,6 +57,9 @@ public class DeveloperListenerManager { registeredImpressionListeners = new HashMap<>(); private static BlockingQueue mCallbackQueue = new LinkedBlockingQueue<>(); + + // TODO(b/258280977): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private static final ThreadPoolExecutor CALLBACK_QUEUE_EXECUTOR = new ThreadPoolExecutor( POOL_SIZE, @@ -182,6 +186,8 @@ static class FIAMThreadFactory implements ThreadFactory { @SuppressWarnings("ThreadPriorityCheck") @Override + // TODO(b/258280977): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public Thread newThread(@NonNull Runnable r) { Thread t = new Thread(r, "FIAM-" + mNameSuffix + threadNumber.getAndIncrement()); t.setDaemon(false); diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/FirebaseInstallations.java b/firebase-installations/src/main/java/com/google/firebase/installations/FirebaseInstallations.java index 5f1b1e7f90d..62385d5caca 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/FirebaseInstallations.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/FirebaseInstallations.java @@ -14,6 +14,7 @@ package com.google.firebase.installations; +import android.annotation.SuppressLint; import android.text.TextUtils; import androidx.annotation.GuardedBy; import androidx.annotation.NonNull; @@ -96,6 +97,8 @@ public class FirebaseInstallations implements FirebaseInstallationsApi { private final AtomicInteger mCount = new AtomicInteger(1); @Override + // TODO(b/258422917): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public Thread newThread(Runnable r) { return new Thread( r, String.format("firebase-installations-executor-%d", mCount.getAndIncrement())); @@ -123,6 +126,8 @@ public Thread newThread(Runnable r) { + "Please retry your last request."; /** package private constructor. */ + // TODO(b/258422917): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") FirebaseInstallations( FirebaseApp firebaseApp, @NonNull Provider heartBeatProvider) { this( @@ -142,6 +147,8 @@ public Thread newThread(Runnable r) { new RandomFidGenerator()); } + // TODO(b/258422917): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") FirebaseInstallations( ExecutorService backgroundExecutor, FirebaseApp firebaseApp, diff --git a/firebase-messaging-directboot/firebase-messaging-directboot.gradle b/firebase-messaging-directboot/firebase-messaging-directboot.gradle index adca39b77ad..aae84d0dc49 100644 --- a/firebase-messaging-directboot/firebase-messaging-directboot.gradle +++ b/firebase-messaging-directboot/firebase-messaging-directboot.gradle @@ -27,10 +27,6 @@ android { timeOutInMs 60 * 1000 } - lintOptions { - abortOnError false - } - compileSdkVersion project.targetSdkVersion defaultConfig { minSdkVersion 19 diff --git a/firebase-messaging-directboot/src/main/java/com/google/firebase/messaging/directboot/threads/PoolableExecutors.java b/firebase-messaging-directboot/src/main/java/com/google/firebase/messaging/directboot/threads/PoolableExecutors.java index 529fe04dcd5..431b127976a 100644 --- a/firebase-messaging-directboot/src/main/java/com/google/firebase/messaging/directboot/threads/PoolableExecutors.java +++ b/firebase-messaging-directboot/src/main/java/com/google/firebase/messaging/directboot/threads/PoolableExecutors.java @@ -14,6 +14,7 @@ package com.google.firebase.messaging.directboot.threads; +import android.annotation.SuppressLint; import androidx.annotation.NonNull; import com.google.errorprone.annotations.CompileTimeConstant; import java.util.concurrent.ExecutorService; @@ -52,6 +53,8 @@ private static class DefaultExecutorFactory implements ExecutorFactory { /** {@inheritDoc} */ @NonNull @Override + // TODO(b/258424598): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public ExecutorService newThreadPool(ThreadPriority priority) { // NOTE: Cached threadpools automatically time out all threads. They have no concept of core // threads; the queue blocks until a thread is started. @@ -61,6 +64,8 @@ public ExecutorService newThreadPool(ThreadPriority priority) { /** {@inheritDoc} */ @NonNull @Override + // TODO(b/258424598): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public ExecutorService newThreadPool(ThreadFactory threadFactory, ThreadPriority priority) { return Executors.unconfigurableExecutorService(Executors.newCachedThreadPool(threadFactory)); } @@ -79,6 +84,8 @@ public ExecutorService newThreadPool(int maxConcurrency, ThreadPriority priority /** {@inheritDoc} */ @NonNull @Override + // TODO(b/258424598): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public ExecutorService newThreadPool( int maxConcurrency, ThreadFactory threadFactory, ThreadPriority priority) { ThreadPoolExecutor executor = @@ -115,6 +122,8 @@ public ExecutorService newSingleThreadExecutor( /** {@inheritDoc} */ @NonNull @Override + // TODO(b/258424598): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public ScheduledExecutorService newScheduledThreadPool( int maxConcurrency, ThreadPriority priority) { // NOTE: There's no way to make a scheduled executor stop threads automatically, because @@ -128,6 +137,8 @@ public ScheduledExecutorService newScheduledThreadPool( /** {@inheritDoc} */ @NonNull @Override + // TODO(b/258424598): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public ScheduledExecutorService newScheduledThreadPool( int maxConcurrency, ThreadFactory threadFactory, ThreadPriority priority) { return Executors.unconfigurableScheduledExecutorService( @@ -136,6 +147,8 @@ public ScheduledExecutorService newScheduledThreadPool( @Override @NonNull + // TODO(b/258424598): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public void executeOneOff( @CompileTimeConstant final String moduleName, @CompileTimeConstant final String name, @@ -146,6 +159,8 @@ public void executeOneOff( @Override @NonNull + // TODO(b/258424598): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public Future submitOneOff( @CompileTimeConstant final String moduleName, @CompileTimeConstant final String name, diff --git a/firebase-messaging/firebase-messaging.gradle b/firebase-messaging/firebase-messaging.gradle index a220aae3c1f..93a4a9994c5 100644 --- a/firebase-messaging/firebase-messaging.gradle +++ b/firebase-messaging/firebase-messaging.gradle @@ -57,11 +57,6 @@ android { timeOutInMs 60 * 1000 } - - lintOptions { - abortOnError false - } - compileSdkVersion project.targetSdkVersion defaultConfig { minSdkVersion 19 diff --git a/firebase-messaging/src/main/java/com/google/firebase/messaging/FcmExecutors.java b/firebase-messaging/src/main/java/com/google/firebase/messaging/FcmExecutors.java index 90bfae54107..385af046e99 100644 --- a/firebase-messaging/src/main/java/com/google/firebase/messaging/FcmExecutors.java +++ b/firebase-messaging/src/main/java/com/google/firebase/messaging/FcmExecutors.java @@ -15,6 +15,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; +import android.annotation.SuppressLint; import com.google.android.gms.common.util.concurrent.NamedThreadFactory; import com.google.firebase.messaging.threads.PoolableExecutors; import com.google.firebase.messaging.threads.ThreadPriority; @@ -54,6 +55,8 @@ static Executor newFileIOExecutor() { } @SuppressWarnings("ThreadChecker") + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private static Executor newCachedSingleThreadExecutor(String threadName) { // Creates a single threaded executor that only keeps the thread alive for a short time when // idle to reduce resource use. @@ -68,23 +71,31 @@ private static Executor newCachedSingleThreadExecutor(String threadName) { /** Creates a single threaded ScheduledPoolExecutor. */ @SuppressWarnings("ThreadChecker") + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") static ScheduledExecutorService newTopicsSyncExecutor() { return new ScheduledThreadPoolExecutor( /* corePoolSize= */ 1, new NamedThreadFactory(THREAD_TOPICS_IO)); } @SuppressWarnings("ThreadChecker") + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") static ExecutorService newNetworkIOExecutor() { // TODO(b/148493968): consider use PoolableExecutors for all FCM threading return Executors.newSingleThreadExecutor(new NamedThreadFactory(THREAD_NETWORK_IO)); } @SuppressWarnings("ThreadChecker") + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") static ExecutorService newTaskExecutor() { return Executors.newSingleThreadExecutor(new NamedThreadFactory(THREAD_TASK)); } @SuppressWarnings("ThreadChecker") + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") static ExecutorService newFileExecutor() { return Executors.newSingleThreadExecutor(new NamedThreadFactory(THREAD_FILE)); } @@ -97,6 +108,8 @@ static ExecutorService newIntentHandleExecutor() { /** Creates a single threaded ScheduledPoolExecutor. */ @SuppressWarnings("ThreadChecker") + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") static ScheduledExecutorService newInitExecutor() { return new ScheduledThreadPoolExecutor( /* corePoolSize= */ 1, new NamedThreadFactory(THREAD_INIT)); diff --git a/firebase-messaging/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java b/firebase-messaging/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java index ac9dedb21ce..c3fd8e2419c 100644 --- a/firebase-messaging/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java +++ b/firebase-messaging/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java @@ -367,6 +367,7 @@ public boolean isNotificationDelegationEnabled() { * @param enable Whether to enable or disable notification delegation. * @return A Task that completes when the notification delegation has been set. */ + @NonNull public Task setNotificationDelegationEnabled(boolean enable) { return ProxyNotificationInitializer.setEnableProxyNotification(initExecutor, context, enable); } @@ -548,6 +549,8 @@ synchronized void syncWithDelaySecondsInternal(long delaySeconds) { } @SuppressWarnings("FutureReturnValueIgnored") + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") void enqueueTaskWithDelaySeconds(Runnable task, long delaySeconds) { synchronized (FirebaseMessaging.class) { if (syncExecutor == null) { diff --git a/firebase-messaging/src/main/java/com/google/firebase/messaging/SyncTask.java b/firebase-messaging/src/main/java/com/google/firebase/messaging/SyncTask.java index 55d842a00a5..77230e96ab8 100644 --- a/firebase-messaging/src/main/java/com/google/firebase/messaging/SyncTask.java +++ b/firebase-messaging/src/main/java/com/google/firebase/messaging/SyncTask.java @@ -44,6 +44,8 @@ class SyncTask implements Runnable { private final FirebaseMessaging firebaseMessaging; @VisibleForTesting + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") ExecutorService processorExecutor = new ThreadPoolExecutor( /* corePoolSize= */ 0, diff --git a/firebase-messaging/src/main/java/com/google/firebase/messaging/WithinAppServiceConnection.java b/firebase-messaging/src/main/java/com/google/firebase/messaging/WithinAppServiceConnection.java index 3f2b7905431..ca0740b000f 100644 --- a/firebase-messaging/src/main/java/com/google/firebase/messaging/WithinAppServiceConnection.java +++ b/firebase-messaging/src/main/java/com/google/firebase/messaging/WithinAppServiceConnection.java @@ -16,6 +16,7 @@ import static com.google.firebase.messaging.FirebaseMessaging.TAG; import static com.google.firebase.messaging.WakeLockHolder.WAKE_LOCK_ACQUIRE_TIMEOUT_MILLIS; +import android.annotation.SuppressLint; import android.content.ComponentName; import android.content.Context; import android.content.Intent; @@ -104,6 +105,8 @@ void finish() { @GuardedBy("this") private boolean connectionInProgress = false; + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") WithinAppServiceConnection(Context context, String action) { // Class instances are owned by a static variable in FirebaseInstanceIdReceiver // and GcmReceiver so that they survive getting gc'd and reinstantiated, so use a diff --git a/firebase-messaging/src/main/java/com/google/firebase/messaging/threads/PoolableExecutors.java b/firebase-messaging/src/main/java/com/google/firebase/messaging/threads/PoolableExecutors.java index 18e50fa0373..dabdecbf795 100644 --- a/firebase-messaging/src/main/java/com/google/firebase/messaging/threads/PoolableExecutors.java +++ b/firebase-messaging/src/main/java/com/google/firebase/messaging/threads/PoolableExecutors.java @@ -14,6 +14,7 @@ package com.google.firebase.messaging.threads; +import android.annotation.SuppressLint; import androidx.annotation.NonNull; import com.google.errorprone.annotations.CompileTimeConstant; import java.util.concurrent.ExecutorService; @@ -52,6 +53,8 @@ private static class DefaultExecutorFactory implements ExecutorFactory { /** {@inheritDoc} */ @NonNull @Override + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public ExecutorService newThreadPool(ThreadPriority priority) { // NOTE: Cached threadpools automatically time out all threads. They have no concept of core // threads; the queue blocks until a thread is started. @@ -61,6 +64,8 @@ public ExecutorService newThreadPool(ThreadPriority priority) { /** {@inheritDoc} */ @NonNull @Override + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public ExecutorService newThreadPool(ThreadFactory threadFactory, ThreadPriority priority) { return Executors.unconfigurableExecutorService(Executors.newCachedThreadPool(threadFactory)); } @@ -79,6 +84,8 @@ public ExecutorService newThreadPool(int maxConcurrency, ThreadPriority priority /** {@inheritDoc} */ @NonNull @Override + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public ExecutorService newThreadPool( int maxConcurrency, ThreadFactory threadFactory, ThreadPriority priority) { ThreadPoolExecutor executor = @@ -115,6 +122,8 @@ public ExecutorService newSingleThreadExecutor( /** {@inheritDoc} */ @NonNull @Override + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public ScheduledExecutorService newScheduledThreadPool( int maxConcurrency, ThreadPriority priority) { // NOTE: There's no way to make a scheduled executor stop threads automatically, because @@ -128,6 +137,8 @@ public ScheduledExecutorService newScheduledThreadPool( /** {@inheritDoc} */ @NonNull @Override + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public ScheduledExecutorService newScheduledThreadPool( int maxConcurrency, ThreadFactory threadFactory, ThreadPriority priority) { return Executors.unconfigurableScheduledExecutorService( @@ -136,6 +147,8 @@ public ScheduledExecutorService newScheduledThreadPool( @Override @NonNull + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public void executeOneOff( @CompileTimeConstant final String moduleName, @CompileTimeConstant final String name, @@ -146,6 +159,8 @@ public void executeOneOff( @Override @NonNull + // TODO(b/258424124): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public Future submitOneOff( @CompileTimeConstant final String moduleName, @CompileTimeConstant final String name, diff --git a/firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/FirebaseModelDownloader.java b/firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/FirebaseModelDownloader.java index cbf8fb1f894..ac67275d045 100644 --- a/firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/FirebaseModelDownloader.java +++ b/firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/FirebaseModelDownloader.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.firebase.ml.modeldownloader; +import android.annotation.SuppressLint; import android.os.Build.VERSION_CODES; import android.util.Log; import androidx.annotation.NonNull; @@ -51,6 +52,8 @@ public class FirebaseModelDownloader { private final FirebaseMlLogger eventLogger; @RequiresApi(api = VERSION_CODES.KITKAT) + // TODO(b/258424267): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") FirebaseModelDownloader( FirebaseApp firebaseApp, FirebaseInstallationsApi firebaseInstallationsApi) { this.firebaseOptions = firebaseApp.getOptions(); diff --git a/firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/internal/CustomModelDownloadService.java b/firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/internal/CustomModelDownloadService.java index 1775ba00d79..b046a6a3778 100644 --- a/firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/internal/CustomModelDownloadService.java +++ b/firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/internal/CustomModelDownloadService.java @@ -14,6 +14,7 @@ package com.google.firebase.ml.modeldownloader.internal; +import android.annotation.SuppressLint; import android.content.Context; import android.content.pm.PackageManager; import android.text.TextUtils; @@ -93,6 +94,8 @@ public class CustomModelDownloadService { private final Context context; private String downloadHost = FIREBASE_DOWNLOAD_HOST; + // TODO(b/258424267): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public CustomModelDownloadService( FirebaseApp firebaseApp, FirebaseInstallationsApi installationsApi) { context = firebaseApp.getApplicationContext(); diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/config/DeviceCacheManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/config/DeviceCacheManager.java index 012edfc8b81..e62f7e248f4 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/config/DeviceCacheManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/config/DeviceCacheManager.java @@ -14,6 +14,7 @@ package com.google.firebase.perf.config; +import android.annotation.SuppressLint; import android.content.Context; import android.content.SharedPreferences; import androidx.annotation.Nullable; @@ -49,6 +50,8 @@ public DeviceCacheManager(ExecutorService serialExecutor) { this.serialExecutor = serialExecutor; } + // TODO(b/258263016): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public static synchronized DeviceCacheManager getInstance() { if (instance == null) { instance = new DeviceCacheManager(Executors.newSingleThreadExecutor()); diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java index 7c27b59c3d9..06c505ddd51 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java @@ -16,6 +16,7 @@ import static com.google.firebase.perf.config.ConfigurationConstants.ExperimentTTID; +import android.annotation.SuppressLint; import android.content.Context; import android.content.pm.PackageInfo; import android.content.pm.PackageManager.NameNotFoundException; @@ -66,6 +67,8 @@ public class RemoteConfigManager { @Nullable private Provider firebaseRemoteConfigProvider; @Nullable private FirebaseRemoteConfig firebaseRemoteConfig; + // TODO(b/258263016): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private RemoteConfigManager() { this( DeviceCacheManager.getInstance(), diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java index 2e1bb5cd8d3..16c0a93b358 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java @@ -14,6 +14,7 @@ package com.google.firebase.perf.metrics; +import android.annotation.SuppressLint; import android.app.Activity; import android.app.Application; import android.app.Application.ActivityLifecycleCallbacks; @@ -133,6 +134,8 @@ public static AppStartTrace getInstance() { return instance != null ? instance : getInstance(TransportManager.getInstance(), new Clock()); } + // TODO(b/258263016): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") static AppStartTrace getInstance(TransportManager transportManager, Clock clock) { if (instance == null) { synchronized (AppStartTrace.class) { diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java index b482268adb3..ad9b1c04d00 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java @@ -79,6 +79,8 @@ public void setApplicationContext(final Context appContext) { // Get PerfSession in main thread first, because it is possible that app changes fg/bg state // which creates a new perfSession, before the following is executed in background thread final PerfSession appStartSession = perfSession; + // TODO(b/258263016): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") ExecutorService executorService = Executors.newSingleThreadExecutor(); syncInitFuture = executorService.submit( diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java index c34fd31dc72..e33d363c0aa 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java @@ -16,6 +16,7 @@ import static android.system.Os.sysconf; +import android.annotation.SuppressLint; import android.os.Build.VERSION; import android.os.Build.VERSION_CODES; import android.system.OsConstants; @@ -80,6 +81,8 @@ public class CpuGaugeCollector { @Nullable private ScheduledFuture cpuMetricCollectorJob = null; private long cpuMetricCollectionRateMs = UNSET_CPU_METRIC_COLLECTION_RATE; + // TODO(b/258263016): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") CpuGaugeCollector() { cpuMetricReadings = new ConcurrentLinkedQueue<>(); cpuMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java index 64cd5998a3a..0d15d3519c1 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java @@ -14,6 +14,7 @@ package com.google.firebase.perf.session.gauges; +import android.annotation.SuppressLint; import android.content.Context; import androidx.annotation.Keep; import androidx.annotation.Nullable; @@ -63,9 +64,11 @@ public class GaugeManager { private ApplicationProcessState applicationProcessState = ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN; + // TODO(b/258263016): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private GaugeManager() { this( - new Lazy<>(() -> Executors.newSingleThreadScheduledExecutor()), + new Lazy<>(Executors::newSingleThreadScheduledExecutor), TransportManager.getInstance(), ConfigResolver.getInstance(), null, diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java index 8c3a3706a19..b221ba07760 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java @@ -14,6 +14,7 @@ package com.google.firebase.perf.session.gauges; +import android.annotation.SuppressLint; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.google.firebase.perf.logging.AndroidLogger; @@ -52,6 +53,8 @@ public class MemoryGaugeCollector { @Nullable private ScheduledFuture memoryMetricCollectorJob = null; private long memoryMetricCollectionRateMs = UNSET_MEMORY_METRIC_COLLECTION_RATE; + // TODO(b/258263016): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") MemoryGaugeCollector() { this(Executors.newSingleThreadScheduledExecutor(), Runtime.getRuntime()); } diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/transport/TransportManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/transport/TransportManager.java index fab187ee6bf..dc99c77aa42 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/transport/TransportManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/transport/TransportManager.java @@ -17,6 +17,7 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MINUTES; +import android.annotation.SuppressLint; import android.content.Context; import android.content.pm.PackageInfo; import android.content.pm.PackageManager.NameNotFoundException; @@ -125,6 +126,8 @@ public class TransportManager implements AppStateCallback { private boolean isForegroundState = false; + // TODO(b/258263016): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private TransportManager() { // MAX_POOL_SIZE must always be 1. We only allow one thread in this Executor. The reason // we specifically use a ThreadPoolExecutor rather than generating one from ExecutorService diff --git a/firebase-segmentation/firebase-segmentation.gradle b/firebase-segmentation/firebase-segmentation.gradle index 215e75fa43e..c5081915348 100644 --- a/firebase-segmentation/firebase-segmentation.gradle +++ b/firebase-segmentation/firebase-segmentation.gradle @@ -38,6 +38,7 @@ android { } dependencies { + implementation project(':firebase-annotations') implementation project(':firebase-common') implementation project(':firebase-components') implementation project(':firebase-installations-interop') diff --git a/firebase-segmentation/src/main/java/com/google/firebase/segmentation/FirebaseSegmentation.java b/firebase-segmentation/src/main/java/com/google/firebase/segmentation/FirebaseSegmentation.java index b7e70bfef69..c9ba9550b38 100644 --- a/firebase-segmentation/src/main/java/com/google/firebase/segmentation/FirebaseSegmentation.java +++ b/firebase-segmentation/src/main/java/com/google/firebase/segmentation/FirebaseSegmentation.java @@ -30,7 +30,6 @@ import com.google.firebase.segmentation.remote.SegmentationServiceClient.Code; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; -import java.util.concurrent.Executors; /** Entry point of Firebase Segmentation SDK. */ public class FirebaseSegmentation { @@ -43,24 +42,29 @@ public class FirebaseSegmentation { private final SegmentationServiceClient backendServiceClient; private final Executor executor; - FirebaseSegmentation(FirebaseApp firebaseApp, FirebaseInstallationsApi firebaseInstallationsApi) { + FirebaseSegmentation( + FirebaseApp firebaseApp, + FirebaseInstallationsApi firebaseInstallationsApi, + Executor blockingExecutor) { this( firebaseApp, firebaseInstallationsApi, new CustomInstallationIdCache(firebaseApp), - new SegmentationServiceClient(firebaseApp.getApplicationContext())); + new SegmentationServiceClient(firebaseApp.getApplicationContext()), + blockingExecutor); } FirebaseSegmentation( FirebaseApp firebaseApp, FirebaseInstallationsApi firebaseInstallationsApi, CustomInstallationIdCache localCache, - SegmentationServiceClient backendServiceClient) { + SegmentationServiceClient backendServiceClient, + Executor blockingExecutor) { this.firebaseApp = firebaseApp; this.firebaseInstallationsApi = firebaseInstallationsApi; this.localCache = localCache; this.backendServiceClient = backendServiceClient; - this.executor = Executors.newFixedThreadPool(4); + this.executor = blockingExecutor; } /** diff --git a/firebase-segmentation/src/main/java/com/google/firebase/segmentation/FirebaseSegmentationRegistrar.java b/firebase-segmentation/src/main/java/com/google/firebase/segmentation/FirebaseSegmentationRegistrar.java index d005b206677..1620d043ae3 100644 --- a/firebase-segmentation/src/main/java/com/google/firebase/segmentation/FirebaseSegmentationRegistrar.java +++ b/firebase-segmentation/src/main/java/com/google/firebase/segmentation/FirebaseSegmentationRegistrar.java @@ -16,13 +16,16 @@ import androidx.annotation.NonNull; import com.google.firebase.FirebaseApp; +import com.google.firebase.annotations.concurrent.Blocking; import com.google.firebase.components.Component; import com.google.firebase.components.ComponentRegistrar; import com.google.firebase.components.Dependency; +import com.google.firebase.components.Qualified; import com.google.firebase.installations.FirebaseInstallationsApi; import com.google.firebase.platforminfo.LibraryVersionComponent; import java.util.Arrays; import java.util.List; +import java.util.concurrent.Executor; /** @hide */ public class FirebaseSegmentationRegistrar implements ComponentRegistrar { @@ -31,15 +34,19 @@ public class FirebaseSegmentationRegistrar implements ComponentRegistrar { @Override @NonNull public List> getComponents() { + Qualified blockingExecutor = Qualified.qualified(Blocking.class, Executor.class); return Arrays.asList( Component.builder(FirebaseSegmentation.class) .name(LIBRARY_NAME) .add(Dependency.required(FirebaseApp.class)) .add(Dependency.required(FirebaseInstallationsApi.class)) + .add(Dependency.required(blockingExecutor)) .factory( c -> new FirebaseSegmentation( - c.get(FirebaseApp.class), c.get(FirebaseInstallationsApi.class))) + c.get(FirebaseApp.class), + c.get(FirebaseInstallationsApi.class), + c.get(blockingExecutor))) .build(), LibraryVersionComponent.create(LIBRARY_NAME, BuildConfig.VERSION_NAME)); } diff --git a/firebase-segmentation/src/test/java/com/google/firebase/segmentation/FirebaseSegmentationTest.java b/firebase-segmentation/src/test/java/com/google/firebase/segmentation/FirebaseSegmentationTest.java index 6a8a37485c2..1e6fb7b7106 100644 --- a/firebase-segmentation/src/test/java/com/google/firebase/segmentation/FirebaseSegmentationTest.java +++ b/firebase-segmentation/src/test/java/com/google/firebase/segmentation/FirebaseSegmentationTest.java @@ -134,7 +134,11 @@ public void cleanUp() throws Exception { public void testUpdateCustomInstallationId_CacheOk_BackendOk() throws Exception { FirebaseSegmentation firebaseSegmentation = new FirebaseSegmentation( - firebaseApp, firebaseInstallationsApi, actualCache, backendClientReturnsOk); + firebaseApp, + firebaseInstallationsApi, + actualCache, + backendClientReturnsOk, + taskExecutor); // No exception, means success. TestOnCompleteListener onCompleteListener = new TestOnCompleteListener<>(); @@ -153,7 +157,11 @@ public void testUpdateCustomInstallationId_CacheOk_BackendError_Retryable() throws InterruptedException { FirebaseSegmentation firebaseSegmentation = new FirebaseSegmentation( - firebaseApp, firebaseInstallationsApi, actualCache, backendClientReturnsError); + firebaseApp, + firebaseInstallationsApi, + actualCache, + backendClientReturnsError, + taskExecutor); // Expect exception try { @@ -185,7 +193,11 @@ public void testUpdateCustomInstallationId_CacheOk_BackendError_NotRetryable() .thenReturn(SegmentationServiceClient.Code.CONFLICT); FirebaseSegmentation firebaseSegmentation = new FirebaseSegmentation( - firebaseApp, firebaseInstallationsApi, actualCache, backendClientReturnsError); + firebaseApp, + firebaseInstallationsApi, + actualCache, + backendClientReturnsError, + taskExecutor); // Expect exception try { @@ -210,7 +222,11 @@ public void testUpdateCustomInstallationId_CacheOk_BackendError_NotRetryable() public void testUpdateCustomInstallationId_CacheError_BackendOk() throws InterruptedException { FirebaseSegmentation firebaseSegmentation = new FirebaseSegmentation( - firebaseApp, firebaseInstallationsApi, cacheReturnsError, backendClientReturnsOk); + firebaseApp, + firebaseInstallationsApi, + cacheReturnsError, + backendClientReturnsOk, + taskExecutor); // Expect exception try { @@ -237,7 +253,11 @@ public void testClearCustomInstallationId_CacheOk_BackendOk() throws Exception { CustomInstallationIdCache.CacheStatus.SYNCED)); FirebaseSegmentation firebaseSegmentation = new FirebaseSegmentation( - firebaseApp, firebaseInstallationsApi, actualCache, backendClientReturnsOk); + firebaseApp, + firebaseInstallationsApi, + actualCache, + backendClientReturnsOk, + taskExecutor); // No exception, means success. TestOnCompleteListener onCompleteListener = new TestOnCompleteListener<>(); @@ -258,7 +278,11 @@ public void testClearCustomInstallationId_CacheOk_BackendError() throws Exceptio CustomInstallationIdCache.CacheStatus.SYNCED)); FirebaseSegmentation firebaseSegmentation = new FirebaseSegmentation( - firebaseApp, firebaseInstallationsApi, actualCache, backendClientReturnsError); + firebaseApp, + firebaseInstallationsApi, + actualCache, + backendClientReturnsError, + taskExecutor); // Expect exception try { @@ -286,7 +310,11 @@ public void testClearCustomInstallationId_CacheOk_BackendError() throws Exceptio public void testClearCustomInstallationId_CacheError_BackendOk() throws InterruptedException { FirebaseSegmentation firebaseSegmentation = new FirebaseSegmentation( - firebaseApp, firebaseInstallationsApi, cacheReturnsError, backendClientReturnsOk); + firebaseApp, + firebaseInstallationsApi, + cacheReturnsError, + backendClientReturnsOk, + taskExecutor); // Expect exception try { diff --git a/firebase-storage/src/main/java/com/google/firebase/storage/StorageTaskScheduler.java b/firebase-storage/src/main/java/com/google/firebase/storage/StorageTaskScheduler.java index d428ef3692f..1245e9668ea 100644 --- a/firebase-storage/src/main/java/com/google/firebase/storage/StorageTaskScheduler.java +++ b/firebase-storage/src/main/java/com/google/firebase/storage/StorageTaskScheduler.java @@ -14,6 +14,7 @@ package com.google.firebase.storage; +import android.annotation.SuppressLint; import androidx.annotation.NonNull; import androidx.annotation.RestrictTo; import java.util.concurrent.BlockingQueue; @@ -36,21 +37,33 @@ public class StorageTaskScheduler { public static StorageTaskScheduler sInstance = new StorageTaskScheduler(); private static BlockingQueue mCommandQueue = new LinkedBlockingQueue<>(); + + // TODO(b/258426744): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private static final ThreadPoolExecutor COMMAND_POOL_EXECUTOR = new ThreadPoolExecutor( 5, 5, 5, TimeUnit.SECONDS, mCommandQueue, new StorageThreadFactory("Command-")); private static BlockingQueue mUploadQueue = new LinkedBlockingQueue<>(); + + // TODO(b/258426744): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private static final ThreadPoolExecutor UPLOAD_QUEUE_EXECUTOR = new ThreadPoolExecutor( 2, 2, 5, TimeUnit.SECONDS, mUploadQueue, new StorageThreadFactory("Upload-")); private static BlockingQueue mDownloadQueue = new LinkedBlockingQueue<>(); + + // TODO(b/258426744): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private static final ThreadPoolExecutor DOWNLOAD_QUEUE_EXECUTOR = new ThreadPoolExecutor( 3, 3, 5, TimeUnit.SECONDS, mDownloadQueue, new StorageThreadFactory("Download-")); private static BlockingQueue mCallbackQueue = new LinkedBlockingQueue<>(); + + // TODO(b/258426744): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") private static final ThreadPoolExecutor CALLBACK_QUEUE_EXECUTOR = new ThreadPoolExecutor( 1, 1, 5, TimeUnit.SECONDS, mCallbackQueue, new StorageThreadFactory("Callbacks-")); @@ -101,6 +114,8 @@ static class StorageThreadFactory implements ThreadFactory { @Override @SuppressWarnings("ThreadPriorityCheck") + // TODO(b/258426744): Migrate to go/firebase-android-executors + @SuppressLint("ThreadPoolCreation") public Thread newThread(@NonNull Runnable r) { Thread t = new Thread(r, "FirebaseStorage-" + mNameSuffix + threadNumber.getAndIncrement()); t.setDaemon(false); diff --git a/tools/lint/src/main/kotlin/CheckRegistry.kt b/tools/lint/src/main/kotlin/CheckRegistry.kt index 4b995e55cb4..15fa532cf14 100644 --- a/tools/lint/src/main/kotlin/CheckRegistry.kt +++ b/tools/lint/src/main/kotlin/CheckRegistry.kt @@ -28,9 +28,8 @@ class CheckRegistry : IssueRegistry() { KotlinInteropDetector.PLATFORM_NULLNESS, NonAndroidxNullabilityDetector.NON_ANDROIDX_NULLABILITY, DeferredApiDetector.INVALID_DEFERRED_API_USE, - ProviderAssignmentDetector.INVALID_PROVIDER_ASSIGNMENT - // TODO(vkryachko): enable the check after suppressing current violations. - // ThreadPoolDetector.THREAD_POOL_CREATION + ProviderAssignmentDetector.INVALID_PROVIDER_ASSIGNMENT, + ThreadPoolDetector.THREAD_POOL_CREATION ) override val api: Int diff --git a/tools/lint/src/main/kotlin/ThreadPoolDetector.kt b/tools/lint/src/main/kotlin/ThreadPoolDetector.kt index 46148b5b0f7..7cd5bc01d6c 100644 --- a/tools/lint/src/main/kotlin/ThreadPoolDetector.kt +++ b/tools/lint/src/main/kotlin/ThreadPoolDetector.kt @@ -22,6 +22,13 @@ class ThreadPoolDetector : Detector(), SourceCodeScanner { "newWorkStealingPool" ) + override fun getApplicableConstructorTypes(): List = listOf( + "java.lang.Thread", + "java.util.concurrent.ForkJoinPool", + "java.util.concurrent.ThreadPoolExecutor", + "java.util.concurrent.ScheduledThreadPoolExecutor" + ) + override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) { if (!isExecutorMethod(method)) { return @@ -30,7 +37,20 @@ class ThreadPoolDetector : Detector(), SourceCodeScanner { context.report( THREAD_POOL_CREATION, context.getCallLocation(node, includeReceiver = false, includeArguments = true), - "Creating thread pools is not allowed.") + "Creating thread pools is not allowed." + ) + } + + override fun visitConstructor( + context: JavaContext, + node: UCallExpression, + constructor: PsiMethod + ) { + context.report( + THREAD_POOL_CREATION, + context.getCallLocation(node, includeReceiver = false, includeArguments = true), + "Creating threads or thread pools is not allowed." + ) } private fun isExecutorMethod(method: PsiMethod): Boolean { diff --git a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/ExecutionModule.java b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/ExecutionModule.java index 64eeae96a48..17643e1ee35 100644 --- a/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/ExecutionModule.java +++ b/transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/ExecutionModule.java @@ -14,6 +14,7 @@ package com.google.android.datatransport.runtime; +import android.annotation.SuppressLint; import dagger.Module; import dagger.Provides; import java.util.concurrent.Executor; @@ -24,6 +25,7 @@ abstract class ExecutionModule { @Singleton @Provides + @SuppressLint("ThreadPoolCreation") static Executor executor() { return new SafeLoggingExecutor(Executors.newSingleThreadExecutor()); }