Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provider should have a filter method #19981

Closed
britter opened this issue Feb 21, 2022 · 11 comments · Fixed by #26067
Closed

Provider should have a filter method #19981

britter opened this issue Feb 21, 2022 · 11 comments · Fixed by #26067
Assignees
Labels
a:feature A new functionality in:provider-api property lazy provider MapProperty ListProperty DirectoryProperty
Milestone

Comments

@britter
Copy link
Member

britter commented Feb 21, 2022

Expected Behavior

When dealing with a provider, I sometimes want to only use it's value if it fits a certain criteria.

val someValue: Provider<Int> = project.provider { 5 }
val even = someValue.filter { it % 2 == 0 }
require(!even.isPresent)

Current Behavior

Feature is missing

Context

I was dealing with a Provider of a list of things and I only wanted to use it in case the list contains some elements and otherwise use getOrElse to provide some default value. This is currently not easily possible due to the missing filter method.

@britter britter added a:feature A new functionality to-triage in:provider-api property lazy provider MapProperty ListProperty DirectoryProperty labels Feb 21, 2022
@lptr
Copy link
Member

lptr commented Feb 21, 2022

This is one more case where Provider should work like Optional wherever possible.

@big-guy
Copy link
Member

big-guy commented Feb 21, 2022

What's the expectation here?

val someValue: Property<Int>
someValue.convention(5)
val even = someValue.filter { it % 2 == 0 }
// ... somewhere else
someValue.set(4)

Is even.isPresent true or false?

@britter
Copy link
Member Author

britter commented Feb 22, 2022

I think it should behave the same as

val someValue: Property<Int>
someValue.convention(5)
val even = someValue.map { it * 2 }
// ... somewhere else
someValue.set(4)

I don't know from the top of my head how this would behave. I think even will have the value 8, am I right?

@big-guy
Copy link
Member

big-guy commented Feb 23, 2022

Right, I could go along with that. In your example, someValue.get() would be 8.

I was asking because Optional.filter behaves eagerly, so in my example even.isPresent would be false if Provider.filter worked like Optional.filter.

You can get close to what you're looking for with map:

val someValue: Provider<Int> = project.provider { 5 }
val even = someValue.map { it % 2 == 0 }
require(!even.get())

Only even is a Provider<Boolean> and loses the value from someValue.

I think we'd need to think more about how a filter would fit into the chain of Provider/Property because I could see it being confusing with file inputs and knowing which dependencies we need to honor.

@bamboo
Copy link
Member

bamboo commented Feb 23, 2022

This is one more case where Provider should work like Optional wherever possible.

I assume you mean the APIs should be similar and not that providers should be eager, right?

@bamboo
Copy link
Member

bamboo commented Feb 23, 2022

I think we'd need to think more about how a filter would fit into the chain of Provider/Property because I could see it being confusing with file inputs and knowing which dependencies we need to honor.

That doesn't sound dissimilar to the situation with orElse, particularly when the configuration cache is involved as we need to defer evaluation as much as possible.

There were talks of introducing a sort of conditional dependency (a task / work dependency that would only be activated at the last possible moment when a particular condition holds).

@lptr
Copy link
Member

lptr commented Feb 23, 2022

This is one more case where Provider should work like Optional wherever possible.

I assume you mean the APIs should be similar and not that providers should be eager, right?

Yes.

@big-guy
Copy link
Member

big-guy commented Feb 24, 2022

That doesn't sound dissimilar to the situation with orElse, particularly when the configuration cache is involved as we need to defer evaluation as much as possible.

Yes, but the difference here is that you can stop once you have a producer with a value.

The dependency comes from someFile or anotherFile, depending on which one is set:

inputFile = someFile.orElse(anotherFile)

The dependency needs to be... both?

inputFile = someFile.filter { it.exists() }.orElse(anotherFile)

Maybe it's a source of footguns to have the presence of a Provider be based on the value of the Provider.

I'm trying to think of a good use case where this would be helpful...

Say you have three locations for a configuration file:

  • gradle.properties
  • gradle/gradle.properties
  • $HOME/gradle.properties

You could have a property (pseudo):

myExt {
    propertiesFile.convention(layout.projectDirectory.file("gradle.properties"))
}

