On Thu, Apr 09, 2015 at 10:09:47AM -0500, Endi Sukma Dewata wrote:
On 4/8/2015 2:40 AM, Fraser Tweedale wrote:
On Mon, Apr 06, 2015 at 08:27:05PM -0500, Endi Sukma Dewata wrote:
On 3/30/2015 2:27 AM, Fraser Tweedale wrote: Let's say a profile is modified then deleted. These operations are replicated to a clone which is monitoring the changes using persistent search. When the clone receives the modify operation it will call readProfile(), which will call createProfile(), which will create an LDAPConfigStore for that profile. When LDAPConfigStore is created, it will read the profile from the database using a separate query. The problem is, at that point it's unclear whether the profile still exists.
The current code might ignore the error if the entry doesn't exist, but in general I don't think the code should make a separate query to retrieve the profile info because it could lead to inconsistencies. It should have used the LDAP entry returned by the persistent search that corresponds to the operation being processed.
Okey doke, new patch attached. The LDAPConfigStore no longer reads the entry on construction, and the LDAPProfileSubsystem reads the config from the incoming persistent search results and loads it into the LDAPConfigStore.
ACK.
There are some minor issues, they can be addressed separately:
1. The try-catch blocks inside the while loop in run() probably can be combined. If an error occurs and it's still (!stopped) it will sleep for a while before continuing.
2. The profileId should be obtained from the cn in the DN instead of from the cn in the entry since the cn in the entry could have multiple values:
String profileId = (String) ldapProfile.getAttribute("cn").getStringValues().nextElement();
3. In LDAPProfileSubsystem.createProfile() the mProfileIds is guarded against duplicates:
if (!mProfiles.containsKey(id)) mProfileIds.addElement(id);
but in ProfileSubsystem.createProfile() it's not:
mProfileIds.addElement(id);
I think they should be consistent:
4. Related to #3, I think the mProfileIds can be replaced with mProfiles.keys() which contains unique profile IDs.
Thanks! Pushed to master (1bfebdb). Patches for above issues are imminent. Cheers, Fraser