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.

Sign up for GitHub

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

Jump to bottom

Allow enum members to define functions #22281

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Allow enum members to define functions #22281

wants to merge 2 commits into from

Conversation

mwashburn
Copy link

Enums which have method implementations cannot be used in queries.

Fixes: #21800
Partially Fixes #8305

Breaking changes (list specific methods/types/messages):
Queries that used to throw exceptions will now work.

@mwashburn mwashburn requested a review from a team as a code owner September 20, 2022 18:50
@hz-devops-test hz-devops-test added the Source: Community PR or issue was opened by a community user label Sep 20, 2022
@devOpsHazelcast
Copy link
Collaborator

devOpsHazelcast commented Sep 20, 2022 •
edited

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@TomaszGaweda
Copy link
Contributor

Thanks for your contribution! Can you please add tests covering the fix?

@hazelcast hazelcast deleted a comment from devOpsHazelcast Sep 21, 2022
@hazelcast hazelcast deleted a comment from devOpsHazelcast Sep 21, 2022
@hazelcast hazelcast deleted a comment from devOpsHazelcast Sep 21, 2022
@hazelcast hazelcast deleted a comment from devOpsHazelcast Sep 21, 2022
@hazelcast hazelcast deleted a comment from devOpsHazelcast Sep 21, 2022
@hazelcast hazelcast deleted a comment from devOpsHazelcast Sep 21, 2022
@hazelcast hazelcast deleted a comment from devOpsHazelcast Sep 21, 2022
@hazelcast hazelcast deleted a comment from devOpsHazelcast Sep 21, 2022
@hazelcast hazelcast deleted a comment from devOpsHazelcast Sep 21, 2022
@hazelcast hazelcast deleted a comment from devOpsHazelcast Sep 21, 2022
@hazelcast hazelcast deleted a comment from devOpsHazelcast Sep 21, 2022
@hazelcast hazelcast deleted a comment from devOpsHazelcast Sep 21, 2022
@TomaszGaweda
Copy link
Contributor

(btw. it's too late for this to be in 5.2, maybe it will make it to 5.3 :) )

@mwashburn
Copy link
Author

tests added.

@TomaszGaweda
Copy link
Contributor

LGTM, Asked for additional review from Core & SQL team members

@vbekiaris
Copy link
Collaborator

run-lab-run

@hz-devops-test
Copy link

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file 
--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Errors: 
[ERROR]   ProcessorClassLoaderTest.setUp:113->createHazelcastMember:139 ? Guardian java....
[ERROR]   ProcessorClassLoaderTest.setUp:113->createHazelcastMember:139 ? Guardian java....
[ERROR]   ProcessorClassLoaderTest.setUp:113->createHazelcastMember:139 ? Guardian java....
[ERROR]   CacheConfigConstructorTest.testConstructor:44 ? Guardian java.lang.NoSuchMetho...
[ERROR]   ConfigConstructorTest.testConstructor:50 ? Guardian java.lang.NoSuchMethodExce...
[ERROR]   DynamicConfigurationAwareConfigConstructorTest.testConstructor:57 ? Guardian j...
[INFO] 
[ERROR] Tests run: 50917, Failures: 0, Errors: 6, Skipped: 238
[INFO] 

[ERROR] There are test failures.

@vbekiaris
Copy link
Collaborator

run-lab-run

@hz-devops-test
Copy link

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file 
--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Errors: 
[ERROR]   ProcessorClassLoaderTest.setUp:113->createHazelcastMember:139 ? Guardian java....
[ERROR]   ProcessorClassLoaderTest.setUp:113->createHazelcastMember:139 ? Guardian java....
[ERROR]   ProcessorClassLoaderTest.setUp:113->createHazelcastMember:139 ? Guardian java....
[ERROR]   CacheConfigConstructorTest.testConstructor:44 ? Guardian java.lang.NoSuchMetho...
[ERROR]   ConfigConstructorTest.testConstructor:50 ? Guardian java.lang.NoSuchMethodExce...
[ERROR]   DynamicConfigurationAwareConfigConstructorTest.testConstructor:57 ? Guardian j...
[INFO] 
[ERROR] Tests run: 50926, Failures: 0, Errors: 6, Skipped: 238
[INFO] 

[ERROR] There are test failures.

@vbekiaris
Copy link
Collaborator

run-lab-run

vbekiaris
vbekiaris approved these changes Nov 15, 2022
View reviewed changes
Copy link
Collaborator

@vbekiaris vbekiaris left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the awesome fix @mwashburn , added one minor comment.

hazelcast/src/test/java/com/hazelcast/query/impl/getters/MethodGetterTest.java
@@ -97,6 +97,17 @@ public void constructor_whenModifierIsPositionAndMethodReturnTypeIsCollection_th
new MethodGetter(null, limbCollectionMethod, "[0]", null);
}

@Test
public void getValue_whenFieldIsEnum()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain the motivation for this test? It is not failing with current master and it does not test MethodGetter with enum elements, but maybe I 'm missing something?

@hz-devops-test
Copy link

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file 
--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   CacheClientListenerTest>CacheListenerTest.testSyncListener:110 Cache operations have not finished in 130 seconds when sync listener is present!
[ERROR] Errors: 
[ERROR]   CacheClientListenerTest>CacheListenerTest.testPutIfAbsentWithSyncListener_whenEntryExists:120->getCachingProvider:64 ? TestTimedOut test timed out after 30000 milliseconds
[ERROR]   ProcessorClassLoaderTest.setUp:113->createHazelcastMember:139 ? Guardian java.lang.NoSuchMethodException: com.hazelcast.config.OnJoinPermissionOperationName.()
[ERROR]   ProcessorClassLoaderTest.setUp:113->createHazelcastMember:139 ? Guardian java.lang.NoSuchMethodException: com.hazelcast.config.OnJoinPermissionOperationName.()
[ERROR]   ProcessorClassLoaderTest.setUp:113->createHazelcastMember:139 ? Guardian java.lang.NoSuchMethodException: com.hazelcast.config.OnJoinPermissionOperationName.()
[ERROR]   CacheConfigConstructorTest.testConstructor:44 ? Guardian java.lang.NoSuchMethodException: com.hazelcast.config.MaxSizePolicy.()
[ERROR]   ConfigConstructorTest.testConstructor:50 ? Guardian java.lang.NoSuchMethodException: com.hazelcast.config.MaxSizePolicy.()
[ERROR]   DynamicConfigurationAwareConfigConstructorTest.testConstructor:57 ? Guardian java.lang.NoSuchMethodException: com.hazelcast.config.MaxSizePolicy.()
[INFO] 
[ERROR] Tests run: 51002, Failures: 1, Errors: 7, Skipped: 238
[INFO] 

[ERROR] There are test failures.

@vbekiaris
Copy link
Collaborator

run-lab-run

@hz-devops-test
Copy link

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file 
--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   ClientMapPartitionLostListenerTest.test_mapPartitionLostListener_invoked_fromOtherNode:133->assertProxyExistsEventually:163->HazelcastTestSupport.assertTrueEventually:1338->HazelcastTestSupport.assertTrueEventually:1236 There is no proxy with name f4788131-63dc-41d5-bb5c-793faf3afab0 created (yet)
[ERROR]   MetadataRaftGroupTest.when_cpSubsystemReset_then_cpGroupIsCreatedWithDifferentGroupId:705->HazelcastRaftTestSupport.waitUntilCPDiscoveryCompleted:91->HazelcastTestSupport.assertTrueEventually:1338->HazelcastTestSupport.assertTrueEventually:1236->HazelcastRaftTestSupport.lambda$waitUntilCPDiscoveryCompleted$1:93
[ERROR] Errors: 
[ERROR]   ProcessorClassLoaderTest.setUp:113->createHazelcastMember:139 ? Guardian java.lang.NoSuchMethodException: com.hazelcast.config.OnJoinPermissionOperationName.()
[ERROR]   ProcessorClassLoaderTest.setUp:113->createHazelcastMember:139 ? Guardian java.lang.NoSuchMethodException: com.hazelcast.config.OnJoinPermissionOperationName.()
[ERROR]   ProcessorClassLoaderTest.setUp:113->createHazelcastMember:139 ? Guardian java.lang.NoSuchMethodException: com.hazelcast.config.OnJoinPermissionOperationName.()
[ERROR]   CacheConfigConstructorTest.testConstructor:44 ? Guardian java.lang.NoSuchMethodException: com.hazelcast.config.InMemoryFormat.()
[ERROR]   ConfigConstructorTest.testConstructor:50 ? Guardian java.lang.NoSuchMethodException: com.hazelcast.config.MetadataPolicy.()
[ERROR]   DynamicConfigurationAwareConfigConstructorTest.testConstructor:57 ? Guardian java.lang.NoSuchMethodException: com.hazelcast.config.MapStoreConfig$InitialLoadMode.()
[INFO] 
[ERROR] Tests run: 51002, Failures: 2, Errors: 6, Skipped: 238
[INFO] 

[ERROR] There are test failures.

@gbarnett-hz
Copy link
Contributor

Hi @mwashburn, thank you for the contribution. Would it be possible to rebase the PR with latest hazelcast:master?

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

2 similar comments
@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@srknzl srknzl removed request for a team and krzysztofslusarski May 6, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vbekiaris vbekiaris vbekiaris approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Module: Serialization Source: Community PR or issue was opened by a community user Team: Core
Projects
None yet
Milestone
No milestone
7 participants
@mwashburn @devOpsHazelcast @TomaszGaweda @vbekiaris @hz-devops-test @gbarnett-hz @mwashburn2