But this is missing the fallback to the other locations.

So you could do...

myExt {
    propertiesFile.convention(provider { 
        // look for each location and return the one that exists
    })
}

Or with filter:

myExt {
    propertiesFile.convention(
        layout.projectDirectory.file("gradle.properties").filter({ it.exists() })
        .orElse(layout.projectDirectory.file("gradle/gradle.properties")).filter({ it.exists() })
        .orElse(homeDir.file("gradle.properties"))
    )
}

I think the simplest thing is actually:

myExt {
    // look for each location and use the one that exists
    propertiesFile.convention(locationFound)
}

But you need to somehow record that the non-existence of some files is an input then.

@wolfs
Copy link
Member

wolfs commented Mar 14, 2022

Having a filter method would also be one way to solve: #12388

I think it would be a better way than to use .map { null }.

@cloudshiftchris
Copy link

Using this extension function in the absence of native support:

public inline fun <reified S> Provider<S>.filter(crossinline predicate: (S) -> Boolean): Provider<S> {
    return flatMap {
        if (predicate(it)) {
            this@filter
        } else {
            Providers.notDefined()
        }
    }
}

@jbartok jbartok added the p:lazy-migration Issues covered by migration to an all-lazy API label May 30, 2023
@lptr
Copy link
Member

lptr commented May 30, 2023

FTR, once we migrate more to providers I expect more code that requires filtering like this, so delivering this feature is kind of a pre-requisite of the migration.

@lptr lptr removed the p:lazy-migration Issues covered by migration to an all-lazy API label Jul 10, 2023
@ov7a ov7a self-assigned this Aug 9, 2023
ov7a added a commit that referenced this issue Aug 10, 2023
ov7a added a commit that referenced this issue Aug 10, 2023
ov7a added a commit that referenced this issue Aug 17, 2023
bot-gradle added a commit that referenced this issue Aug 23, 2023
Fixes #19981

### Reviewing cheatsheet

Before merging the PR, comments starting with
- ❌ ❓**must** be fixed
- 🤔 💅 **should** be fixed
- 💭 **may** be fixed
- 🎉 celebrate happy things

Co-authored-by: Vlad Chesnokov <vchesnokov@gradle.com>
ov7a added a commit that referenced this issue Aug 23, 2023
bot-gradle added a commit that referenced this issue Aug 23, 2023
Fixes #19981

### Reviewing cheatsheet

Before merging the PR, comments starting with
- ❌ ❓**must** be fixed
- 🤔 💅 **should** be fixed
- 💭 **may** be fixed
- 🎉 celebrate happy things

Co-authored-by: Vlad Chesnokov <vchesnokov@gradle.com>
bot-gradle added a commit that referenced this issue Aug 23, 2023
Fixes #19981

### Reviewing cheatsheet

Before merging the PR, comments starting with
- ❌ ❓**must** be fixed
- 🤔 💅 **should** be fixed
- 💭 **may** be fixed
- 🎉 celebrate happy things

Co-authored-by: Vlad Chesnokov <vchesnokov@gradle.com>
bot-gradle added a commit that referenced this issue Aug 24, 2023
Fixes #19981

### Reviewing cheatsheet

Before merging the PR, comments starting with
- ❌ ❓**must** be fixed
- 🤔 💅 **should** be fixed
- 💭 **may** be fixed
- 🎉 celebrate happy things

Co-authored-by: Vlad Chesnokov <vchesnokov@gradle.com>
@bot-gradle bot-gradle added this to the 8.4 RC1 milestone Aug 25, 2023
bot-gradle added a commit that referenced this issue Oct 26, 2023
…d of Predicate

### Context
It was missed during the [original PR](#26067) for
* #19981

Reported by @lacasseio from the Nokee project.

### Reviewing cheatsheet

Before merging the PR, comments starting with
- ❌ ❓**must** be fixed
- 🤔 💅 **should** be fixed
- 💭 **may** be fixed
- 🎉 celebrate happy things

Co-authored-by: Vlad Chesnokov <vchesnokov@gradle.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:feature A new functionality in:provider-api property lazy provider MapProperty ListProperty DirectoryProperty
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants