We're updating the issue view to help you get more done. 

GroupManager.getSharedGroups returning old values

Description

Tests for adding and removing members of a Group

Take this scenario to run some tests

  1. create the user "user001" and "user002"

  2. create the groups "customers" and "vendors"

  3. Mark "Enable contact list group sharing" in both groups

  4. add "user002" in the group "customers"

Test 1 (Failed)

Given: group "customers" has only the user002
*And:* Load /user-roster.jsp?username=user001
When: add "user001" in the group "customers"
And: clear "Roster" cache
*Then:* Page "/user-roster.jsp?username=user001" does NOT show the user002 

Analysis

It can be seen that once the getSharedGroups is called, it is not updated anymore (except when the "Group Metadata Cache" is cleared).

In the GroupManager class, the listeners for memberAdded, memberRemoved, adminAdded and adminRemoved only remove the bareJID from groupMetaCache, not the userNode ("user001" in such case).
It makes the groupMetaCache to be inconsistent when RosterManager invoke getSharedGroups


Tests for changing the 'sharedRoster.showInRoster' property of a Group

  1. create the user "user001"

  2. create the groups "all" and "customers"

  3. add the "user001" in the group "customers"

Test 1 (OK)

Given: group "all" has no user
And: "Group Metadata Cache" has been cleared
When: change the group "all" to "Enable contact list group sharing" > "Share group with additional users" > "All users"
Then: GroupManager.getInstance().getSharedGroups("user001") = [all, customers]

Test 2 (Failed)

Given: group "all" has no user
And: "Group Metadata Cache" has been cleared
And: invoke GroupManager.getInstance().getSharedGroups("user001") = [customers]
When: change the group "all" to "Enable contact list group sharing" > "Share group with additional users" > "All users"
Then: GroupManager.getInstance().getSharedGroups("user001") still is [customers]

Analysis

Changing the sharing type of a group ('sharedRoster.showInRoster') was not propertly handled by groupModified listener.


Tests for changing the 'sharedRoster.groupList' property of a Group

  1. create the user "user001"

  2. create the groups "all", "customers" and "vendors"

  3. add the "user001" in the group "customers"

Test 1 (OK)

Given: group "vendors" has no user
And: "Group Metadata Cache" has been cleared
When: change the group "all" to "Enable contact list group sharing" > "Share group with additional users" > "The following groups" > "customers"
Then: GroupManager.getInstance().getSharedGroups("user001") = [all, customers]

Test 1 (Failed)

Given: group "vendors" has no user
And: "Group Metadata Cache" has been cleared
invoke GroupManager.getInstance().getSharedGroups("user001") = [customers]
When: change the group "all" to "Enable contact list group sharing" > "Share group with additional users" > "The following groups" > "customers"
Then: GroupManager.getInstance().getSharedGroups("user001") still is [customers]

Analysis

Changing the shared groupList ('sharedRoster.groupList') was not propertly handled by groupModified listener.
Another bug was found: the value of property 'sharedRoster.showInRoster' is not set to 'spefgroups'. The OF changes it to 'onlyGroup' when editing the groups. The differentiation is done verifying if the 'groupList' is not empty.

Environment

None

Acceptance Test - Entry

None

Activity

Show:
André Berenguel
April 7, 2018, 5:29 PM

After some time, I tested these cases in the master branch of Openfire (commit 6c32335bf).

  1. The GroupManager#getSharedGroups is still inconsistent for userNode keys ("user001", for example). The only single caller of GroupManager#getSharedGroups is the RosterManager#getSharedGroups, that checks if the user really belongs to the group. If the cache contains extra groups, it is not a problem: the RosterManager ignores it. If the cache does not contains a new group, the RosterManager doesn't process it, so the Roster has less groups than expected.

  2. In order to execute the GroupManager.getInstance().getSharedGroups("user001") graphically, firstly clear the Roster cache in system-cache.jsp, so try to see the Roster via user-roster.jsp?username=user001.

  3. I've rewritten the test cases in order to it be more reproducible. 

 

André Berenguel
April 7, 2018, 6:13 PM

I've just rebased the old PR in the master branch.

Fixed

Assignee

André Berenguel

Reporter

André Berenguel

Labels

None

Expected Effort

Medium

Ignite Forum URL

None

Fix versions

Affects versions

Priority

Critical
Configure