Bug #51

condition are tested after modifications are calculated

Added by Thomas Chemineau about 1 year ago. Updated about 1 year ago.

Status:Closed Start:06/05/2009
Priority:Normal Due date:
Assigned to:Thomas Chemineau % Done:

100%

Category:Core
Target version:1.1.0
Problem in version:

Description

Well, this is definitely a bug :)

If in lsc.properties I put something like the following :

lsc.tasks.myTask.condition.update = StringUtils.startsWith(srcBean.getAttributeValueById("userPassword"),"tes")
lsc.syncoptions.myTask.userPassword.force_value = "{SHA}" + StringUtils.hash(StringUtils.HASH_SHA1, srcBean.getAttributeValueById("userPassword"))

I get this log :

384  [main] DEBUG  org.lsc.AbstractSynchronize.synchronize2Ldap(AbstractSynchronize.java:295)   - Synchronizing org.lsc.objects.brinksUtilisateur for uid=elilly,ou=utilisateurs,dc=brinks,dc=fr
418  [main] DEBUG  org.lsc.beans.BeanComparator.getModifyEntry(BeanComparator.java:284)   - Do nothing (userpasswordsyncwith)
419  [main] DEBUG  org.lsc.beans.BeanComparator.getModifyEntry(BeanComparator.java:318)   - Modifying entry "uid=elilly,ou=utilisateurs" 
#### DEBUG #### {SHA}nU4eI71bcnBGqeO0t9tXvY1u5oQ= and tes
#### DEBUG #### false
432  [main] DEBUG  org.lsc.AbstractSynchronize.logShouldAction(AbstractSynchronize.java:489)   - Update condition false. Should have modified object uid=elilly,ou=utilisateurs,dc=brinks,dc=fr
dn: uid=elilly,ou=utilisateurs,dc=brinks,dc=fr
changetype: modify
replace: userpassword
userpassword: {SHA}nU4eI71bcnBGqeO0t9tXvY1u5oQ=

Well, you see, conditions will be tested on modified values.

In code, a condition is tested after creating the modification object. In fact, this is because we would like to print modifications that could be applied on the destination repository if the condition was true. Maybe could we calculate modifications, after testing conditions, and skip those if false. But, if an option is passed to LSC, at the execution time, LSC will calculate modifications and printing them before skipping (could be usefull for log).

An idea ?

cloneSrcBean-1.0.patch (14.9 KB) Thomas Chemineau, 11/05/2009 14:04

cloneSrcBean-1.1.patch (15.2 KB) Thomas Chemineau, 11/05/2009 14:30

cloneSrcBean-1.2.patch - Proposed patch updated (version 1.2) (18.8 KB) Jonathan Clarke, 12/05/2009 13:05


Related issues

related to Bug #47: srcBean.getDistinguishName does not works like expected Closed 04/05/2009

Associated revisions

Revision 211
Added by Jonathan Clarke about 1 year ago

Place condition evaluation before building modifications and interpreting syncoptions. Fixes #51

Revision 212
Added by Jonathan Clarke about 1 year ago

Add default values for previous patch. References #51

Revision 213
Added by Jonathan Clarke about 1 year ago

Fix a NPE in last commit. Fixes #51.

Revision 233
Added by Thomas Chemineau about 1 year ago

fixes #51 - all JNDI modifications are now calculated on a clone of srcBean, so that the syncoptions work better.

Revision 234
Added by Thomas Chemineau about 1 year ago

fixes #51 - cannot call logShouldAction method if jndiModification is null.

Revision 238
Added by Jonathan Clarke about 1 year ago

Fixes #51. Clear up conditions in clean method, and use continue to simplify and speed up. Stop on initializeExceptions.

History

Updated by Jonathan Clarke about 1 year ago

  • Status changed from New to Assigned

I think this is a bug too.

I will commit a patch that places the condition evaluation before calculating modifications. This has 2 important consequences:
1) Performance is improved, we no longer do lots of useless calculations just to throw them away if the condition evaluates to false.
2) We can use the "-n/-nc/-nu/-nd" command line options to force calculations anyway, and log them (with level DEBUG)

Updated by Jonathan Clarke about 1 year ago

  • Status changed from Assigned to Feedback
  • % Done changed from 0 to 100

Applied in changeset r211.

Updated by Jonathan Clarke about 1 year ago

Appliqué par commit r213.

Updated by Thomas Chemineau about 1 year ago

Here is a patch that clone srcBean to itmBean in BeanComparator (getModifyEntry and getAddEntry methods). Modifications are calculated on clone (itmBean). Syncoptions could now use real values (from srcBean).

Please, tests, tests, take a beer, and tests again and again!

Updated by Thomas Chemineau about 1 year ago

IBean now implements Cloneable.
Patch v1.1.

Updated by Jonathan Clarke about 1 year ago

Thomas,

I have updated your patch to always clone the source bean, from the calculateModifications() method. This is so that the DN generation is done on the intermediary bean, not the source bean (see issue #47), and it factorizes the code.

Apart from that, the patch looks good to me. I will do some more testing on my version 1.2 (attached).

Jon

Updated by Thomas Chemineau about 1 year ago

Appliqué par commit r233.

Updated by Thomas Chemineau about 1 year ago

Appliqué par commit r234.

Updated by Jonathan Clarke about 1 year ago

Applied in changeset r238.

Updated by Jonathan Clarke about 1 year ago

  • Status changed from Feedback to Closed

This fix seems good. Thomas and I have done some testing, and we haven't seen heard anything bad. Closing the issue.

Also available in: Atom PDF