From fd81de0116a8bbef4e2656822fa3d7586e6216e9 Mon Sep 17 00:00:00 2001 From: Henri Sweers Date: Wed, 4 Mar 2015 03:31:34 -0800 Subject: [PATCH 1/6] Add ChildTestView and inheritance test --- .../io/sweers/barber/sample/BarberTest.java | 9 +++++ .../sweers/barber/sample/ChildTestView.java | 37 +++++++++++++++++++ .../src/main/res/layout/child_test_view.xml | 25 +++++++++++++ sample/src/main/res/values/attrs.xml | 4 ++ 4 files changed, 75 insertions(+) create mode 100644 sample/src/main/java/io/sweers/barber/sample/ChildTestView.java create mode 100644 sample/src/main/res/layout/child_test_view.xml diff --git a/sample/src/androidTest/java/io/sweers/barber/sample/BarberTest.java b/sample/src/androidTest/java/io/sweers/barber/sample/BarberTest.java index c0d7765..d1a64e2 100644 --- a/sample/src/androidTest/java/io/sweers/barber/sample/BarberTest.java +++ b/sample/src/androidTest/java/io/sweers/barber/sample/BarberTest.java @@ -10,12 +10,14 @@ public class BarberTest extends AndroidTestCase { private TestView testView; + private ChildTestView childTestView; private Resources res; @Override protected void setUp() throws Exception { super.setUp(); testView = (TestView) View.inflate(getContext(), R.layout.test_view, null); + childTestView = (ChildTestView) View.inflate(getContext(), R.layout.child_test_view, null); res = getContext().getResources(); } @@ -112,4 +114,11 @@ public void testNonResString() { assertEquals("Hello", testView.testNonResString1); assertNull(testView.testNonResString2); } + + @SmallTest + public void testInheritance() { + assertTrue(childTestView.testBoolean); + assertEquals(3, childTestView.testInt); + assertEquals(3, childTestView.coolNumber); + } } diff --git a/sample/src/main/java/io/sweers/barber/sample/ChildTestView.java b/sample/src/main/java/io/sweers/barber/sample/ChildTestView.java new file mode 100644 index 0000000..5edb6aa --- /dev/null +++ b/sample/src/main/java/io/sweers/barber/sample/ChildTestView.java @@ -0,0 +1,37 @@ +package io.sweers.barber.sample; + +import android.annotation.TargetApi; +import android.content.Context; +import android.os.Build; +import android.util.AttributeSet; + +import io.sweers.barber.Barber; +import io.sweers.barber.StyledAttr; + +/** + * Testing inheritance + */ +public class ChildTestView extends TestView { + + @StyledAttr(R.styleable.ChildTestView_coolNumber) + protected int coolNumber = 0; + + public ChildTestView(Context context) { + super(context); + } + + public ChildTestView(Context context, AttributeSet attrs) { + this(context, attrs, 0); + } + + public ChildTestView(Context context, AttributeSet attrs, int defStyleAttr) { + super(context, attrs, defStyleAttr); + Barber.style(this, attrs, R.styleable.ChildTestView, defStyleAttr); + } + + @TargetApi(Build.VERSION_CODES.LOLLIPOP) + public ChildTestView(Context context, AttributeSet attrs, int defStyleAttr, int defStyleRes) { + super(context, attrs, defStyleAttr, defStyleRes); + Barber.style(this, attrs, R.styleable.ChildTestView, defStyleAttr, defStyleRes); + } +} diff --git a/sample/src/main/res/layout/child_test_view.xml b/sample/src/main/res/layout/child_test_view.xml new file mode 100644 index 0000000..7f19cd0 --- /dev/null +++ b/sample/src/main/res/layout/child_test_view.xml @@ -0,0 +1,25 @@ + + \ No newline at end of file diff --git a/sample/src/main/res/values/attrs.xml b/sample/src/main/res/values/attrs.xml index 5a1a43b..0bcf4c9 100644 --- a/sample/src/main/res/values/attrs.xml +++ b/sample/src/main/res/values/attrs.xml @@ -26,4 +26,8 @@ + + + + \ No newline at end of file From 5e20556bf0a87a77a59f0fcbebab7b9e64619568 Mon Sep 17 00:00:00 2001 From: Henri Sweers Date: Wed, 4 Mar 2015 03:36:25 -0800 Subject: [PATCH 2/6] Support inheritance This is actually quite a bit of changes under the hood, but for the sake of keeping stable commits I'm going to put it all together. * Adopt Butterknife's approach to making all generates classes implement a IBarbershopInterface, and do class lookups of this interface rather than method lookups * Fix inheritance by determining if a generated class has a parent and then extending from the parent instead of implementing the IBarbershopInterface. This way, style() can just call super.style() to ensure that the entire hierarchy runs --- .../main/java/io/sweers/barber/Barber.java | 60 +++++++++---------- .../io/sweers/barber/BarberProcessor.java | 35 ++++++++++- .../java/io/sweers/barber/Barbershop.java | 37 +++++++++--- 3 files changed, 89 insertions(+), 43 deletions(-) diff --git a/api/src/main/java/io/sweers/barber/Barber.java b/api/src/main/java/io/sweers/barber/Barber.java index f3438da..2363f42 100644 --- a/api/src/main/java/io/sweers/barber/Barber.java +++ b/api/src/main/java/io/sweers/barber/Barber.java @@ -4,8 +4,6 @@ import android.util.Log; import android.view.View; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.util.LinkedHashMap; import java.util.Map; @@ -21,9 +19,9 @@ public class Barber { public static final String ANDROID_PREFIX = "android."; public static final String JAVA_PREFIX = "java."; private static final String TAG = "Barber"; - private static final Method NO_OP = null; + private static final IBarbershop NO_OP = null; private static boolean debug = false; - private static final Map, Method> BARBERSHOPS = new LinkedHashMap<>(); + private static final Map, IBarbershop> BARBERSHOPS = new LinkedHashMap<>(); public static void style(View target, AttributeSet set, int[] attrs) { style(target, set, attrs, 0); @@ -35,35 +33,26 @@ public static void style(View target, AttributeSet set, int[] attrs, int defStyl public static void style(View target, AttributeSet set, int[] attrs, int defStyleAttr, int defStyleRes) { Class targetClass = target.getClass(); - try { - if (debug) { - Log.d(TAG, "Looking up barbershop for " + targetClass.getName()); - } - Method style = findStyleMethodForClass(targetClass); - if (style != NO_OP) { - style.invoke(null, target, set, attrs, defStyleAttr, defStyleRes); - } - } catch (InvocationTargetException | NoSuchMethodException | IllegalAccessException e) { - Throwable t = e; - if (t instanceof InvocationTargetException) { - t = t.getCause(); - } - throw new RuntimeException("Unable to inject styleable value for " + target, t); + if (debug) { + Log.d(TAG, "Looking up barbershop for " + targetClass.getName()); + } + IBarbershop barbershop = findBarbershopForClass(targetClass); + if (barbershop != NO_OP) { + barbershop.style(target, set, attrs, defStyleAttr, defStyleRes); } } /** - * Searches for the style() method given an instance of a generated class. Caches for efficiency. + * Searches for $$Barbershop class for the given instance, cached for efficiency. * - * @param cls Instance of a *$$Barbershop instance - * @return style Method for the instance - * @throws NoSuchMethodException + * @param cls Source class to find a matching $$Barbershop class for + * @return $$Barbershop class instance */ - private static Method findStyleMethodForClass(Class cls) throws NoSuchMethodException { - Method style = BARBERSHOPS.get(cls); - if (style != null) { - if (debug) Log.d(TAG, "HIT: Cached in shop map."); - return style; + private static IBarbershop findBarbershopForClass(Class cls) { + IBarbershop barbershop = BARBERSHOPS.get(cls); + if (barbershop != null) { + if (debug) Log.d(TAG, "HIT: Cached in barbershop map."); + return barbershop; } String clsName = cls.getName(); if (clsName.startsWith(ANDROID_PREFIX) || clsName.startsWith(JAVA_PREFIX)) { @@ -73,8 +62,9 @@ private static Method findStyleMethodForClass(Class cls) throws NoSuchMethodE return NO_OP; } try { - Class barbershop = Class.forName(clsName + SUFFIX); - style = barbershop.getMethod("style", cls, AttributeSet.class, int[].class, int.class, int.class); + Class barbershopClass = Class.forName(clsName + SUFFIX); + //noinspection unchecked + barbershop = (IBarbershop) barbershopClass.newInstance(); if (debug) { Log.d(TAG, "HIT: Class loaded barbershop class."); } @@ -82,10 +72,16 @@ private static Method findStyleMethodForClass(Class cls) throws NoSuchMethodE if (debug) { Log.d(TAG, "Not found. Trying superclass " + cls.getSuperclass().getName()); } - style = findStyleMethodForClass(cls.getSuperclass()); + barbershop = findBarbershopForClass(cls.getSuperclass()); + } catch (InstantiationException | IllegalAccessException e) { + e.printStackTrace(); } - BARBERSHOPS.put(cls, style); - return style; + BARBERSHOPS.put(cls, barbershop); + return barbershop; } + /** DO NOT USE. Exposed for generated classes' use. */ + public interface IBarbershop { + public void style(final T target, final AttributeSet set, final int[] attrs, final int defStyleAttr, final int defStyleRes); + } } diff --git a/compiler/src/main/java/io/sweers/barber/BarberProcessor.java b/compiler/src/main/java/io/sweers/barber/BarberProcessor.java index 8c47f22..a13bd9f 100644 --- a/compiler/src/main/java/io/sweers/barber/BarberProcessor.java +++ b/compiler/src/main/java/io/sweers/barber/BarberProcessor.java @@ -5,6 +5,7 @@ import java.io.IOException; import java.util.HashSet; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; @@ -18,6 +19,9 @@ import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.TypeElement; +import javax.lang.model.type.DeclaredType; +import javax.lang.model.type.TypeKind; +import javax.lang.model.type.TypeMirror; import javax.lang.model.util.Elements; import javax.lang.model.util.Types; import javax.tools.Diagnostic; @@ -56,6 +60,7 @@ public synchronized void init(ProcessingEnvironment processingEnv) { @Override public boolean process(Set annotations, RoundEnvironment roundEnv) { Map targetClassMap = new LinkedHashMap<>(); + Set erasedTargetNames = new LinkedHashSet<>(); for (Element element : roundEnv.getElementsAnnotatedWith(StyledAttr.class)) { try { @@ -64,13 +69,20 @@ public boolean process(Set annotations, RoundEnvironment return false; } TypeElement enclosingElement = (TypeElement) element.getEnclosingElement(); - Barbershop barbershop = getOrCreateBarber(targetClassMap, enclosingElement); + Barbershop barbershop = getOrCreateBarber(targetClassMap, enclosingElement, erasedTargetNames); barbershop.createAndAddBinding(element); } catch (Exception e) { error(element, "%s", e.getMessage()); } } + for (Map.Entry entry : targetClassMap.entrySet()) { + String parentClassFqcn = findParentFqcn(entry.getKey(), erasedTargetNames); + if (parentClassFqcn != null) { + entry.getValue().setParentBarbershop(parentClassFqcn + Barber.SUFFIX); + } + } + for (Barbershop barbershop : targetClassMap.values()) { try { barbershop.writeToFiler(filer); @@ -82,7 +94,7 @@ public boolean process(Set annotations, RoundEnvironment return true; } - private Barbershop getOrCreateBarber(Map targetClassMap, TypeElement enclosingElement) { + private Barbershop getOrCreateBarber(Map targetClassMap, TypeElement enclosingElement, Set erasedTargetNames) { Barbershop barbershop = targetClassMap.get(enclosingElement); if (barbershop == null) { String targetType = enclosingElement.getQualifiedName().toString(); @@ -90,11 +102,30 @@ private Barbershop getOrCreateBarber(Map targetClassMap String className = getClassName(enclosingElement, classPackage) + Barber.SUFFIX; barbershop = new Barbershop(classPackage, className, targetType); targetClassMap.put(enclosingElement, barbershop); + erasedTargetNames.add(enclosingElement.toString()); } return barbershop; } + /** + * Finds the parent barbershop type in the supplied set, if any. + */ + private String findParentFqcn(TypeElement typeElement, Set parents) { + TypeMirror type; + while (true) { + type = typeElement.getSuperclass(); + if (type.getKind() == TypeKind.NONE) { + return null; + } + typeElement = (TypeElement) ((DeclaredType) type).asElement(); + if (parents.contains(typeElement.toString())) { + String packageName = getPackageName(typeElement); + return packageName + "." + getClassName(typeElement, packageName); + } + } + } + private void error(Element e, String msg, Object... args) { messager.printMessage(Diagnostic.Kind.ERROR, String.format(msg, args), e); } diff --git a/compiler/src/main/java/io/sweers/barber/Barbershop.java b/compiler/src/main/java/io/sweers/barber/Barbershop.java index ee03af5..9192fd3 100644 --- a/compiler/src/main/java/io/sweers/barber/Barbershop.java +++ b/compiler/src/main/java/io/sweers/barber/Barbershop.java @@ -6,7 +6,9 @@ import com.squareup.javapoet.ClassName; import com.squareup.javapoet.JavaFile; import com.squareup.javapoet.MethodSpec; +import com.squareup.javapoet.ParameterizedTypeName; import com.squareup.javapoet.TypeSpec; +import com.squareup.javapoet.TypeVariableName; import java.io.IOException; import java.util.HashMap; @@ -31,6 +33,7 @@ class Barbershop { private final String className; private final String targetClass; private final Map fieldBindings; + private String parentBarbershop; Barbershop(String classPackage, String className, String targetClass) { this.classPackage = classPackage; @@ -39,6 +42,10 @@ class Barbershop { this.fieldBindings = new HashMap<>(); } + void setParentBarbershop(String parentBarbershop) { + this.parentBarbershop = parentBarbershop; + } + /** * Generates the class code and writes to a new source file. * @@ -46,11 +53,19 @@ class Barbershop { * @throws IOException */ public void writeToFiler(Filer filer) throws IOException { - TypeSpec.Builder bridge = TypeSpec.classBuilder(className) - .addModifiers(Modifier.PUBLIC, Modifier.FINAL) + ClassName targetClassName = ClassName.get(classPackage, targetClass); + TypeSpec.Builder barberShop = TypeSpec.classBuilder(className) + .addModifiers(Modifier.PUBLIC) + .addTypeVariable(TypeVariableName.get("T", targetClassName)) .addMethod(generateStyleMethod()); - JavaFile javaFile = JavaFile.builder(classPackage, bridge.build()).build(); + if (parentBarbershop == null) { + barberShop.addSuperinterface(ParameterizedTypeName.get(ClassName.get(Barber.IBarbershop.class), TypeVariableName.get("T"))); + } else { + barberShop.superclass(ParameterizedTypeName.get(ClassName.bestGuess(parentBarbershop), TypeVariableName.get("T"))); + } + + JavaFile javaFile = JavaFile.builder(classPackage, barberShop.build()).build(); javaFile.writeTo(filer); } @@ -59,18 +74,22 @@ public void writeToFiler(Filer filer) throws IOException { * @return A complete MethodSpec implementation for the class's style() method. */ private MethodSpec generateStyleMethod() { - ClassName targetClassName = ClassName.get(classPackage, targetClass); MethodSpec.Builder builder = MethodSpec.methodBuilder("style") - .addModifiers(Modifier.PUBLIC, Modifier.STATIC, Modifier.FINAL) + .addAnnotation(Override.class) + .addModifiers(Modifier.PUBLIC) .returns(void.class) - .addParameter(targetClassName, "target", Modifier.FINAL) + .addParameter(TypeVariableName.get("T"), "target", Modifier.FINAL) .addParameter(AttributeSet.class, "set", Modifier.FINAL) .addParameter(int[].class, "attrs", Modifier.FINAL) .addParameter(int.class, "defStyleAttr", Modifier.FINAL) - .addParameter(int.class, "defStyleRes", Modifier.FINAL) + .addParameter(int.class, "defStyleRes", Modifier.FINAL); + + if (parentBarbershop != null) { + builder.addStatement("super.style(target, set, attrs, defStyleAttr, defStyleRes)"); + } - // Don't do anything if there's no AttributeSet instance - .beginControlFlow("if (set == null)") + // Don't do anything if there's no AttributeSet instance + builder.beginControlFlow("if (set == null)") .addStatement("return") .endControlFlow() From 1519452dd6320c211c593ffb725e9c2111975a5f Mon Sep 17 00:00:00 2001 From: Henri Sweers Date: Wed, 4 Mar 2015 23:27:20 -0800 Subject: [PATCH 3/6] Update inheritance tests --- .../java/io/sweers/barber/sample/BarberTest.java | 5 ++++- .../java/io/sweers/barber/sample/ChildTestView.java | 10 ++++++++-- sample/src/main/res/layout/child_test_view.xml | 4 +++- sample/src/main/res/values/attrs.xml | 4 +++- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/sample/src/androidTest/java/io/sweers/barber/sample/BarberTest.java b/sample/src/androidTest/java/io/sweers/barber/sample/BarberTest.java index d1a64e2..1b82d3c 100644 --- a/sample/src/androidTest/java/io/sweers/barber/sample/BarberTest.java +++ b/sample/src/androidTest/java/io/sweers/barber/sample/BarberTest.java @@ -118,7 +118,10 @@ public void testNonResString() { @SmallTest public void testInheritance() { assertTrue(childTestView.testBoolean); +// assertNull(childTestView.testString); assertEquals(3, childTestView.testInt); - assertEquals(3, childTestView.coolNumber); +// assertEquals(3, childTestView.childInt); +// assertTrue(childTestView.childBoolean); +// assertEquals("child", childTestView.childString); } } diff --git a/sample/src/main/java/io/sweers/barber/sample/ChildTestView.java b/sample/src/main/java/io/sweers/barber/sample/ChildTestView.java index 5edb6aa..4fe386b 100644 --- a/sample/src/main/java/io/sweers/barber/sample/ChildTestView.java +++ b/sample/src/main/java/io/sweers/barber/sample/ChildTestView.java @@ -13,8 +13,14 @@ */ public class ChildTestView extends TestView { - @StyledAttr(R.styleable.ChildTestView_coolNumber) - protected int coolNumber = 0; + @StyledAttr(R.styleable.ChildTestView_childInt) + protected int childInt = 0; + + @StyledAttr(R.styleable.ChildTestView_childBoolean) + protected boolean childBoolean = false; + + @StyledAttr(R.styleable.ChildTestView_childString) + protected String childString; public ChildTestView(Context context) { super(context); diff --git a/sample/src/main/res/layout/child_test_view.xml b/sample/src/main/res/layout/child_test_view.xml index 7f19cd0..7d83058 100644 --- a/sample/src/main/res/layout/child_test_view.xml +++ b/sample/src/main/res/layout/child_test_view.xml @@ -21,5 +21,7 @@ barber:testResourceId="@array/buzzwords" barber:testNonResString1="Hello" barber:testNonResString2="@string/testStringRes" - barber:coolNumber="3" + barber:childInt="3" + barber:childBoolean="true" + barber:childString="child" /> \ No newline at end of file diff --git a/sample/src/main/res/values/attrs.xml b/sample/src/main/res/values/attrs.xml index 0bcf4c9..91c2ba5 100644 --- a/sample/src/main/res/values/attrs.xml +++ b/sample/src/main/res/values/attrs.xml @@ -28,6 +28,8 @@ - + + + \ No newline at end of file From ca733175c1174a84d2f9fb59ea2492688d668d82 Mon Sep 17 00:00:00 2001 From: Henri Sweers Date: Wed, 4 Mar 2015 23:30:29 -0800 Subject: [PATCH 4/6] Only style parent if it hasn't been styled yet, otherwise style child This adds some checking and methods to do so. Essentially, each $$Barbershop instance has a WeakReference field that holds a reference to the last target styled (which should fall through from parent to children). If a style() method is called, it will only call the super's style method if the parent hasn't been styled yet, otherwise it proceeds as normal and updates the current last target styled with itself. This however isn't threadsafe --- .../java/io/sweers/barber/Barbershop.java | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/compiler/src/main/java/io/sweers/barber/Barbershop.java b/compiler/src/main/java/io/sweers/barber/Barbershop.java index 9192fd3..37adaaf 100644 --- a/compiler/src/main/java/io/sweers/barber/Barbershop.java +++ b/compiler/src/main/java/io/sweers/barber/Barbershop.java @@ -11,6 +11,7 @@ import com.squareup.javapoet.TypeVariableName; import java.io.IOException; +import java.lang.ref.WeakReference; import java.util.HashMap; import java.util.Map; @@ -57,7 +58,9 @@ public void writeToFiler(Filer filer) throws IOException { TypeSpec.Builder barberShop = TypeSpec.classBuilder(className) .addModifiers(Modifier.PUBLIC) .addTypeVariable(TypeVariableName.get("T", targetClassName)) - .addMethod(generateStyleMethod()); + .addMethod(generateStyleMethod()) + .addMethod(generateCheckParentMethod()) + .addField(ParameterizedTypeName.get(ClassName.get(WeakReference.class), TypeVariableName.get("T")), "lastStyledTarget"); if (parentBarbershop == null) { barberShop.addSuperinterface(ParameterizedTypeName.get(ClassName.get(Barber.IBarbershop.class), TypeVariableName.get("T"))); @@ -85,9 +88,15 @@ private MethodSpec generateStyleMethod() { .addParameter(int.class, "defStyleRes", Modifier.FINAL); if (parentBarbershop != null) { - builder.addStatement("super.style(target, set, attrs, defStyleAttr, defStyleRes)"); + builder.beginControlFlow("if (!super.hasStyled(target))") + .addStatement("super.style(target, set, attrs, defStyleAttr, defStyleRes)") + .addStatement("return") + .endControlFlow(); } + // Update our latest target + builder.addStatement("this.lastStyledTarget = new WeakReference<>(target)"); + // Don't do anything if there's no AttributeSet instance builder.beginControlFlow("if (set == null)") .addStatement("return") @@ -115,6 +124,20 @@ private MethodSpec generateStyleMethod() { return builder.build(); } + private MethodSpec generateCheckParentMethod() { + MethodSpec.Builder builder = MethodSpec.methodBuilder("hasStyled") + .returns(boolean.class) + .addModifiers(Modifier.PROTECTED) + .addParameter(TypeVariableName.get("T"), "target", Modifier.FINAL) + .addStatement("return this.lastStyledTarget != null && this.lastStyledTarget.get() != null && this.lastStyledTarget.get() == target"); + + if (parentBarbershop != null) { + builder.addAnnotation(Override.class); + } + + return builder.build(); + } + private String getFormattedStatementForBinding(FieldBinding binding) { String statement; if (binding.kind == STANDARD) { From 6f58123f1c075317624c71517bd5da0ac85b6433 Mon Sep 17 00:00:00 2001 From: Henri Sweers Date: Fri, 6 Mar 2015 15:49:37 -0800 Subject: [PATCH 5/6] Improve tests and add a grandchild --- .../io/sweers/barber/sample/BarberTest.java | 11 ++++-- .../barber/sample/GrandChildTestView.java | 37 +++++++++++++++++++ .../src/main/res/layout/child_test_view.xml | 1 - .../main/res/layout/grand_child_test_view.xml | 27 ++++++++++++++ sample/src/main/res/values/attrs.xml | 5 +++ 5 files changed, 76 insertions(+), 5 deletions(-) create mode 100644 sample/src/main/java/io/sweers/barber/sample/GrandChildTestView.java create mode 100644 sample/src/main/res/layout/grand_child_test_view.xml diff --git a/sample/src/androidTest/java/io/sweers/barber/sample/BarberTest.java b/sample/src/androidTest/java/io/sweers/barber/sample/BarberTest.java index 1b82d3c..b4baeaf 100644 --- a/sample/src/androidTest/java/io/sweers/barber/sample/BarberTest.java +++ b/sample/src/androidTest/java/io/sweers/barber/sample/BarberTest.java @@ -11,6 +11,7 @@ public class BarberTest extends AndroidTestCase { private TestView testView; private ChildTestView childTestView; + private GrandChildTestView grandChildTestView; private Resources res; @Override @@ -18,6 +19,7 @@ protected void setUp() throws Exception { super.setUp(); testView = (TestView) View.inflate(getContext(), R.layout.test_view, null); childTestView = (ChildTestView) View.inflate(getContext(), R.layout.child_test_view, null); + grandChildTestView = (GrandChildTestView) View.inflate(getContext(), R.layout.grand_child_test_view, null); res = getContext().getResources(); } @@ -118,10 +120,11 @@ public void testNonResString() { @SmallTest public void testInheritance() { assertTrue(childTestView.testBoolean); -// assertNull(childTestView.testString); + assertNull(childTestView.testString); assertEquals(3, childTestView.testInt); -// assertEquals(3, childTestView.childInt); -// assertTrue(childTestView.childBoolean); -// assertEquals("child", childTestView.childString); + assertEquals(3, childTestView.childInt); + assertTrue(childTestView.childBoolean); + assertEquals("child", childTestView.childString); + assertEquals("grandChild", grandChildTestView.grandChildString); } } diff --git a/sample/src/main/java/io/sweers/barber/sample/GrandChildTestView.java b/sample/src/main/java/io/sweers/barber/sample/GrandChildTestView.java new file mode 100644 index 0000000..69cf512 --- /dev/null +++ b/sample/src/main/java/io/sweers/barber/sample/GrandChildTestView.java @@ -0,0 +1,37 @@ +package io.sweers.barber.sample; + +import android.annotation.TargetApi; +import android.content.Context; +import android.os.Build; +import android.util.AttributeSet; + +import io.sweers.barber.Barber; +import io.sweers.barber.StyledAttr; + +/** + * Created by hsweers on 3/6/15. + */ +public class GrandChildTestView extends ChildTestView { + + @StyledAttr(R.styleable.GrandChildTestView_grandChildString) + public String grandChildString; + + public GrandChildTestView(Context context) { + super(context); + } + + public GrandChildTestView(Context context, AttributeSet attrs) { + this(context, attrs, 0); + } + + public GrandChildTestView(Context context, AttributeSet attrs, int defStyleAttr) { + super(context, attrs, defStyleAttr); + Barber.style(this, attrs, R.styleable.GrandChildTestView, defStyleAttr); + } + + @TargetApi(Build.VERSION_CODES.LOLLIPOP) + public GrandChildTestView(Context context, AttributeSet attrs, int defStyleAttr, int defStyleRes) { + super(context, attrs, defStyleAttr, defStyleRes); + Barber.style(this, attrs, R.styleable.GrandChildTestView, defStyleAttr, 0); + } +} diff --git a/sample/src/main/res/layout/child_test_view.xml b/sample/src/main/res/layout/child_test_view.xml index 7d83058..07f28f5 100644 --- a/sample/src/main/res/layout/child_test_view.xml +++ b/sample/src/main/res/layout/child_test_view.xml @@ -9,7 +9,6 @@ barber:testColor="@android:color/holo_red_dark" barber:testFloat="0.75" barber:testCharSequence="charsequence" - barber:testString="this is a string" barber:testTextArray="@array/buzzwords" barber:testColorStateList="@color/button_selector" barber:testDrawable="@drawable/ic_action_refresh" diff --git a/sample/src/main/res/layout/grand_child_test_view.xml b/sample/src/main/res/layout/grand_child_test_view.xml new file mode 100644 index 0000000..46476a1 --- /dev/null +++ b/sample/src/main/res/layout/grand_child_test_view.xml @@ -0,0 +1,27 @@ + + \ No newline at end of file diff --git a/sample/src/main/res/values/attrs.xml b/sample/src/main/res/values/attrs.xml index 91c2ba5..f80e3bd 100644 --- a/sample/src/main/res/values/attrs.xml +++ b/sample/src/main/res/values/attrs.xml @@ -32,4 +32,9 @@ + + + + + \ No newline at end of file From 239b95b92a42331f88e172b63e465a81283ef666 Mon Sep 17 00:00:00 2001 From: Henri Sweers Date: Fri, 6 Mar 2015 15:50:45 -0800 Subject: [PATCH 6/6] Keep a WeakHashSet of last styled refs This *should* make things threadsafe, as each reference can only check against parents using the same refs. WeakHashSet is nice because it also automatically prunes itself of stale references. --- .../java/io/sweers/barber/WeakHashSet.java | 193 ++++++++++++++++++ .../java/io/sweers/barber/Barbershop.java | 10 +- 2 files changed, 198 insertions(+), 5 deletions(-) create mode 100644 api/src/main/java/io/sweers/barber/WeakHashSet.java diff --git a/api/src/main/java/io/sweers/barber/WeakHashSet.java b/api/src/main/java/io/sweers/barber/WeakHashSet.java new file mode 100644 index 0000000..e07a96d --- /dev/null +++ b/api/src/main/java/io/sweers/barber/WeakHashSet.java @@ -0,0 +1,193 @@ +package io.sweers.barber; + +/* + * The contents of this file are subject to the terms + * of the Common Development and Distribution License + * (the "License"). You may not use this file except + * in compliance with the License. + * + * You can obtain a copy of the license at + * glassfish/bootstrap/legal/CDDLv1.0.txt or + * https://glassfish.dev.java.net/public/CDDLv1.0.html. + * See the License for the specific language governing + * permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL + * HEADER in each file and include the License file at + * glassfish/bootstrap/legal/CDDLv1.0.txt. If applicable, + * add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your + * own identifying information: Portions Copyright [yyyy] + * [name of copyright owner] + */ + +/* + * Copyright 2005 The Apache Software Foundation. + * + * 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. + */ + + +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; +import java.util.HashSet; +import java.util.Iterator; + +/** + * A weak HashSet. An element stored in the WeakHashSet might be + * garbage collected, if there is no strong reference to this element. + */ + +public class WeakHashSet extends HashSet { + /** + * Helps to detect garbage collected values. + */ + ReferenceQueue queue = new ReferenceQueue(); + + /** + * Returns an iterator over the elements in this set. The elements + * are returned in no particular order. + * + * @return an Iterator over the elements in this set. + */ + public Iterator iterator() { + // remove garbage collected elements + processQueue(); + + // get an iterator of the superclass WeakHashSet + final Iterator i = super.iterator(); + + return new Iterator() { + public boolean hasNext() { + return i.hasNext(); + } + + public Object next() { + // unwrap the element + return getReferenceObject((WeakReference) i.next()); + } + + public void remove() { + // remove the element from the HashSet + i.remove(); + } + }; + } + + /** + * Returns true if this set contains the specified element. + * + * @param o element whose presence in this set is to be tested. + * @return true if this set contains the specified element. + */ + public boolean contains(Object o) { + return super.contains(WeakElement.create(o)); + } + + /** + * Adds the specified element to this set if it is not already + * present. + * + * @param o element to be added to this set. + * @return true if the set did not already contain the specified + * element. + */ + public boolean add(Object o) { + processQueue(); + return super.add(WeakElement.create(o, this.queue)); + } + + /** + * Removes the given element from this set if it is present. + * + * @param o object to be removed from this set, if present. + * @return true if the set contained the specified element. + */ + public boolean remove(Object o) { + boolean ret = super.remove(WeakElement.create(o)); + processQueue(); + return ret; + } + + /** + * A convenience method to return the object held by the + * weak reference or null if it does not exist. + */ + private Object getReferenceObject(WeakReference ref) { + return (ref == null) ? null : ref.get(); + } + + /** + * Removes all garbage collected values with their keys from the map. + * Since we don't know how much the ReferenceQueue.poll() operation + * costs, we should call it only in the add() method. + */ + private void processQueue() { + WeakElement wv = null; + + while ((wv = (WeakElement) this.queue.poll()) != null) { + super.remove(wv); + } + } + + /** + * A WeakHashSet stores objects of class WeakElement. + * A WeakElement wraps the element that should be stored in the WeakHashSet. + * WeakElement inherits from java.lang.ref.WeakReference. + * It redefines equals and hashCode which delegate to the corresponding methods + * of the wrapped element. + */ + static private class WeakElement extends WeakReference { + private int hash; /* Hashcode of key, stored here since the key + may be tossed by the GC */ + + private WeakElement(Object o) { + super(o); + hash = o.hashCode(); + } + + private WeakElement(Object o, ReferenceQueue q) { + super(o, q); + hash = o.hashCode(); + } + + private static WeakElement create(Object o) { + return (o == null) ? null : new WeakElement(o); + } + + private static WeakElement create(Object o, ReferenceQueue q) { + return (o == null) ? null : new WeakElement(o, q); + } + + /* A WeakElement is equal to another WeakElement iff they both refer to objects + that are, in turn, equal according to their own equals methods */ + public boolean equals(Object o) { + if (this == o) + return true; + if (!(o instanceof WeakElement)) + return false; + Object t = this.get(); + Object u = ((WeakElement) o).get(); + if (t == u) + return true; + if ((t == null) || (u == null)) + return false; + return t.equals(u); + } + + public int hashCode() { + return hash; + } + } + +} \ No newline at end of file diff --git a/compiler/src/main/java/io/sweers/barber/Barbershop.java b/compiler/src/main/java/io/sweers/barber/Barbershop.java index 37adaaf..6c4494d 100644 --- a/compiler/src/main/java/io/sweers/barber/Barbershop.java +++ b/compiler/src/main/java/io/sweers/barber/Barbershop.java @@ -4,6 +4,7 @@ import android.util.AttributeSet; import com.squareup.javapoet.ClassName; +import com.squareup.javapoet.FieldSpec; import com.squareup.javapoet.JavaFile; import com.squareup.javapoet.MethodSpec; import com.squareup.javapoet.ParameterizedTypeName; @@ -11,7 +12,6 @@ import com.squareup.javapoet.TypeVariableName; import java.io.IOException; -import java.lang.ref.WeakReference; import java.util.HashMap; import java.util.Map; @@ -59,11 +59,11 @@ public void writeToFiler(Filer filer) throws IOException { .addModifiers(Modifier.PUBLIC) .addTypeVariable(TypeVariableName.get("T", targetClassName)) .addMethod(generateStyleMethod()) - .addMethod(generateCheckParentMethod()) - .addField(ParameterizedTypeName.get(ClassName.get(WeakReference.class), TypeVariableName.get("T")), "lastStyledTarget"); + .addMethod(generateCheckParentMethod()); if (parentBarbershop == null) { barberShop.addSuperinterface(ParameterizedTypeName.get(ClassName.get(Barber.IBarbershop.class), TypeVariableName.get("T"))); + barberShop.addField(FieldSpec.builder(WeakHashSet.class, "lastStyledTargets", Modifier.PROTECTED).initializer("new $T()", WeakHashSet.class).build()); } else { barberShop.superclass(ParameterizedTypeName.get(ClassName.bestGuess(parentBarbershop), TypeVariableName.get("T"))); } @@ -95,7 +95,7 @@ private MethodSpec generateStyleMethod() { } // Update our latest target - builder.addStatement("this.lastStyledTarget = new WeakReference<>(target)"); + builder.addStatement("this.lastStyledTargets.add(target)"); // Don't do anything if there's no AttributeSet instance builder.beginControlFlow("if (set == null)") @@ -129,7 +129,7 @@ private MethodSpec generateCheckParentMethod() { .returns(boolean.class) .addModifiers(Modifier.PROTECTED) .addParameter(TypeVariableName.get("T"), "target", Modifier.FINAL) - .addStatement("return this.lastStyledTarget != null && this.lastStyledTarget.get() != null && this.lastStyledTarget.get() == target"); + .addStatement("return this.lastStyledTargets.contains(target)"); if (parentBarbershop != null) { builder.addAnnotation(Override.class);