Skip to content

Commit

Permalink
Add an option to use string keys for dagger.android and allow the key…
Browse files Browse the repository at this point in the history
…s to be obfuscated

This is enabled by default internally, where proguard understands -identifiernamestring, but disabled externally where it doesn't (and where R8 is not yet the standard). It can be enabled with the -Adagger.android.experimentalUseStringKeys flag for users that don't use proguard. This way we can guarantee that any project without modification will continue to work when proguarded.

This solves an issue where classloading of every Activity/Fragment/Service injector that's a child of the root component is forced into application startup. It also solves an issue where framework components that were introduced in newer versions of Android cannot be easily added without producing separate binaries, adding @ContributesAndroidInjector conditionally depending on the minimum API version.

Fixes #1064

I've decided to hold off on documenting this until we have a viable solution externally (when R8 is the standard, and it supports META-INF/proguard files).

RELNOTES:
  - If you use `dagger.android` and you aren't already installing `AndroidInjectionModule` (or `AndroidSupportInjectionModule`), you should do so, otherwise you are likely to get a missing binding error from Dagger when you upgrade.
  - Add an option to use string keys for dagger.android and allow the keys to be obfuscated.
    - Note that if you use a bytecode shrinker/optimizer, this is only viable if you use a version of R8 that can read generated proguard rules (or if you extract the contents of the generated rule and include it in your build)
    - You can enable this mode with the flag `-Adagger.android.experimentalUseStringKeys`.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=204987571
  • Loading branch information
ronshapiro committed Aug 1, 2018
1 parent e42c864 commit 3a08964
Show file tree
Hide file tree
Showing 30 changed files with 1,598 additions and 68 deletions.
44 changes: 44 additions & 0 deletions java/dagger/android/AndroidInjectionKey.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright (C) 2018 The Dagger Authors.
*
* 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 dagger.android;

import static java.lang.annotation.ElementType.METHOD;

import dagger.MapKey;
import dagger.internal.Beta;
import java.lang.annotation.Documented;
import java.lang.annotation.Target;

/**
* {@link MapKey} annotation to key {@link AndroidInjector.Factory} bindings. The {@linkplain
* #value() value} of the annotation is the canonical name of the class that will be passed to
* {@link AndroidInjector#inject(Object)}.
*
* <p>All key strings will be obfuscated by ProGuard/R8/AppReduce if the named class is obfuscated.
*
* <p>
* You should only use this annotation if you are using a version of ProGuard/R8/AppReduce that
* supports the {@code -identifiernamestring} flag.
*/
@Beta
@MapKey
@Target(METHOD)
@Documented
public @interface AndroidInjectionKey {
/** The fully qualified class name of the type to be injected. */
String value();
}
23 changes: 21 additions & 2 deletions java/dagger/android/AndroidInjectionModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,42 @@ public abstract class AndroidInjectionModule {
abstract Map<Class<? extends Activity>, AndroidInjector.Factory<? extends Activity>>
activityInjectorFactories();

@Multibinds
abstract Map<String, AndroidInjector.Factory<? extends Activity>>
activityInjectorFactoriesWithStringKeys();

@Multibinds
abstract Map<Class<? extends Fragment>, AndroidInjector.Factory<? extends Fragment>>
fragmentInjectorFactories();

@Multibinds
abstract Map<String, AndroidInjector.Factory<? extends Fragment>>
fragmentInjectorFactoriesWithStringKeys();

@Multibinds
abstract Map<Class<? extends Service>, AndroidInjector.Factory<? extends Service>>
serviceInjectorFactories();

@Multibinds
abstract Map<String, AndroidInjector.Factory<? extends Service>>
serviceInjectorFactoriesWithStringKeys();

@Multibinds
abstract Map<
Class<? extends BroadcastReceiver>, AndroidInjector.Factory<? extends BroadcastReceiver>>
broadcastReceiverInjectorFactories();

@Multibinds
abstract Map<
Class<? extends ContentProvider>, AndroidInjector.Factory<? extends ContentProvider>>
abstract Map<String, AndroidInjector.Factory<? extends BroadcastReceiver>>
broadcastReceiverInjectorFactoriesWithStringKeys();

@Multibinds
abstract Map<Class<? extends ContentProvider>, AndroidInjector.Factory<? extends ContentProvider>>
contentProviderInjectorFactories();

@Multibinds
abstract Map<String, AndroidInjector.Factory<? extends ContentProvider>>
contentProviderInjectorFactoriesWithStringKeys();

private AndroidInjectionModule() {}
}
6 changes: 5 additions & 1 deletion java/dagger/android/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ load("//tools:maven.bzl", "pom_file", "POM_VERSION")

