Improved attribute matching for values sourced from a module metadata…

… file, to allow the consumer to use enum types or `Named` types.

subprojects/core/src/main/java/org/gradle/api/internal/attributes/DefaultImmutableAttributesFactory.java

@@ -18,6 +18,7 @@
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import org.gradle.api.attributes.Attribute;
+import org.gradle.api.internal.changedetection.state.isolation.Isolatable;
 import org.gradle.api.internal.changedetection.state.isolation.IsolatableFactory;
 
 import java.util.ArrayList;
@@ -56,20 +57,27 @@ public <T> ImmutableAttributes of(Attribute<T> key, T value) {
     }
 
     @Override
-    public synchronized <T> ImmutableAttributes concat(ImmutableAttributes node, Attribute<T> key, T value) {
-        List<DefaultImmutableAttributes> nodeChildren = children.get(node);
-        if (nodeChildren == null) {
-            nodeChildren = Lists.newArrayList();
-            children.put(node, nodeChildren);
-        }
-        for (DefaultImmutableAttributes child : nodeChildren) {
-            if (child.attribute.equals(key) && child.value.isolate().equals(value)) {
-                return child;
+    public <T> ImmutableAttributes concat(ImmutableAttributes node, Attribute<T> key, T value) {
+        return concat(node, key, isolatableFactory.isolate(value));
+    }
+
+    @Override
+    public <T> ImmutableAttributes concat(ImmutableAttributes node, Attribute<T> key, Isolatable<T> value) {
+        synchronized (this) {
+            List<DefaultImmutableAttributes> nodeChildren = children.get(node);
+            if (nodeChildren == null) {
+                nodeChildren = Lists.newArrayList();
+                children.put(node, nodeChildren);
+            }
+            for (DefaultImmutableAttributes child : nodeChildren) {
+                if (child.attribute.equals(key) && child.value.equals(value)) {
+                    return child;
+                }
             }
+            DefaultImmutableAttributes child = new DefaultImmutableAttributes((DefaultImmutableAttributes) node, key, value);
+            nodeChildren.add(child);
+            return child;
         }
-        DefaultImmutableAttributes child = new DefaultImmutableAttributes((DefaultImmutableAttributes) node, key, isolatableFactory.isolate(value));
-        nodeChildren.add(child);
-        return child;
     }
 
     public ImmutableAttributes getRoot() {

subprojects/core/src/main/java/org/gradle/api/internal/attributes/ImmutableAttributesFactory.java

@@ -16,6 +16,7 @@
 package org.gradle.api.internal.attributes;
 
 import org.gradle.api.attributes.Attribute;
+import org.gradle.api.internal.changedetection.state.isolation.Isolatable;
 
 public interface ImmutableAttributesFactory {
     /**
@@ -38,6 +39,11 @@ public interface ImmutableAttributesFactory {
      */
     <T> ImmutableAttributes concat(ImmutableAttributes node, Attribute<T> key, T value);
 
+    /**
+     * Adds the given attribute to the given container. Note: the container _should not_ contain the given attribute.
+     */
+    <T> ImmutableAttributes concat(ImmutableAttributes node, Attribute<T> key, Isolatable<T> value);
+
     /**
      * Merges the second container into the first container and returns the result. Values in the second container win.
      */

subprojects/core/src/main/java/org/gradle/api/internal/changedetection/state/CoercingStringValueSnapshot.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright 2017 the original author or 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 org.gradle.api.internal.changedetection.state;
+
+import org.gradle.api.Named;
+import org.gradle.api.internal.changedetection.state.isolation.Isolatable;
+import org.gradle.api.internal.changedetection.state.isolation.IsolatableEnumValueSnapshot;
+import org.gradle.api.internal.model.NamedObjectInstantiator;
+import org.gradle.internal.Cast;
+
+import javax.annotation.Nullable;
+
+public class CoercingStringValueSnapshot extends StringValueSnapshot {
+    private final String value;
+    private final NamedObjectInstantiator instantiator;
+
+    public CoercingStringValueSnapshot(String value, NamedObjectInstantiator instantiator) {
+        super(value);
+        this.value = value;
+        this.instantiator = instantiator;
+    }
+
+    @Nullable
+    @Override
+    public <S> Isolatable<S> coerce(Class<S> type) {
+        if (type.isInstance(value)) {
+            return Cast.uncheckedCast(this);
+        }
+        if (type.isEnum()) {
+            return Cast.uncheckedCast(new IsolatableEnumValueSnapshot(Enum.valueOf(type.asSubclass(Enum.class), getValue())));
+        }
+        if (Named.class.isAssignableFrom(type)) {
+            return Cast.uncheckedCast(new IsolatedManagedNamedTypeSnapshot(instantiator.named((Class<? extends Named>) type, getValue()), instantiator));
+        }
+        return null;
+    }
+}

subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/maven/MavenDependencyWithGradleMetadataResolutionIntegrationTest.groovy

@@ -18,6 +18,7 @@ package org.gradle.integtests.resolve.maven
 
 import org.gradle.integtests.fixtures.AbstractHttpDependencyResolutionTest
 import org.gradle.integtests.fixtures.resolve.ResolveTestFixture
+import spock.lang.Unroll
 
 class MavenDependencyWithGradleMetadataResolutionIntegrationTest extends AbstractHttpDependencyResolutionTest {
     def resolve = new ResolveTestFixture(buildFile)
@@ -159,7 +160,7 @@ dependencies {
         }
     }
 
-    def "uses runtime dependencies and files from selected variant"() {
+    def "uses runtime dependencies from pom and files from selected variant"() {
         def b = mavenHttpRepo.module("test", "b", "2.0").publish()
         def a = mavenHttpRepo.module("test", "a", "1.2")
             .dependsOn(b)
@@ -412,6 +413,87 @@ task checkRelease {
         succeeds("checkRelease")
     }
 
+    @Unroll
+    def "consumer can use attribute of type - #type"() {
+        def a = mavenHttpRepo.module("test", "a", "1.2")
+            .withModuleMetadata()
+        a.artifact(classifier: 'debug')
+        a.publish()
+        a.moduleMetadata.file.text = """
+{
+    "formatVersion": "0.1",
+    "variants": [
+        {
+            "name": "debug",
+            "attributes": {
+                "buildType": "debug"
+            },
+            "files": [ 
+                { "name": "a-1.2-debug.jar", "url": "a-1.2-debug.jar" }
+            ]
+        },
+        {
+            "name": "release",
+            "attributes": {
+                "buildType": "release"
+            }
+        }
+    ]
+}
+"""
+
+        given:
+        settingsFile << "rootProject.name = 'test'"
+        buildFile << """
+repositories {
+    maven { 
+        url = '${mavenHttpRepo.uri}' 
+        useGradleMetadata() // internal opt-in for now
+    }
+}
+
+enum BuildTypeEnum {
+    debug, release
+}
+interface BuildType extends Named {
+}
+
+def attr = Attribute.of("buildType", ${type})
+configurations { 
+    debug { attributes.attribute(attr, ${debugValue}) }
+    release { attributes.attribute(attr, ${releaseValue}) }
+}
+dependencies {
+    debug 'test:a:1.2'
+    release 'test:a:1.2'
+}
+task checkDebug {
+    doLast { assert configurations.debug.files*.name == ['a-1.2-debug.jar'] }
+}
+task checkRelease {
+    doLast { assert configurations.release.files*.name == [] }
+}
+"""
+
+        a.pom.expectGet()
+        a.moduleMetadata.expectGet()
+        a.artifact(classifier: 'debug').expectGet()
+
+        expect:
+        succeeds("checkDebug")
+
+        and:
+        server.resetExpectations()
+
+        and:
+        succeeds("checkRelease")
+
+        where:
+        type            | debugValue                          | releaseValue
+        "BuildTypeEnum" | "BuildTypeEnum.debug"               | "BuildTypeEnum.release"
+        "BuildType"     | "objects.named(BuildType, 'debug')" | "objects.named(BuildType, 'release')"
+    }
+
     def "reports failure to locate module"() {
         def m = mavenHttpRepo.module("test", "a", "1.2")
 

subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/DefaultDependencyManagementServices.java

@@ -72,6 +72,7 @@
 import org.gradle.api.internal.file.FileCollectionFactory;
 import org.gradle.api.internal.file.FileResolver;
 import org.gradle.api.internal.filestore.ivy.ArtifactIdentifierFileStore;
+import org.gradle.api.internal.model.NamedObjectInstantiator;
 import org.gradle.api.internal.project.ProjectInternal;
 import org.gradle.api.internal.tasks.TaskResolver;
 import org.gradle.initialization.ProjectAccessListener;
@@ -145,7 +146,7 @@ BaseRepositoryFactory createBaseRepositoryFactory(LocalMavenRepositoryLocator lo
                 artifactIdentifierFileStore,
                 externalResourceFileStore,
                 new GradlePomModuleDescriptorParser(versionSelectorScheme, moduleIdentifierFactory, fileResourceRepository),
-                new ModuleMetadataParser(attributesFactory),
+                new ModuleMetadataParser(attributesFactory, NamedObjectInstantiator.INSTANCE),
                 authenticationSchemeRegistry,
                 ivyContextManager,
                 moduleIdentifierFactory,

subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/DependencyManagementBuildScopeServices.java

@@ -69,6 +69,7 @@
 import org.gradle.api.internal.file.TmpDirTemporaryFileProvider;
 import org.gradle.api.internal.file.collections.DirectoryFileTreeFactory;
 import org.gradle.api.internal.filestore.ivy.ArtifactIdentifierFileStore;
+import org.gradle.api.internal.model.NamedObjectInstantiator;
 import org.gradle.api.internal.notations.ClientModuleNotationParserFactory;
 import org.gradle.api.internal.notations.DependencyNotationParser;
 import org.gradle.api.internal.notations.ProjectDependencyFactory;
@@ -188,7 +189,8 @@ ModuleMetaDataCache createModuleDescriptorCache(BuildCommencedTimeProvider timeP
             cacheLockingManager,
             artifactCacheMetaData,
             moduleIdentifierFactory,
-            attributesFactory);
+            attributesFactory,
+            NamedObjectInstantiator.INSTANCE);
     }
 
     ArtifactAtRepositoryCachedArtifactIndex createArtifactAtRepositoryCachedResolutionIndex(BuildCommencedTimeProvider timeProvider, CacheLockingManager cacheLockingManager) {

subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/parser/ModuleMetadataParser.java

@@ -22,6 +22,8 @@
 import org.gradle.api.attributes.Attribute;
 import org.gradle.api.internal.attributes.ImmutableAttributes;
 import org.gradle.api.internal.attributes.ImmutableAttributesFactory;
+import org.gradle.api.internal.changedetection.state.CoercingStringValueSnapshot;
+import org.gradle.api.internal.model.NamedObjectInstantiator;
 import org.gradle.internal.component.external.model.MutableComponentVariant;
 import org.gradle.internal.component.external.model.MutableComponentVariantResolveMetadata;
 import org.gradle.internal.resource.local.LocallyAvailableExternalResource;
@@ -39,9 +41,11 @@
 public class ModuleMetadataParser {
     public static final String FORMAT_VERSION = "0.1";
     private final ImmutableAttributesFactory attributesFactory;
+    private final NamedObjectInstantiator instantiator;
 
-    public ModuleMetadataParser(ImmutableAttributesFactory attributesFactory) {
+    public ModuleMetadataParser(ImmutableAttributesFactory attributesFactory, NamedObjectInstantiator instantiator) {
         this.attributesFactory = attributesFactory;
+        this.instantiator = instantiator;
     }
 
     public void parse(final LocallyAvailableExternalResource resource, final MutableComponentVariantResolveMetadata metadata) {
@@ -148,7 +152,7 @@ private ImmutableAttributes consumeAttributes(JsonReader reader) throws IOExcept
         while(reader.peek()!= END_OBJECT) {
             String attrName = reader.nextName();
             String attrValue = reader.nextString();
-            attributes = attributesFactory.concat(attributes, Attribute.of(attrName, String.class), attrValue);
+            attributes = attributesFactory.concat(attributes, Attribute.of(attrName, String.class), new CoercingStringValueSnapshot(attrValue, instantiator));
         }
         reader.endObject();
         return attributes;

subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/modulecache/DefaultModuleMetaDataCache.java

@@ -23,6 +23,7 @@
 import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.ModuleComponentRepository;
 import org.gradle.api.internal.artifacts.ivyservice.resolveengine.result.ComponentIdentifierSerializer;
 import org.gradle.api.internal.attributes.ImmutableAttributesFactory;
+import org.gradle.api.internal.model.NamedObjectInstantiator;
 import org.gradle.cache.PersistentIndexedCache;
 import org.gradle.internal.Factory;
 import org.gradle.internal.component.external.model.ModuleComponentResolveMetadata;
@@ -44,11 +45,11 @@ public class DefaultModuleMetaDataCache implements ModuleMetaDataCache {
     private final ModuleMetadataStore moduleMetadataStore;
     private PersistentIndexedCache<ModuleComponentAtRepositoryKey, ModuleMetadataCacheEntry> cache;
 
-    public DefaultModuleMetaDataCache(BuildCommencedTimeProvider timeProvider, CacheLockingManager cacheLockingManager, ArtifactCacheMetaData artifactCacheMetaData, ImmutableModuleIdentifierFactory moduleIdentifierFactory, ImmutableAttributesFactory attributesFactory) {
+    public DefaultModuleMetaDataCache(BuildCommencedTimeProvider timeProvider, CacheLockingManager cacheLockingManager, ArtifactCacheMetaData artifactCacheMetaData, ImmutableModuleIdentifierFactory moduleIdentifierFactory, ImmutableAttributesFactory attributesFactory, NamedObjectInstantiator instantiator) {
         this.timeProvider = timeProvider;
         this.cacheLockingManager = cacheLockingManager;
 
-        moduleMetadataStore = new ModuleMetadataStore(new DefaultPathKeyFileStore(artifactCacheMetaData.getMetaDataStoreDirectory()), new ModuleMetadataSerializer(attributesFactory), moduleIdentifierFactory);
+        moduleMetadataStore = new ModuleMetadataStore(new DefaultPathKeyFileStore(artifactCacheMetaData.getMetaDataStoreDirectory()), new ModuleMetadataSerializer(attributesFactory, instantiator), moduleIdentifierFactory);
     }
 
     private PersistentIndexedCache<ModuleComponentAtRepositoryKey, ModuleMetadataCacheEntry> getCache() {

subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/modulecache/ModuleMetadataSerializer.java

@@ -28,6 +28,8 @@
 import org.gradle.api.internal.artifacts.ivyservice.NamespaceId;
 import org.gradle.api.internal.attributes.ImmutableAttributes;
 import org.gradle.api.internal.attributes.ImmutableAttributesFactory;
+import org.gradle.api.internal.changedetection.state.CoercingStringValueSnapshot;
+import org.gradle.api.internal.model.NamedObjectInstantiator;
 import org.gradle.internal.component.external.descriptor.Artifact;
 import org.gradle.internal.component.external.descriptor.Configuration;
 import org.gradle.internal.component.external.descriptor.DefaultExclude;
@@ -66,13 +68,15 @@ public class ModuleMetadataSerializer {
     private static final byte TYPE_IVY = 1;
     private static final byte TYPE_MAVEN = 2;
     private final ImmutableAttributesFactory attributesFactory;
+    private final NamedObjectInstantiator instantiator;
 
-    public ModuleMetadataSerializer(ImmutableAttributesFactory attributesFactory) {
+    public ModuleMetadataSerializer(ImmutableAttributesFactory attributesFactory, NamedObjectInstantiator instantiator) {
         this.attributesFactory = attributesFactory;
+        this.instantiator = instantiator;
     }
 
     public MutableModuleComponentResolveMetadata read(Decoder decoder, ImmutableModuleIdentifierFactory moduleIdentifierFactory) throws IOException {
-        return new Reader(decoder, moduleIdentifierFactory, attributesFactory).read();
+        return new Reader(decoder, moduleIdentifierFactory, attributesFactory, instantiator).read();
     }
 
     public void write(Encoder encoder, ModuleComponentResolveMetadata metadata) throws IOException {
@@ -297,13 +301,15 @@ private static class Reader {
         private final Decoder decoder;
         private final ImmutableModuleIdentifierFactory moduleIdentifierFactory;
         private final ImmutableAttributesFactory attributesFactory;
+        private final NamedObjectInstantiator instantiator;
         private ModuleComponentIdentifier id;
         private ModuleVersionIdentifier mvi;
 
-        private Reader(Decoder decoder, ImmutableModuleIdentifierFactory moduleIdentifierFactory, ImmutableAttributesFactory attributesFactory) {
+        private Reader(Decoder decoder, ImmutableModuleIdentifierFactory moduleIdentifierFactory, ImmutableAttributesFactory attributesFactory, NamedObjectInstantiator instantiator) {
             this.decoder = decoder;
             this.moduleIdentifierFactory = moduleIdentifierFactory;
             this.attributesFactory = attributesFactory;
+            this.instantiator = instantiator;
         }
 
         public MutableModuleComponentResolveMetadata read() throws IOException {
@@ -352,7 +358,7 @@ private ImmutableAttributes readAttributes() throws IOException {
             for (int i = 0; i < count; i++) {
                 String name = decoder.readString();
                 String value = decoder.readString();
-                attributes = attributesFactory.concat(attributes, Attribute.of(name, String.class), value);
+                attributes = attributesFactory.concat(attributes, Attribute.of(name, String.class), new CoercingStringValueSnapshot(value, instantiator));
             }
             return attributes;
         }

subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/parser/ModuleMetadataParserTest.groovy

@@ -19,14 +19,15 @@ package org.gradle.api.internal.artifacts.ivyservice.ivyresolve.parser
 import org.gradle.api.Transformer
 import org.gradle.api.attributes.Attribute
 import org.gradle.api.internal.attributes.ImmutableAttributes
+import org.gradle.api.internal.model.NamedObjectInstantiator
 import org.gradle.internal.component.external.model.MutableComponentVariant
 import org.gradle.internal.component.external.model.MutableComponentVariantResolveMetadata
 import org.gradle.internal.resource.local.LocallyAvailableExternalResource
 import org.gradle.util.TestUtil
 import spock.lang.Specification
 
 class ModuleMetadataParserTest extends Specification {
-    def parser = new ModuleMetadataParser(TestUtil.attributesFactory())
+    def parser = new ModuleMetadataParser(TestUtil.attributesFactory(), NamedObjectInstantiator.INSTANCE)
 
     def "parses minimal metadata resource"() {
         def metadata = Mock(MutableComponentVariantResolveMetadata)