[sakai-core-team] KNL-1250 / KNL-800: syncing of provider to the realm

[Sam Ottenhoff]

I'm looking for some quick feedback on KNL-1250.  Without any feedback, I
am planning on committing the large KNL-1250 patch from Western University
later today.

  https://jira.sakaiproject.org/browse/KNL-1250

The background here: KNL-800 was committed by UMich in 2011 (
https://jira.sakaiproject.org/browse/KNL-800) and refactored a fair bit of
code in DbAuthzGroupService to synchronize roles from parent sites to the
sub-groups.  Unfortunately, when the UMich change was committed, the kernel
was silently swallowing errors related to inserts to the database when the
record already existed (unnecessary inserts to the database).  If you test
on trunk today, you will see lots of "recordAlreadyExists" SQL errors in
your logs because of a commit I made yesterday to KNL-385.  The commit I
made simply prints a WARN to the logs but changes no logic.  KNL-1183 made
the bug from KNL-800 more visible, but the way I see it, the bug is clearly
introduced in KNL-800.

The possible options going forward:

1) Use the patch from Western University submitted to KNL-1250.  I have
tested this patch for the past couple days with all combinations of
providers/no-providers/groups/no groups that I can think of and it passes
all of my testing.  The down side is that this is a large patch with a lot
of refactoring.  Paul from Western includes a long explanation of all of
the changes and the reasons for the changes.

2) We revert the UMich KNL-800 work from 2011.  The down side is that it
isn't a clean reverse merge, and the fix that KNL-800 addresses is a good
fix.

My recommendation is that we proceed with the Western University patch in
KNL-1250 and do lots of testing and then merge the fix into 10.x if it
passes all testing.  Other opinions?

--Sam
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://collab.sakaiproject.org/pipermail/sakai-core-team/attachments/20140528/83597225/attachment.html