Collapse deferred-binding property values that depend on collapsed property values. Patch by: bobv Review by: rjrjr Review at http://gwt-code-reviews.appspot.com/1525804 git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10565 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/cfg/BindingProperty.java b/dev/core/src/com/google/gwt/dev/cfg/BindingProperty.java index 36cb777..039bcc2 100644 --- a/dev/core/src/com/google/gwt/dev/cfg/BindingProperty.java +++ b/dev/core/src/com/google/gwt/dev/cfg/BindingProperty.java
@@ -31,10 +31,10 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; -import java.util.Map.Entry; import java.util.regex.Pattern; /** @@ -48,13 +48,15 @@ private static final String EMPTY = ""; private List<SortedSet<String>> collapsedValues = Lists.create(); - private final Map<Condition, SortedSet<String>> conditionalValues = new LinkedHashMap<Condition, SortedSet<String>>(); + private final Map<Condition, SortedSet<String>> conditionalValues = + new LinkedHashMap<Condition, SortedSet<String>>(); private final SortedSet<String> definedValues = new TreeSet<String>(); private PropertyProvider provider; private Class<? extends PropertyProviderGenerator> providerGenerator; private String fallback; - private HashMap<String,LinkedList<LinkedHashSet<String>>> fallbackValueMap; - private HashMap<String,LinkedList<String>> fallbackValues = new HashMap<String,LinkedList<String>>(); + private HashMap<String, LinkedList<LinkedHashSet<String>>> fallbackValueMap; + private HashMap<String, LinkedList<String>> fallbackValues = + new HashMap<String, LinkedList<String>>(); private final ConditionAll rootCondition = new ConditionAll(); { @@ -77,8 +79,7 @@ // Expanded in normalizeCollapsedValues() continue; } else if (!definedValues.contains(value)) { - throw new IllegalArgumentException( - "Attempting to collapse unknown value " + value); + throw new IllegalArgumentException("Attempting to collapse unknown value " + value); } } @@ -99,7 +100,8 @@ } /** - * Adds fall back value to given property name. + * Adds fall back value to given property name. + * * @param value the property value. * @param fallbackValue the fall back value for given property value. */ @@ -164,13 +166,15 @@ } /** - * Returns the map of values to fall back values. the list of fall - * back values is in decreasing order of preference. + * Returns the map of values to fall back values. the list of fall back values + * is in decreasing order of preference. + * * @return map of property value to fall back values. */ - public Map<String,? extends List<? extends Set<String>>> getFallbackValuesMap() { + public Map<String, ? extends List<? extends Set<String>>> getFallbackValuesMap() { if (fallbackValueMap == null) { - HashMap<String,LinkedList<LinkedHashSet<String>>> valuesMap = new HashMap<String,LinkedList<LinkedHashSet<String>>>(); + HashMap<String, LinkedList<LinkedHashSet<String>>> valuesMap = + new HashMap<String, LinkedList<LinkedHashSet<String>>>(); // compute closure of fall back values preserving order for (Entry<String, LinkedList<String>> e : fallbackValues.entrySet()) { String from = e.getKey(); @@ -179,8 +183,7 @@ LinkedList<String> childList = fallbackValues.get(from); LinkedHashSet<String> children = new LinkedHashSet<String>(); children.addAll(childList); - while (children != null && children.size() > 0) - { + while (children != null && children.size() > 0) { alternates.add(children); LinkedHashSet<String> newChildren = new LinkedHashSet<String>(); for (String child : children) { @@ -199,7 +202,7 @@ } return fallbackValueMap; } - + public PropertyProvider getProvider() { return provider; } @@ -262,7 +265,7 @@ * the currently-defined values. * * @throws IllegalArgumentException if any of the provided values were not - * provided to {@link #addDefinedValue(Condition,String)}. + * provided to {@link #addDefinedValue(Condition,String)}. */ public void setAllowedValues(Condition condition, String... values) { SortedSet<String> temp = new TreeSet<String>(Arrays.asList(values)); @@ -308,6 +311,29 @@ * Create a minimal number of equivalence sets, expanding any glob patterns. */ void normalizeCollapsedValues() { + /* + * Properties that depend upon a collapsed property value must be recombined + * into an equivalence set. + */ + if (!getRequiredProperties().isEmpty()) { + // This defines additional equivalence sets + Map<String, SortedSet<String>> requiredPropertyNamesToDependentValues = + new HashMap<String, SortedSet<String>>(); + for (Map.Entry<Condition, SortedSet<String>> entry : getConditionalValues().entrySet()) { + for (String requiredPropertyName : entry.getKey().getRequiredProperties()) { + SortedSet<String> set = requiredPropertyNamesToDependentValues.get(requiredPropertyName); + if (set == null) { + set = new TreeSet<String>(); + requiredPropertyNamesToDependentValues.put(requiredPropertyName, set); + } + set.addAll(entry.getValue()); + } + } + for (SortedSet<String> valuesToCollapse : requiredPropertyNamesToDependentValues.values()) { + addCollapsedValues(valuesToCollapse.toArray(new String[valuesToCollapse.size()])); + } + } + if (collapsedValues.isEmpty()) { return; } @@ -368,8 +394,8 @@ } // The values of the maps will now contain the minimal number of sets - collapsedValues = new ArrayList<SortedSet<String>>( - new IdentityHashSet<SortedSet<String>>(map.values())); + collapsedValues = + new ArrayList<SortedSet<String>>(new IdentityHashSet<SortedSet<String>>(map.values())); // Sort the list Lists.sort(collapsedValues, new Comparator<SortedSet<String>>() {
diff --git a/dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java b/dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java index 48dfef1..58a4018 100644 --- a/dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java +++ b/dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java
@@ -61,12 +61,12 @@ * We delete items from this set, but want to retain the original order as * much as possible. */ - Set<BindingProperty> bindingProps = new LinkedHashSet<BindingProperty>( - properties.getBindingProperties()); + Set<BindingProperty> bindingProps = + new LinkedHashSet<BindingProperty>(properties.getBindingProperties()); // Accumulates the order in which the properties should be evaluated - Map<String, BindingProperty> evaluationOrder = new LinkedHashMap<String, BindingProperty>( - bindingProps.size()); + Map<String, BindingProperty> evaluationOrder = + new LinkedHashMap<String, BindingProperty>(bindingProps.size()); /* * Insert a property after all of the properties that it depends upon have @@ -87,19 +87,16 @@ } if (!changed) { - throw new IllegalStateException( - "Cycle detected within remaining property dependencies " - + bindingProps.toString()); + throw new IllegalStateException("Cycle detected within remaining property dependencies " + + bindingProps.toString()); } } - return evaluationOrder.values().toArray( - new BindingProperty[evaluationOrder.size()]); + return evaluationOrder.values().toArray(new BindingProperty[evaluationOrder.size()]); } - private static void permute(BindingProperty[] properties, - Set<String> activeLinkerNames, String[] soFar, int whichProp, - List<String[]> permutations) { + private static void permute(BindingProperty[] properties, Set<String> activeLinkerNames, + String[] soFar, int whichProp, List<String[]> permutations) { int lastProp = properties.length - 1; BindingProperty prop = properties[whichProp]; @@ -111,18 +108,17 @@ } else { BindingProperty[] answerable = new BindingProperty[soFar.length]; System.arraycopy(properties, 0, answerable, 0, soFar.length); - PropertyOracle propertyOracle = new StaticPropertyOracle(answerable, - soFar, new ConfigurationProperty[0]); + PropertyOracle propertyOracle = + new StaticPropertyOracle(answerable, soFar, new ConfigurationProperty[0]); for (Condition cond : prop.getConditionalValues().keySet()) { try { - if (cond.isTrue(TreeLogger.NULL, new DeferredBindingQuery( - propertyOracle, activeLinkerNames))) { + if (cond.isTrue(TreeLogger.NULL, new DeferredBindingQuery(propertyOracle, + activeLinkerNames))) { winner = cond; } } catch (UnableToCompleteException e) { - throw new IllegalStateException( - "Should never get here for simple properties", e); + throw new IllegalStateException("Should never get here for simple properties", e); } } } @@ -140,8 +136,7 @@ nextStep[whichProp] = knownValue; if (whichProp < lastProp) { - permute(properties, activeLinkerNames, nextStep, whichProp + 1, - permutations); + permute(properties, activeLinkerNames, nextStep, whichProp + 1, permutations); } else { // Finished this permutation. permutations.add(nextStep); @@ -152,14 +147,12 @@ private final Properties properties; private final List<String[]> values; - public PropertyPermutations(Properties properties, - Set<String> activeLinkerNames) { + public PropertyPermutations(Properties properties, Set<String> activeLinkerNames) { this.properties = properties; this.values = allPermutationsOf(properties, activeLinkerNames); } - public PropertyPermutations(PropertyPermutations allPermutations, - int firstPerm, int numPerms) { + public PropertyPermutations(PropertyPermutations allPermutations, int firstPerm, int numPerms) { this.properties = allPermutations.properties; values = allPermutations.values.subList(firstPerm, firstPerm + numPerms); } @@ -167,8 +160,7 @@ /** * Copy constructor that allows the list of property values to be reset. */ - public PropertyPermutations(PropertyPermutations allPermutations, - List<String[]> values) { + public PropertyPermutations(PropertyPermutations allPermutations, List<String[]> values) { this.properties = allPermutations.properties; this.values = values; } @@ -180,17 +172,18 @@ */ public List<PropertyPermutations> collapseProperties() { // Collate property values in this map - SortedMap<CollapsedPropertyKey, List<String[]>> map = new TreeMap<CollapsedPropertyKey, List<String[]>>(); + SortedMap<CollapsedPropertyKey, List<String[]>> map = + new TreeMap<CollapsedPropertyKey, List<String[]>>(); // Loop over all possible property value permutations for (Iterator<String[]> it = iterator(); it.hasNext();) { String[] propertyValues = it.next(); assert propertyValues.length == getOrderedProperties().length; - StaticPropertyOracle oracle = new StaticPropertyOracle( - getOrderedProperties(), propertyValues, new ConfigurationProperty[0]); - CollapsedPropertyKey key = new CollapsedPropertyKey( - oracle); + StaticPropertyOracle oracle = + new StaticPropertyOracle(getOrderedProperties(), propertyValues, + new ConfigurationProperty[0]); + CollapsedPropertyKey key = new CollapsedPropertyKey(oracle); List<String[]> list = map.get(key); if (list == null) { @@ -201,8 +194,7 @@ } // Return the collated values - List<PropertyPermutations> toReturn = new ArrayList<PropertyPermutations>( - map.size()); + List<PropertyPermutations> toReturn = new ArrayList<PropertyPermutations>(map.size()); for (List<String[]> list : map.values()) { toReturn.add(new PropertyPermutations(this, list)); } @@ -230,4 +222,34 @@ public int size() { return values.size(); } + + /** + * For debugging use only. + */ + @Override + public String toString() { + StringBuilder sb = new StringBuilder(); + sb.append("{"); + boolean needsComma = false; + for (String[] strings : values) { + if (needsComma) { + sb.append(", "); + } else { + needsComma = true; + } + sb.append("["); + boolean innerNeedsComma = false; + for (String string : strings) { + if (innerNeedsComma) { + sb.append(", "); + } else { + innerNeedsComma = true; + } + sb.append(string); + } + sb.append("]"); + } + sb.append("}"); + return sb.toString(); + } }
diff --git a/dev/core/test/com/google/gwt/dev/cfg/ModuleDefTest.java b/dev/core/test/com/google/gwt/dev/cfg/ModuleDefTest.java index 9827bd4..8fcf69a 100644 --- a/dev/core/test/com/google/gwt/dev/cfg/ModuleDefTest.java +++ b/dev/core/test/com/google/gwt/dev/cfg/ModuleDefTest.java
@@ -21,13 +21,14 @@ import com.google.gwt.core.ext.UnableToCompleteException; import com.google.gwt.core.ext.linker.ArtifactSet; import com.google.gwt.core.ext.linker.LinkerOrder; -import com.google.gwt.core.ext.linker.Shardable; import com.google.gwt.core.ext.linker.LinkerOrder.Order; +import com.google.gwt.core.ext.linker.Shardable; import junit.framework.TestCase; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.SortedSet; @@ -44,14 +45,13 @@ } @Override - public ArtifactSet link(TreeLogger logger, LinkerContext context, - ArtifactSet artifacts, boolean onePermutation) { + public ArtifactSet link(TreeLogger logger, LinkerContext context, ArtifactSet artifacts, + boolean onePermutation) { return null; } @Override - public ArtifactSet relink(TreeLogger logger, LinkerContext context, - ArtifactSet newArtifacts) { + public ArtifactSet relink(TreeLogger logger, LinkerContext context, ArtifactSet newArtifacts) { return null; } } @@ -120,14 +120,10 @@ List<SortedSet<String>> collapsedValues = b.getCollapsedValues(); assertEquals(4, collapsedValues.size()); - assertEquals(Arrays.asList("a", "b"), new ArrayList<String>( - collapsedValues.get(0))); - assertEquals(Arrays.asList("c", "d", "e"), new ArrayList<String>( - collapsedValues.get(1))); - assertEquals(Arrays.asList("f1", "f2", "f3"), new ArrayList<String>( - collapsedValues.get(2))); - assertEquals(Arrays.asList("g1a", "g2a"), new ArrayList<String>( - collapsedValues.get(3))); + assertEquals(Arrays.asList("a", "b"), new ArrayList<String>(collapsedValues.get(0))); + assertEquals(Arrays.asList("c", "d", "e"), new ArrayList<String>(collapsedValues.get(1))); + assertEquals(Arrays.asList("f1", "f2", "f3"), new ArrayList<String>(collapsedValues.get(2))); + assertEquals(Arrays.asList("g1a", "g2a"), new ArrayList<String>(collapsedValues.get(3))); // Collapse everything b.addCollapsedValues("*"); @@ -135,8 +131,79 @@ collapsedValues = b.getCollapsedValues(); assertEquals(1, collapsedValues.size()); - assertEquals(Arrays.asList(b.getDefinedValues()), new ArrayList<String>( - collapsedValues.get(0))); + assertEquals(Arrays.asList(b.getDefinedValues()), new ArrayList<String>(collapsedValues.get(0))); + } + + /** + * This tests the behavior of properties that depend on other properties with + * collapsed values. A dependency on a collapsed value forms an equivalent set + * for the dependent values. In other words, collapsing is contagious. + */ + public void testCollapsedPropertiesWithDependencies() { + ModuleDef def = new ModuleDef("fake"); + Properties p = def.getProperties(); + // <define-property name="p1" values="a,b,c" /> + BindingProperty p1 = p.createBinding("p1"); + p1.addDefinedValue(p1.getRootCondition(), "a"); + p1.addDefinedValue(p1.getRootCondition(), "b"); + p1.addDefinedValue(p1.getRootCondition(), "c"); + // <collapse-property name="p1" values="b,c" /> + p1.addCollapsedValues("b", "c"); + + BindingProperty p2 = p.createBinding("p2"); + // <define-property name="p2" values="d,e,f" /> + p2.addDefinedValue(p2.getRootCondition(), "d"); + p2.addDefinedValue(p2.getRootCondition(), "e"); + p2.addDefinedValue(p2.getRootCondition(), "f"); + /*- + * <set-property name="p2" values="e,f"> + * <when-property-is name="p1" value="c" /> + * </set-property> + */ + p2.setAllowedValues(new ConditionWhenPropertyIs("p1", "c"), "e", "f"); + + /* + * The net effect of the above is to delete the [c,d] permutation and to + * make the e and f values dependent on the c value. Because c is a + * collapsed value, e and f are then automatically collapsed together when + * p2 is normalized. + */ + + p1.normalizeCollapsedValues(); + p2.normalizeCollapsedValues(); + + PropertyPermutations perms = new PropertyPermutations(p, Collections.<String> emptySet()); + List<PropertyPermutations> hardPerms = perms.collapseProperties(); + + assertEquals(4, hardPerms.size()); + { + // {[a,d]} + PropertyPermutations perm = hardPerms.get(0); + assertEquals(1, perm.size()); + assertEquals(Arrays.asList("a", "d"), Arrays.asList(perm.getOrderedPropertyValues(0))); + } + { + // {[a,e], [a,f]} + PropertyPermutations perm = hardPerms.get(1); + assertEquals(2, perm.size()); + assertEquals(Arrays.asList("a", "e"), Arrays.asList(perm.getOrderedPropertyValues(0))); + assertEquals(Arrays.asList("a", "f"), Arrays.asList(perm.getOrderedPropertyValues(1))); + } + { + // {[b,d]} + PropertyPermutations perm = hardPerms.get(2); + assertEquals(1, perm.size()); + assertEquals(Arrays.asList("b", "d"), Arrays.asList(perm.getOrderedPropertyValues(0))); + } + { + // {[b,e], [b,f], [c,e], [c,f]} + PropertyPermutations perm = hardPerms.get(3); + assertEquals(4, perm.size()); + assertEquals(Arrays.asList("b", "e"), Arrays.asList(perm.getOrderedPropertyValues(0))); + assertEquals(Arrays.asList("b", "f"), Arrays.asList(perm.getOrderedPropertyValues(1))); + assertEquals(Arrays.asList("c", "e"), Arrays.asList(perm.getOrderedPropertyValues(2))); + assertEquals(Arrays.asList("c", "f"), Arrays.asList(perm.getOrderedPropertyValues(3))); + } } public void testLinkerOrder() throws UnableToCompleteException { @@ -154,13 +221,14 @@ def.addLinker("post2"); def.addLinker("primary"); - Class<?>[] expectedClasses = { - FakeLinkerPre2.class, FakeLinkerPre.class, FakeLinkerPost.class, - FakeLinkerPost2.class, FakeLinkerPrimary.class}; + Class<?>[] expectedClasses = + { + FakeLinkerPre2.class, FakeLinkerPre.class, FakeLinkerPost.class, FakeLinkerPost2.class, + FakeLinkerPrimary.class}; assertEquals(FakeLinkerPrimary.class, def.getActivePrimaryLinker()); // Test iteration order - assertEquals(Arrays.asList(expectedClasses), - new ArrayList<Class<? extends Linker>>(def.getActiveLinkers())); + assertEquals(Arrays.asList(expectedClasses), new ArrayList<Class<? extends Linker>>(def + .getActiveLinkers())); } public void testLinkerRedefinition() throws UnableToCompleteException { @@ -179,12 +247,12 @@ // Intentional duplication def.addLinker("post"); - Class<?>[] expectedClasses = { - FakeLinkerPre2.class, FakeLinkerPost2.class, FakeLinkerPrimary2.class}; + Class<?>[] expectedClasses = + {FakeLinkerPre2.class, FakeLinkerPost2.class, FakeLinkerPrimary2.class}; assertEquals(FakeLinkerPrimary2.class, def.getActivePrimaryLinker()); // Test iteration order - assertEquals(Arrays.asList(expectedClasses), - new ArrayList<Class<? extends Linker>>(def.getActiveLinkers())); + assertEquals(Arrays.asList(expectedClasses), new ArrayList<Class<? extends Linker>>(def + .getActiveLinkers())); } public void testLinkerRedefinitionErrors() throws UnableToCompleteException { @@ -230,12 +298,12 @@ def.addLinker("primary"); def.addLinker("primary2"); - Class<?>[] expectedClasses = { - FakeLinkerPre.class, FakeLinkerPost.class, FakeLinkerPrimary2.class}; + Class<?>[] expectedClasses = + {FakeLinkerPre.class, FakeLinkerPost.class, FakeLinkerPrimary2.class}; assertEquals(FakeLinkerPrimary2.class, def.getActivePrimaryLinker()); // Test iteration order - assertEquals(Arrays.asList(expectedClasses), - new ArrayList<Class<? extends Linker>>(def.getActiveLinkers())); + assertEquals(Arrays.asList(expectedClasses), new ArrayList<Class<? extends Linker>>(def + .getActiveLinkers())); } public void testValidModuleName() { @@ -244,15 +312,15 @@ assertFalse(ModuleDef.isValidModuleName("com..Foo")); assertFalse(ModuleDef.isValidModuleName("com.7.Foo")); assertFalse(ModuleDef.isValidModuleName("com.7foo.Foo")); - + assertTrue(ModuleDef.isValidModuleName("com.foo.Foo")); assertTrue(ModuleDef.isValidModuleName("com.$foo.Foo")); assertTrue(ModuleDef.isValidModuleName("com._foo.Foo")); assertTrue(ModuleDef.isValidModuleName("com.foo7.Foo")); - - // For legacy reasons, allow the last part of the name is not - // required to be a valid ident. In the past, naming rules - // were enforced for top level modules, but not nested modules. + + // For legacy reasons, allow the last part of the name is not + // required to be a valid ident. In the past, naming rules + // were enforced for top level modules, but not nested modules. assertTrue(ModuleDef.isValidModuleName("com.foo.F-oo")); assertTrue(ModuleDef.isValidModuleName("com.foo.7Foo")); assertTrue(ModuleDef.isValidModuleName("com.foo.+Foo"));