Site Caching Seems Confused when a tool (sakai.iframe.site) does saveSiteInfo()
Description
Attachments
- 31 Jul 2013, 10:24 PM
is related to
Activity
Matthew Jones November 26, 2013 at 3:55 PM
Verified that this fixes it, is also a problem since 2.9.2
Hudson CI Server September 24, 2013 at 5:15 PM
Integrated in kernel-trunk #710 (See http://builds.sakaiproject.org:8080/job/kernel-trunk/710/)
https://sakaiproject.atlassian.net/browse/KNL-1102#icft=KNL-1102 - Site caching seems confused when a tool does saveSiteInfo (Revision 129943)
Result = SUCCESS
Matthew Jones September 24, 2013 at 4:08 PM
Rather than leave this bug hanging open, I'm committing the last patch Noah sent as it works for me and seems reasonable.
Charles R Severance August 1, 2013 at 9:44 AM
I like the patch to siteInfo() - I am happy to accept the fact that PortletIframe may have a bug - given who wrote it I will look for that bug - my main goal was to get a review of saveSiteInfo() with an eye to cacheing which has happened.
Noah Botimer August 1, 2013 at 9:35 AM
Oops – sorry I didn't address that. I think it's also fine for saveSiteInfo to bump the caches. I am just not seeing that as the problem and wanted to isolate the staleness issue from caching improvements.
To be clear, I think there is still some bug in the portlet code.
It seems to me that the saveSiteInfo shortcut method (really, the underlying storage update) is legacy and should follow a standard save instead of a one-off query, so cache management happens in a more consistent way. I initially changed saveSiteInfo to just retrieve/update/save the site, but the permissions checks are very subtly different, so I changed course (consolidating the transaction and changing only the flow/checks for the portlet). The save() method checks for SECURE_UPDATE_GROUP_MEMBERSHIP or SECURE_UPDATE_SITE_MEMBERSHIP or SECURE_UPDATE_SITE, while saveSiteInfo always and only checks SECURE_UPDATE_SITE. It may be totally OK to normalize this but I couldn't find any written account of the intention either way.
My patch to saveSiteInfo is here:
Index: kernel-impl/src/main/java/org/sakaiproject/site/impl/BaseSiteService.java
===================================================================
— kernel-impl/src/main/java/org/sakaiproject/site/impl/BaseSiteService.java (revision 128041)
+++ kernel-impl/src/main/java/org/sakaiproject/site/impl/BaseSiteService.java (working copy)
@@ -1111,18 +1111,10 @@
*/
public void saveSiteInfo(String id, String description, String infoUrl) throws IdUnusedException, PermissionException
{
- String ref = siteReference(id);
-
- // check security (throws if not permitted)
- unlock(SECURE_UPDATE_SITE, ref);
-
- // check for existance
- if (!storage().check(id))
- {
- throw new IdUnusedException(id);
- }
-
- storage().saveInfo(id, description, infoUrl);
+ Site site = getSite(id);
+ site.setDescription(description);
+ site.setInfoUrl(infoUrl);
+ save(site);
}
/**
This is a strange little problem. It only seems to fail on the Oracle nightly instance. I don't know if this is Oracle-related or has something to do with site caching setup. Steps to reproduce are as follows:
Make a project site including the Home tool
Navigate to the site Home
Edit the instance and set the URL to something like http://www.dr-chuck.com/
Press "Save"
Good behavior: You see www.dr-chuck.com
Bad behavior: you see the old setting (likely blank)
When you see the bad behavior - press "Refresh" and it will be fine.
You can then press edit again and change the URL to http://www.pythonlearn.com/ and press "Save"
If it is broken - you still see www.dr-chuck.com until you press "Refresh"
You can also press "Edit" again and see the new value is there in the properties
If you press "Cancel" you will see the new information
When it is not working (i.e. on Oracle/nightly) the change is not effective until a second time the page is shown. When it is working (MySql, HSQL) as soon as you press save you see what you are supposed to see.
I think that someone should look carefully at the saveSiteInfo() method in the DB implementation of the site service and think through the cache implications.