# Work around b/70476182 which prevents Kythe from connecting :producers to the .java files it
# contains.
SRCS = glob(["*.java"])
SRCS = glob([
"*.java",
"internal/*.java",
])

filegroup(
name = "android-srcs",
Expand Down Expand Up @@ -82,6 +85,7 @@ javadoc_library(
name = "android-javadoc",
srcs = [":android-srcs"],
android_api_level = 26,
exclude_packages = ["dagger.android.internal"],
root_packages = ["dagger.android"],
deps = [":android"],
)
47 changes: 37 additions & 10 deletions java/dagger/android/DispatchingAndroidInjector.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package dagger.android;

import static dagger.internal.DaggerCollections.newLinkedHashMapWithExpectedSize;
import static dagger.internal.Preconditions.checkNotNull;

import android.app.Activity;
Expand All @@ -26,6 +27,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import javax.inject.Inject;
import javax.inject.Provider;

Expand All @@ -47,13 +49,39 @@ public final class DispatchingAndroidInjector<T> implements AndroidInjector<T> {
"No injector factory bound for Class<%1$s>. Injector factories were bound for supertypes "
+ "of %1$s: %2$s. Did you mean to bind an injector factory for the subtype?";

private final Map<Class<? extends T>, Provider<AndroidInjector.Factory<? extends T>>>
injectorFactories;
private final Map<String, Provider<AndroidInjector.Factory<? extends T>>> injectorFactories;

@Inject
DispatchingAndroidInjector(
Map<Class<? extends T>, Provider<AndroidInjector.Factory<? extends T>>> injectorFactories) {
this.injectorFactories = injectorFactories;
Map<Class<? extends T>, Provider<Factory<? extends T>>> injectorFactoriesWithClassKeys,
Map<String, Provider<Factory<? extends T>>> injectorFactoriesWithStringKeys) {
this.injectorFactories = merge(injectorFactoriesWithClassKeys, injectorFactoriesWithStringKeys);
}

/**
* Merges the two maps into one by transforming the values of the {@code classKeyedMap} with
* {@link Class#getName()}.
*
* <p>An SPI plugin verifies the logical uniqueness of the keysets of these two maps so we're
* assured there's no overlap.
*
* <p>Ideally we could achieve this with a generic {@code @Provides} method, but we'd need to have
* <i>N</i> modules that each extend one base module.
*/
private static <C, V> Map<String, V> merge(
Map<Class<? extends C>, V> classKeyedMap, Map<String, V> stringKeyedMap) {
if (classKeyedMap.isEmpty()) {
return stringKeyedMap;
}

Map<String, V> merged =
newLinkedHashMapWithExpectedSize(classKeyedMap.size() + stringKeyedMap.size());
merged.putAll(stringKeyedMap);
for (Entry<Class<? extends C>, V> entry : classKeyedMap.entrySet()) {
merged.put(entry.getKey().getName(), entry.getValue());
}

return Collections.unmodifiableMap(merged);
}

/**
Expand All @@ -66,7 +94,7 @@ public final class DispatchingAndroidInjector<T> implements AndroidInjector<T> {
@CanIgnoreReturnValue
public boolean maybeInject(T instance) {
Provider<AndroidInjector.Factory<? extends T>> factoryProvider =
injectorFactories.get(instance.getClass());
injectorFactories.get(instance.getClass().getName());
if (factoryProvider == null) {
return false;
}
Expand Down Expand Up @@ -119,13 +147,12 @@ public static final class InvalidInjectorBindingException extends RuntimeExcepti

/** Returns an error message with the class names that are supertypes of {@code instance}. */
private String errorMessageSuggestions(T instance) {
List<String> suggestions = new ArrayList<String>();
for (Class<? extends T> activityClass : injectorFactories.keySet()) {
if (activityClass.isInstance(instance)) {
suggestions.add(activityClass.getCanonicalName());
List<String> suggestions = new ArrayList<>();
for (Class<?> clazz = instance.getClass(); clazz != null; clazz = clazz.getSuperclass()) {
if (injectorFactories.containsKey(clazz.getCanonicalName())) {
suggestions.add(clazz.getCanonicalName());
}
}
Collections.sort(suggestions);

return suggestions.isEmpty()
? String.format(NO_SUPERTYPES_BOUND_FORMAT, instance.getClass().getCanonicalName())
Expand Down
37 changes: 37 additions & 0 deletions java/dagger/android/internal/AndroidInjectionKeys.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (C) 2018 The Dagger Authors.
*
* 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 dagger.android.internal;

/**
* An internal implementation detail of Dagger's generated code. This is not guaranteed to remain
* consistent from version to version.
*/
public final class AndroidInjectionKeys {
/**
* Accepts the fully qualified name of a class that is injected with {@code dagger.android}.
*
* <p>From a runtime perspective, this method does nothing except return its single argument. It
* is used as a signal to bytecode shrinking tools that its argument should be rewritten if it
* corresponds to a class that has been obfuscated/relocated. Once it is done so, it is expected
* that the argument will be inlined and this method will go away.
*/
public static String of(String mapKey) {
return mapKey;
}

private AndroidInjectionKeys() {}
}
39 changes: 31 additions & 8 deletions java/dagger/android/processor/AndroidMapKeyValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,19 @@
package dagger.android.processor;

import static com.google.auto.common.AnnotationMirrors.getAnnotatedAnnotations;
import static com.google.auto.common.AnnotationMirrors.getAnnotationValue;
import static com.google.auto.common.MoreElements.getAnnotationMirror;
import static com.google.auto.common.MoreElements.isAnnotationPresent;
import static com.google.common.collect.Iterables.getOnlyElement;
import static dagger.android.processor.AndroidMapKeys.annotationsAndFrameworkTypes;
import static dagger.android.processor.AndroidMapKeys.injectedTypeFromMapKey;

import com.google.auto.common.BasicAnnotationProcessor.ProcessingStep;
import com.google.auto.common.MoreElements;
import com.google.auto.common.MoreTypes;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.SetMultimap;
import dagger.Binds;
import dagger.android.AndroidInjectionKey;
import dagger.android.AndroidInjector;
import java.lang.annotation.Annotation;
import java.util.Set;
Expand Down Expand Up @@ -62,7 +63,10 @@ final class AndroidMapKeyValidator implements ProcessingStep {

@Override
public Set<? extends Class<? extends Annotation>> annotations() {
return annotationsAndFrameworkTypes(elements).keySet();
return ImmutableSet.<Class<? extends Annotation>>builder()
.addAll(annotationsAndFrameworkTypes(elements).keySet())
.add(AndroidInjectionKey.class)
.build();
}

@Override
Expand All @@ -84,7 +88,7 @@ private void validateMethod(Class<? extends Annotation> annotation, ExecutableEl
return;
}

TypeMirror frameworkType = annotationsAndFrameworkTypes(elements).get(annotation);
TypeMirror frameworkType = frameworkTypeForMapKey(method, annotation);

if (!getAnnotatedAnnotations(method, Scope.class).isEmpty()) {
SuppressWarnings suppressedWarnings = method.getAnnotation(SuppressWarnings.class);
Expand All @@ -102,11 +106,16 @@ private void validateMethod(Class<? extends Annotation> annotation, ExecutableEl

DeclaredType intendedReturnType = injectorFactoryOf(types.getWildcardType(frameworkType, null));
if (!MoreTypes.equivalence().equivalent(returnType, intendedReturnType)) {
String subject =
annotation.equals(AndroidInjectionKey.class)
? method.toString()
: String.format("@%s methods", annotation.getCanonicalName());

messager.printMessage(
Kind.ERROR,
String.format(
"@%s methods should bind %s, not %s. See https://google.github.io/dagger/android",
annotation.getCanonicalName(), intendedReturnType, returnType),
"%s should bind %s, not %s. See https://google.github.io/dagger/android",
subject, intendedReturnType, returnType),
method);
}

Expand Down Expand Up @@ -135,16 +144,30 @@ private void validateMapKeyMatchesBindsParameter(
Class<? extends Annotation> annotation, ExecutableElement method) {
TypeMirror parameterType = getOnlyElement(method.getParameters()).asType();
AnnotationMirror annotationMirror = getAnnotationMirror(method, annotation).get();
TypeMirror mapKeyValue = (TypeMirror) getAnnotationValue(annotationMirror, "value").getValue();
if (!types.isAssignable(parameterType, injectorFactoryOf(mapKeyValue))) {
TypeMirror mapKeyType =
elements.getTypeElement(injectedTypeFromMapKey(annotationMirror).get()).asType();
if (!types.isAssignable(parameterType, injectorFactoryOf(mapKeyType))) {
messager.printMessage(
Kind.ERROR,
String.format("%s does not implement AndroidInjector<%s>", parameterType, mapKeyValue),
String.format("%s does not implement AndroidInjector<%s>", parameterType, mapKeyType),
method,
annotationMirror);
}
}

private TypeMirror frameworkTypeForMapKey(
ExecutableElement method, Class<? extends Annotation> annotation) {
AnnotationMirror annotationMirror = getAnnotationMirror(method, annotation).get();
TypeMirror mapKeyType =
elements.getTypeElement(injectedTypeFromMapKey(annotationMirror).get()).asType();
return annotationsAndFrameworkTypes(elements)
.values()
.stream()
.filter(frameworkType -> types.isAssignable(mapKeyType, frameworkType))
.findFirst()
.get();
}

/** Returns a {@link DeclaredType} for {@code AndroidInjector.Factory<implementationType>}. */
private DeclaredType injectorFactoryOf(TypeMirror implementationType) {
return types.getDeclaredType(factoryElement(), implementationType);
Expand Down
26 changes: 26 additions & 0 deletions java/dagger/android/processor/AndroidMapKeys.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package dagger.android.processor;

import static com.google.auto.common.AnnotationMirrors.getAnnotationValue;
import static com.google.auto.common.MoreElements.isAnnotationPresent;
import static com.google.common.collect.Iterables.getOnlyElement;
import static java.util.stream.Collectors.toMap;
Expand All @@ -25,9 +26,12 @@
import com.google.auto.common.MoreTypes;
import com.google.common.collect.ImmutableMap;
import dagger.MapKey;
import dagger.android.AndroidInjectionKey;
import java.lang.annotation.Annotation;
import java.util.List;
import java.util.Optional;
import java.util.stream.Stream;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.TypeElement;
import javax.lang.model.type.TypeMirror;
Expand All @@ -50,12 +54,17 @@ static ImmutableMap<Class<? extends Annotation>, TypeMirror> annotationsAndFrame
elements.getPackageElement("dagger.android.support"))
.filter(packageElement -> packageElement != null)
.flatMap(packageElement -> typesIn(packageElement.getEnclosedElements()).stream())
.filter(AndroidMapKeys::isNotAndroidInjectionKey)
.filter(type -> isAnnotationPresent(type, MapKey.class))
.filter(mapKey -> mapKey.getAnnotation(MapKey.class).unwrapValue())
.flatMap(AndroidMapKeys::classForAnnotationElement)
.collect(toMap(key -> key, key -> mapKeyValue(key, elements))));
}

private static boolean isNotAndroidInjectionKey(TypeElement type) {
return !type.getQualifiedName().contentEquals(AndroidInjectionKey.class.getCanonicalName());
}

private static Stream<Class<? extends Annotation>> classForAnnotationElement(TypeElement type) {
try {
@SuppressWarnings("unchecked")
Expand All @@ -75,4 +84,21 @@ private static TypeMirror mapKeyValue(Class<? extends Annotation> annotation, El
return ((WildcardType) getOnlyElement(MoreTypes.asDeclared(returnType).getTypeArguments()))
.getExtendsBound();
}

/**
* If {@code mapKey} is {@link AndroidInjectionKey}, returns the string value for the map key. If
* it's {@link dagger.android.ActivityKey} or one of the other class-based keys, returns the
* fully-qualified class name of the annotation value. Otherwise returns {@link Optional#empty()}.
*/
static Optional<String> injectedTypeFromMapKey(AnnotationMirror mapKey) {
Object mapKeyClass = getAnnotationValue(mapKey, "value").getValue();
if (mapKeyClass instanceof String) {
return Optional.of((String) mapKeyClass);
} else if (mapKeyClass instanceof TypeMirror) {
TypeElement type = MoreTypes.asTypeElement((TypeMirror) mapKeyClass);
return Optional.of(type.getQualifiedName().toString());
} else {
return Optional.empty();
}
}
}
Loading

0 comments on commit 3a08964

Please sign in to comment.