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

Fix superclass functional interface resolution in Painless #81698

Merged
merged 7 commits into from
Dec 14, 2021

Conversation

jdconrad
Copy link
Contributor

@jdconrad jdconrad commented Dec 13, 2021

A recent change [1] to how we load our allow list changed the resolution for how Painless looks up methods of super classes. However, functional interface loading was not changed which caused a bug where a functional interface would not look at its super interfaces for the functional interface method [2].

This fixes the issue by going through each super interface until the functional interface method is found when the target interface doesn't have the functional interface method.

[1] #76955
[2] #81696

@jdconrad jdconrad added >bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v8.0.0 v8.1.0 v7.16.2 labels Dec 13, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Dec 13, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine
Copy link
Collaborator

Hi @jdconrad, I've created a changelog YAML for you.

@elasticsearchmachine
Copy link
Collaborator

Hi @jdconrad, I've updated the changelog YAML for you.

@elasticsearchmachine
Copy link
Collaborator

Hi @jdconrad, I've updated the changelog YAML for you.

@jdconrad
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/docs-check

@jdconrad
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/part-1

@jdconrad jdconrad removed the >bug label Dec 14, 2021
@jdconrad
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/part-1

@jdconrad
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/docs-check

@elasticsearchmachine
Copy link
Collaborator

Hi @jdconrad, I've updated the changelog YAML for you.

@jdconrad jdconrad changed the title Fix subclass functional interface resolution in Painless Fix superclass functional interface resolution in Painless Dec 14, 2021
Copy link
Contributor

@stu-elastic stu-elastic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the following test?

    public void testUnaryOperator() {
        assertEquals("doremi", exec("List lst = ['abc', '123']; lst.replaceAll(f -> f.replace('abc', 'doremi')); lst.get(0);"));
        assertEquals("doremi", exec("def lst = ['abc', '123']; lst.replaceAll(f -> f.replace('abc', 'doremi')); lst.get(0);"));
    }

Can you thank @megglos and @TheFireCookie in the commit message?

We'll also have to write up a Known issues section in the 7.16 release notes

@jdconrad jdconrad added the auto-backport Automatically create backport pull requests when merged label Dec 14, 2021
@jdconrad jdconrad merged commit 8913b71 into elastic:master Dec 14, 2021
jdconrad added a commit to jdconrad/elasticsearch that referenced this pull request Dec 14, 2021
…1698)

A recent change [1] to how we load our allow list changed the resolution for how Painless looks up 
methods of super classes. However, functional interface loading was not changed which caused a 
bug where a functional interface would not look at its super interfaces for the functional interface 
method [2].

This fixes the issue by going through each super interface until the functional interface method is 
found when the target interface doesn't have the functional interface method.

[1] elastic#76955
[2] elastic#81696

Also a big thanks to @megglos and @TheFireCookie for their help with this issue.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.0
7.16

jdconrad added a commit to jdconrad/elasticsearch that referenced this pull request Dec 14, 2021
…1698)

A recent change [1] to how we load our allow list changed the resolution for how Painless looks up 
methods of super classes. However, functional interface loading was not changed which caused a 
bug where a functional interface would not look at its super interfaces for the functional interface 
method [2].

This fixes the issue by going through each super interface until the functional interface method is 
found when the target interface doesn't have the functional interface method.

[1] elastic#76955
[2] elastic#81696

Also a big thanks to @megglos and @TheFireCookie for their help with this issue.
jdconrad added a commit that referenced this pull request Dec 14, 2021
…81737)

A recent change [1] to how we load our allow list changed the resolution for how Painless looks up 
methods of super classes. However, functional interface loading was not changed which caused a 
bug where a functional interface would not look at its super interfaces for the functional interface 
method [2].

This fixes the issue by going through each super interface until the functional interface method is 
found when the target interface doesn't have the functional interface method.

[1] #76955
[2] #81696

Also a big thanks to @megglos and @TheFireCookie for their help with this issue.
jdconrad added a commit that referenced this pull request Dec 14, 2021
…81738)

A recent change [1] to how we load our allow list changed the resolution for how Painless looks up 
methods of super classes. However, functional interface loading was not changed which caused a 
bug where a functional interface would not look at its super interfaces for the functional interface 
method [2].

This fixes the issue by going through each super interface until the functional interface method is 
found when the target interface doesn't have the functional interface method.

[1] #76955
[2] #81696

Also a big thanks to @megglos and @TheFireCookie for their help with this issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >regression Team:Core/Infra Meta label for core/infra team v7.16.2 v8.0.0-rc1 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants