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

Added load event listening capability to client side added listener #13380

Merged
merged 1 commit into from
Jul 5, 2018
Merged

Added load event listening capability to client side added listener #13380

merged 1 commit into from
Jul 5, 2018

Conversation

ahmetmircik
Copy link
Member

@ahmetmircik ahmetmircik commented Jun 29, 2018 •
edited
Loading

After merge, java client will also listen loaded event via EntryLoadedListener. This capability was missing.

Thanks @lazerion for finding this.

client part of #1542

@ahmetmircik ahmetmircik added this to the 3.11 milestone Jun 29, 2018
@ahmetmircik ahmetmircik requested review from vbekiaris and a user June 29, 2018 09:45
@ahmetmircik ahmetmircik force-pushed the fix/3.11/clientSideEntryLoadedListener branch from cdce576 to dbabd76 Compare June 29, 2018 10:06
@ghost
Copy link

ghost commented Jun 29, 2018 •
edited by ghost
Loading

@ahmetmircik
bootstrapped test suite from ahmetmircik:fix/3.11/clientSideEntryLoadedListener works fine 👍

@ghost
Copy link

ghost commented Jun 29, 2018

@ahmetmircik one point though, we got 2000 loaded events on client side, however we load 1000 items. Cluster size is 2 btw, is it expected? do we need to look into it?

@ahmetmircik
Copy link
Member Author

run-lab-run

@ahmetmircik ahmetmircik force-pushed the fix/3.11/clientSideEntryLoadedListener branch 2 times, most recently from e51c470 to bab9c99 Compare July 1, 2018 12:10
@ahmetmircik
Copy link
Member Author

run-lab-run

2 similar comments
@Holmistr
Copy link
Contributor

Holmistr commented Jul 2, 2018

run-lab-run

@ahmetmircik
Copy link
Member Author

run-lab-run

@ahmetmircik ahmetmircik force-pushed the fix/3.11/clientSideEntryLoadedListener branch from bab9c99 to 48d01eb Compare July 2, 2018 08:48
@Holmistr
Copy link
Contributor

Holmistr commented Jul 2, 2018

run-lab-run

vbekiaris
vbekiaris reviewed Jul 2, 2018
View reviewed changes
hazelcast/src/test/java/com/hazelcast/map/impl/MapListenerAdapterTest.java
public void ensure_map_listener_adapter_implements_listeners_for_all_entry_event_types_except_invalidation() {
MapListenerAdapter mapListenerAdapterInstance = new MapListenerAdapter();
Map<EntryEventType, ConstructorFunction<MapListener, ListenerAdapter>> constructors = getConstructors();
for (Map.Entry<EntryEventType, ConstructorFunction<MapListener, ListenerAdapter>> entry : constructors.entrySet()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we should iterate over all EntryEventType.values() to make sure a constructor exists for each one of them (except INVALIDATION).

Copy link
Member Author

Choose a reason for hiding this comment

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

If constructor does not return null, this means the listener object (MapListenerAdapter in the test) implements the required interface to listen the event. For the test we expect MapListenerAdapter to implement all interfaces except one for INVALIDATION. This test is needed to not to forget addition of new listener interfaces to the adapter.

@ahmetmircik ahmetmircik force-pushed the fix/3.11/clientSideEntryLoadedListener branch from 48d01eb to c1d6c1a Compare July 2, 2018 21:49
@ahmetmircik ahmetmircik force-pushed the fix/3.11/clientSideEntryLoadedListener branch from c1d6c1a to d9c12a6 Compare July 3, 2018 09:25
@ghost
Copy link

ghost commented Jul 3, 2018

@ahmetmircik many thanks, after latest updates we got expected event count. 👍

@ahmetmircik ahmetmircik merged commit 918ec2b into hazelcast:master Jul 5, 2018
@ahmetmircik ahmetmircik deleted the fix/3.11/clientSideEntryLoadedListener branch July 5, 2018 08:13
@ahmetmircik ahmetmircik mentioned this pull request Jun 17, 2019
Merged
@mmedenjak mmedenjak added the Source: Internal PR or issue was opened by an employee label Apr 13, 2020
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

Assignees
No one assigned
Labels
Source: Internal PR or issue was opened by an employee Team: Client Team: Core Type: Defect
Projects
None yet
Milestone
3.11
Development

Successfully merging this pull request may close these issues.
4 participants
@ahmetmircik @Holmistr @vbekiaris @mmedenjak