diff --git a/platforms/core-configuration/file-collections/src/integTest/groovy/org/gradle/api/file/FileCollectionLifecycleIntegrationTest.groovy b/platforms/core-configuration/file-collections/src/integTest/groovy/org/gradle/api/file/FileCollectionLifecycleIntegrationTest.groovy index 82a5a87448f..262b52fa9e3 100644 --- a/platforms/core-configuration/file-collections/src/integTest/groovy/org/gradle/api/file/FileCollectionLifecycleIntegrationTest.groovy +++ b/platforms/core-configuration/file-collections/src/integTest/groovy/org/gradle/api/file/FileCollectionLifecycleIntegrationTest.groovy @@ -140,7 +140,7 @@ class FileCollectionLifecycleIntegrationTest extends AbstractIntegrationSpec imp fails('broken') then: - failure.assertHasCause("The value for this file collection is final and cannot be changed.") + failure.assertHasCause("The value for this file collection is final and cannot be changed any further.") } def "can disallow changes to file collection without finalizing value"() { @@ -165,7 +165,7 @@ class FileCollectionLifecycleIntegrationTest extends AbstractIntegrationSpec imp fails('broken') then: - failure.assertHasCause("The value for this file collection cannot be changed.") + failure.assertHasCause("The value for this file collection cannot be changed any further.") } def "can write but cannot read strict project file collection instance before project configuration completes"() { @@ -233,11 +233,11 @@ class FileCollectionLifecycleIntegrationTest extends AbstractIntegrationSpec imp run("show") then: - outputContains("get files failed with: Cannot query the value for this file collection because configuration of root project 'broken' has not completed yet.") - outputContains("get elements failed with: Cannot query the value for this file collection because configuration of root project 'broken' has not completed yet.") - outputContains("get files in afterEvaluate failed with: Cannot query the value for this file collection because configuration of root project 'broken' has not completed yet.") - outputContains("get elements in afterEvaluate failed with: Cannot query the value for this file collection because configuration of root project 'broken' has not completed yet.") - outputContains("set after read failed with: The value for this file collection is final and cannot be changed.") + outputContains("get files failed with: Cannot query the value of this file collection because configuration of root project 'broken' has not completed yet.") + outputContains("get elements failed with: Cannot query the value of this file collection because configuration of root project 'broken' has not completed yet.") + outputContains("get files in afterEvaluate failed with: Cannot query the value of this file collection because configuration of root project 'broken' has not completed yet.") + outputContains("get elements in afterEvaluate failed with: Cannot query the value of this file collection because configuration of root project 'broken' has not completed yet.") + outputContains("set after read failed with: The value for this file collection is final and cannot be changed any further.") output.count("value = [${file('some-file-2')}]") == 2 output.count("elements = [${file('some-file-2')}]") == 2 } @@ -274,7 +274,7 @@ class FileCollectionLifecycleIntegrationTest extends AbstractIntegrationSpec imp run("show") then: - outputContains("set failed with: The value for this file collection is final and cannot be changed.") + outputContains("set failed with: The value for this file collection is final and cannot be changed any further.") output.count("value = [${file('some-file')}]") == 2 } @@ -313,7 +313,7 @@ class FileCollectionLifecycleIntegrationTest extends AbstractIntegrationSpec imp run("show") then: - outputContains("finalize failed with: Cannot finalize the value for this file collection because configuration of root project 'broken' has not completed yet.") + outputContains("finalize failed with: Cannot finalize the value of this file collection because configuration of root project 'broken' has not completed yet.") output.count("value = [${file('some-file')}]") == 2 } diff --git a/platforms/core-configuration/file-collections/src/main/java/org/gradle/api/internal/file/collections/DefaultConfigurableFileCollection.java b/platforms/core-configuration/file-collections/src/main/java/org/gradle/api/internal/file/collections/DefaultConfigurableFileCollection.java index aaec6600ea4..9ec6c1230dd 100644 --- a/platforms/core-configuration/file-collections/src/main/java/org/gradle/api/internal/file/collections/DefaultConfigurableFileCollection.java +++ b/platforms/core-configuration/file-collections/src/main/java/org/gradle/api/internal/file/collections/DefaultConfigurableFileCollection.java @@ -27,6 +27,8 @@ import org.gradle.api.internal.file.UnionFileCollection; import org.gradle.api.internal.provider.HasConfigurableValueInternal; import org.gradle.api.internal.provider.PropertyHost; +import org.gradle.api.internal.provider.ValueState; +import org.gradle.api.internal.provider.ValueSupplier; import org.gradle.api.internal.provider.support.LazyGroovySupport; import org.gradle.api.internal.tasks.DefaultTaskDependency; import org.gradle.api.internal.tasks.TaskDependencyFactory; @@ -55,22 +57,15 @@ * A {@link org.gradle.api.file.FileCollection} which resolves a set of paths relative to a {@link org.gradle.api.internal.file.FileResolver}. */ public class DefaultConfigurableFileCollection extends CompositeFileCollection implements ConfigurableFileCollection, Managed, HasConfigurableValueInternal, LazyGroovySupport { - public static final EmptyCollector EMPTY_COLLECTOR = new EmptyCollector(); - - private enum State { - Mutable, ImplicitFinalizeNextQuery, FinalizeNextQuery, Final - } - + private static final EmptyCollector EMPTY_COLLECTOR = new EmptyCollector(); private final PathSet filesWrapper; private final String displayName; private final PathToFileResolver resolver; private final TaskDependencyFactory dependencyFactory; private final PropertyHost host; private final DefaultTaskDependency buildDependency; - private State state = State.Mutable; - private boolean disallowChanges; - private boolean disallowUnsafeRead; private ValueCollector value = EMPTY_COLLECTOR; + private ValueState valueState; public DefaultConfigurableFileCollection(@Nullable String displayName, PathToFileResolver fileResolver, TaskDependencyFactory dependencyFactory, Factory patternSetFactory, PropertyHost host) { super(dependencyFactory, patternSetFactory); @@ -78,6 +73,7 @@ public DefaultConfigurableFileCollection(@Nullable String displayName, PathToFil this.resolver = fileResolver; this.dependencyFactory = dependencyFactory; this.host = host; + this.valueState = ValueState.newState(host); filesWrapper = new PathSet(); buildDependency = dependencyFactory.configurableDependency(); } @@ -99,46 +95,40 @@ public Object unpackState() { @Override public void finalizeValue() { - if (state == State.Final) { - return; - } - if (disallowUnsafeRead) { - String reason = host.beforeRead(null); - if (reason != null) { - throw new IllegalStateException("Cannot finalize the value for " + displayNameForThisCollection() + " because " + reason + "."); - } + if (valueState.shouldFinalize(this::displayNameForThisCollection, null)) { + finalizeNow(); } + } + + private void finalizeNow() { calculateFinalizedValue(); - state = State.Final; - disallowChanges = true; + valueState = valueState.finalState(); } public boolean isFinalizing() { - return state != State.Mutable; + return valueState.isFinalizing(); } @Override public void disallowChanges() { - disallowChanges = true; + valueState.disallowChanges(); } @Override public void finalizeValueOnRead() { - if (state == State.Mutable || state == State.ImplicitFinalizeNextQuery) { - state = State.FinalizeNextQuery; - } + valueState.finalizeOnNextGet(); } @Override public void implicitFinalizeValue() { - if (state == State.Mutable) { - state = State.ImplicitFinalizeNextQuery; - } + // Property prevents reads *and* mutations, + // however CFCs only want automatic finalization on query, + // so we do not #disallowChanges(). + valueState.finalizeOnNextGet(); } public void disallowUnsafeRead() { - disallowUnsafeRead = true; - finalizeValueOnRead(); + valueState.disallowUnsafeRead(); } public int getFactoryId() { @@ -255,13 +245,7 @@ public FileCollectionInternal replace(FileCollectionInternal original, Supplier< } private void assertMutable() { - if (state == State.Final && disallowChanges) { - throw new IllegalStateException("The value for " + displayNameForThisCollection() + " is final and cannot be changed."); - } else if (disallowChanges) { - throw new IllegalStateException("The value for " + displayNameForThisCollection() + " cannot be changed."); - } else if (state == State.Final) { - throw new IllegalStateException("The value for " + displayNameForThisCollection() + " is final and cannot be changed."); - } + valueState.beforeMutate(this::displayNameForThisCollection); } private String displayNameForThisCollection() { @@ -311,20 +295,7 @@ public void visitFileTreeBackedByFile(File file, FileTreeInternal fileTree, File @Override protected void visitChildren(Consumer visitor) { - if (disallowUnsafeRead && state != State.Final) { - String reason = host.beforeRead(null); - if (reason != null) { - throw new IllegalStateException("Cannot query the value for " + displayNameForThisCollection() + " because " + reason + "."); - } - } - if (state == State.ImplicitFinalizeNextQuery) { - calculateFinalizedValue(); - state = State.Final; - } else if (state == State.FinalizeNextQuery) { - calculateFinalizedValue(); - state = State.Final; - disallowChanges = true; - } + valueState.finalizeOnReadIfNeeded(this::displayNameForThisCollection, null, ValueSupplier.ValueConsumer.IgnoreUnsafeRead, unused -> finalizeNow()); value.visitContents(visitor); } diff --git a/platforms/core-configuration/file-collections/src/test/groovy/org/gradle/api/internal/file/collections/DefaultConfigurableFileCollectionSpec.groovy b/platforms/core-configuration/file-collections/src/test/groovy/org/gradle/api/internal/file/collections/DefaultConfigurableFileCollectionSpec.groovy index b2f61af50f3..538c8b7dec4 100644 --- a/platforms/core-configuration/file-collections/src/test/groovy/org/gradle/api/internal/file/collections/DefaultConfigurableFileCollectionSpec.groovy +++ b/platforms/core-configuration/file-collections/src/test/groovy/org/gradle/api/internal/file/collections/DefaultConfigurableFileCollectionSpec.groovy @@ -598,21 +598,21 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == 'The value for is final and cannot be changed.' + e.message == 'The value for is final and cannot be changed any further.' when: collection.setFrom() then: def e2 = thrown(IllegalStateException) - e2.message == 'The value for is final and cannot be changed.' + e2.message == 'The value for is final and cannot be changed any further.' when: collection.setFrom(['some', 'more']) then: def e3 = thrown(IllegalStateException) - e3.message == 'The value for is final and cannot be changed.' + e3.message == 'The value for is final and cannot be changed any further.' } def "cannot mutate from set when finalized"() { @@ -627,28 +627,28 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == 'The value for is final and cannot be changed.' + e.message == 'The value for is final and cannot be changed any further.' when: collection.from.add('b') then: def e2 = thrown(IllegalStateException) - e2.message == 'The value for is final and cannot be changed.' + e2.message == 'The value for is final and cannot be changed any further.' when: collection.from.remove('a') then: def e3 = thrown(IllegalStateException) - e3.message == 'The value for is final and cannot be changed.' + e3.message == 'The value for is final and cannot be changed any further.' when: collection.from.iterator().remove() then: def e4 = thrown(IllegalStateException) - e4.message == 'The value for is final and cannot be changed.' + e4.message == 'The value for is final and cannot be changed any further.' } def "cannot add paths when finalized"() { @@ -663,7 +663,7 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == 'The value for is final and cannot be changed.' + e.message == 'The value for is final and cannot be changed any further.' } def "resolves path to file when queried after implicitly finalized"() { @@ -1112,14 +1112,14 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == 'The value for is final and cannot be changed.' + e.message == 'The value for is final and cannot be changed any further.' when: collection.setFrom(['some', 'more']) then: def e2 = thrown(IllegalStateException) - e2.message == 'The value for is final and cannot be changed.' + e2.message == 'The value for is final and cannot be changed any further.' } def "cannot add paths when queried after finalize on read"() { @@ -1135,14 +1135,14 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == 'The value for is final and cannot be changed.' + e.message == 'The value for is final and cannot be changed any further.' when: collection.from() then: def e2 = thrown(IllegalStateException) - e2.message == 'The value for is final and cannot be changed.' + e2.message == 'The value for is final and cannot be changed any further.' } def "cannot mutate from set when queried after finalize on read"() { @@ -1158,28 +1158,28 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == 'The value for is final and cannot be changed.' + e.message == 'The value for is final and cannot be changed any further.' when: collection.from.add('b') then: def e2 = thrown(IllegalStateException) - e2.message == 'The value for is final and cannot be changed.' + e2.message == 'The value for is final and cannot be changed any further.' when: collection.from.remove('a') then: def e3 = thrown(IllegalStateException) - e3.message == 'The value for is final and cannot be changed.' + e3.message == 'The value for is final and cannot be changed any further.' when: collection.from.iterator().remove() then: def e4 = thrown(IllegalStateException) - e4.message == 'The value for is final and cannot be changed.' + e4.message == 'The value for is final and cannot be changed any further.' } def "cannot specify paths when changes disallowed"() { @@ -1193,14 +1193,14 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == 'The value for cannot be changed.' + e.message == 'The value for cannot be changed any further.' when: collection.setFrom(['some', 'more']) then: def e2 = thrown(IllegalStateException) - e2.message == 'The value for cannot be changed.' + e2.message == 'The value for cannot be changed any further.' } def "cannot mutate from set when changes disallowed"() { @@ -1214,28 +1214,28 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == 'The value for cannot be changed.' + e.message == 'The value for cannot be changed any further.' when: collection.from.add('b') then: def e2 = thrown(IllegalStateException) - e2.message == 'The value for cannot be changed.' + e2.message == 'The value for cannot be changed any further.' when: collection.from.remove('a') then: def e3 = thrown(IllegalStateException) - e3.message == 'The value for cannot be changed.' + e3.message == 'The value for cannot be changed any further.' when: collection.from.iterator().remove() then: def e4 = thrown(IllegalStateException) - e4.message == 'The value for cannot be changed.' + e4.message == 'The value for cannot be changed any further.' } def "cannot add paths when changes disallowed"() { @@ -1249,7 +1249,7 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == 'The value for cannot be changed.' + e.message == 'The value for cannot be changed any further.' } def "cannot specify paths when changes disallowed and implicitly finalized"() { @@ -1264,14 +1264,14 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == 'The value for cannot be changed.' + e.message == 'The value for cannot be changed any further.' when: collection.setFrom(['some', 'more']) then: def e2 = thrown(IllegalStateException) - e2.message == 'The value for cannot be changed.' + e2.message == 'The value for cannot be changed any further.' } def "resolves closure to files when changes disallowed"() { @@ -1395,7 +1395,7 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == "Cannot query the value for because ." + e.message == "Cannot query the value of because ." and: 1 * host.beforeRead(null) >> "" @@ -1430,7 +1430,7 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == "Cannot query the value for because ." + e.message == "Cannot query the value of because ." and: 1 * host.beforeRead(null) >> "" @@ -1459,7 +1459,7 @@ class DefaultConfigurableFileCollectionSpec extends FileCollectionSpec { then: def e = thrown(IllegalStateException) - e.message == "Cannot finalize the value for because ." + e.message == "Cannot finalize the value of because ." and: 1 * host.beforeRead(null) >> ""