Cleanup realization of pending provider

[lacasseio]

It avoids concurrent modification exception where a provider tries to add
to the store while the store is busy realizing the provider. To work around this, we use the doAddRealized internal method.

The PR also clean up the realization of a provider by introducing the realizePending(ProviderInternal) method to the store. The domain object collections now use this method instead of relying on the side effect of Provider#get().

Context

See gradle/gradle-native#709

Gradle Core Team Checklist

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation
• Recognize contributor in release notes

[adammurdoch]

It's not clear why we would make this change. What's the use case? The description says "a provider tries to add to the store while the store is busy realizing the provider" but doesn't specify whether this is a user specified provider added using addLater() or addAllLater() or a provider that is implicitly added by register(). The description also doesn't specify what code is mutating the container. Is it user code or Gradle code? In the addLater() case, is it the user code that calculates the provider's value? or is it some configuration action attached to the container? In the register() case, is it some configuration action attached to the container?

I think a functional test that shows the use case would be useful.

[lacasseio]

The issue surface when we want to generalize the domain object collection tests. First, we are hitting the issue fixed by #6617. Then, only certain container are hitting the failure as the underlying store behave differently between collection. The store IterationOrderRetainingSetElementSource and ListElementSource are both affected. Unfortunately, there isn't a way to write a functional test as everything that is created by Gradle's public API is of type NamedDomainObjectSet and his children which use SortedSetElementSource as the store. The issue appears when we want to add generalize test which includes all collections. We could not target the collection behaving differently or we could fix the issue.

What is happening is we are adding a new Element to the store while we are iterating the inserted list. We can change to use an index based iteration which works around the issue for the concurrent modification. However, in some case, the current behavior of AbstractNamedDomainObjectCreatingProvider will duplicate the element inside the store because we are explicitly calling add onto the container itself which, again, behave differently between stores. In the case of the specific store, the add actually add an element to the store without many checks. Duplicating elements while we realize them is an unwanted behavior. We are left to how we actually do the realization.

The simplest solution I found, instead of reversing the ownership like I was working previously, is to complete the adding (doAddRealized) instead of adding the element itself (doAdd). Again, the various store behaves differently which cause issues. On top of that, we have two paths for realizing an element: 1) initiated by the store, when adding an all configuration action for example or 2) initiated by the provider when calling get(). Those two paths cause issues for correctly detecting when an object is removed, defining what configuration actions should be called after creation and collecting any failures that may happen during realization.

I don't believe it's exactly a use case by just the internal implementation not allowing all collection to behave the same. I can't add a test to the store to show this issue as we aren't changing the store implementation, they are correctly implemented. We are changing the way we use the store from within the collection. The way we are using it just happens to make everything behave the same throughout the collections. I hope this explains it a bit better.