Skip to content

Commit

Permalink
SAK-44985 Discussions topic and forum cache invalidation (sakaiprojec…
Browse files Browse the repository at this point in the history 
…t#9351)

* SAK-44985 Discussions topic and forum cache invalidation

https://jira.sakaiproject.org/browse/SAK-44985

* Remove the use of ThreadLocalManager
  • Loading branch information
@ern
ern authored Jun 10, 2021
1 parent 18fda3d commit e6f1396
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 64 deletions.
2 changes: 2 additions & 0 deletions ...forums-api/src/java/org/sakaiproject/api/app/messageforums/ui/DiscussionForumManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -666,4 +666,6 @@ public void saveTopicMessagePermissions(DiscussionTopic topic,
* @return the LRS statement, or empty if student not found or LRS service not available
*/
public Optional<LRS_Statement> getStatementForGrade(String studentUid, String forumTitle, double score);

void setUiPermissionsManager(UIPermissionsManager uiPermissionsManager);
}
2 changes: 2 additions & 0 deletions ...geforums-api/src/java/org/sakaiproject/api/app/messageforums/ui/UIPermissionsManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,4 +225,6 @@ public interface UIPermissionsManager
BulkPermission getBulkPermissions(DiscussionTopic topic, DiscussionForum forum);

BulkPermission getBulkPermissions(DiscussionForum forum);

void clearMembershipsFromCacheForArea(Area area);
}
59 changes: 22 additions & 37 deletions ...r/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ public String processActionSaveTemplateSettings()
}

setObjectPermissions(template.getArea());
areaManager.saveArea(template.getArea());
areaManager.saveArea(template.getArea());
return gotoMain();
}

Expand Down Expand Up @@ -1937,22 +1937,10 @@ private String saveTopicSettings(boolean draft)

topic = forumManager.saveTopic(topic, draft);

//anytime a forum settings change, we should update synoptic info for forums
//since permissions could have changed.
if(!isNew){
if(beforeChangeHM != null){
if(permissionsUpdated){
// set flag to true since permissions could have changed. This will force a clearing and resetting
// of the permissions cache for this area.
threadLocalManager.set("msgcntr_clear_permission_set#" + topic.getBaseForum().getArea().getId(), Boolean.TRUE);
}
updateSynopticMessagesForForumComparingOldMessagesCount(getSiteId(), topic.getBaseForum().getId(), topic.getId(), beforeChangeHM, SynopticMsgcntrManager.NUM_OF_ATTEMPTS);
}
}
//forumManager
// .saveTopicControlPermissions(topic, topicControlPermissions);
//forumManager
// .saveTopicMessagePermissions(topic, topicMessagePermissions);
//anytime a forum settings change, we should update synoptic info for forums
if (!isNew && beforeChangeHM != null) {
updateSynopticMessagesForForumComparingOldMessagesCount(getSiteId(), topic.getBaseForum().getId(), topic.getId(), beforeChangeHM, SynopticMsgcntrManager.NUM_OF_ATTEMPTS);
}
}
}
return gotoMain();
Expand Down Expand Up @@ -6623,25 +6611,25 @@ public String generatePermissionScript(){
}

public void setObjectPermissions(Object target){
Area area = null;
DiscussionForum forum = null;
DiscussionTopic topic = null;

Set<DBMembershipItem> oldMembershipItemSet = null;
Set<DBMembershipItem> membershipItemSet = new HashSet<>();
if (permissions != null) {
Area area = null;
DiscussionForum forum = null;
DiscussionTopic topic = null;

if (target instanceof DiscussionForum){
forum = ((DiscussionForum) target);
oldMembershipItemSet = uiPermissionsManager.getForumItemsSet(forum);
} else if (target instanceof Area){
area = ((Area) target);
oldMembershipItemSet = uiPermissionsManager.getAreaItemsSet(area);
} else if (target instanceof Topic){
topic = ((DiscussionTopic) target);
oldMembershipItemSet = uiPermissionsManager.getTopicItemsSet(topic);
}
Set<DBMembershipItem> oldMembershipItemSet = null;
Set<DBMembershipItem> membershipItemSet = new HashSet<>();

if (target instanceof DiscussionForum){
forum = ((DiscussionForum) target);
oldMembershipItemSet = uiPermissionsManager.getForumItemsSet(forum);
} else if (target instanceof Area){
area = ((Area) target);
oldMembershipItemSet = uiPermissionsManager.getAreaItemsSet(area);
} else if (target instanceof Topic){
topic = ((DiscussionTopic) target);
oldMembershipItemSet = uiPermissionsManager.getTopicItemsSet(topic);
}

if (permissions != null) {
for (PermissionBean permBean : permissions) {
//for group awareness
//DBMembershipItem membershipItem = permissionLevelManager.createDBMembershipItem(permBean.getItem().getName(), permBean.getSelectedLevel(), DBMembershipItem.TYPE_ROLE);
Expand Down Expand Up @@ -6670,9 +6658,6 @@ public void setObjectPermissions(Object target){
membershipItemSet.forEach(i -> ((DBMembershipItemImpl) i).setTopic(t));
}
permissionLevelManager.deleteMembershipItems(oldMembershipItemSet);
if (area != null) {
threadLocalManager.set("msgcntr_clear_permission_set#" + area.getId(), Boolean.TRUE);
}
}
siteMembers = null;
}
Expand Down
41 changes: 30 additions & 11 deletions .../src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.sakaiproject.api.app.messageforums.AreaControlPermission;
import org.sakaiproject.api.app.messageforums.AreaManager;
import org.sakaiproject.api.app.messageforums.Attachment;
import org.sakaiproject.api.app.messageforums.BaseForum;
import org.sakaiproject.api.app.messageforums.DBMembershipItem;
import org.sakaiproject.api.app.messageforums.DiscussionForum;
import org.sakaiproject.api.app.messageforums.DiscussionForumService;
Expand All @@ -62,6 +63,7 @@
import org.sakaiproject.api.app.messageforums.events.ForumsTopicEventParams;
import org.sakaiproject.api.app.messageforums.events.ForumsTopicEventParams.TopicEvent;
import org.sakaiproject.api.app.messageforums.ui.DiscussionForumManager;
import org.sakaiproject.api.app.messageforums.ui.UIPermissionsManager;
import org.sakaiproject.authz.api.AuthzGroup;
import org.sakaiproject.authz.api.AuthzGroupService;
import org.sakaiproject.authz.api.GroupNotDefinedException;
Expand All @@ -87,7 +89,6 @@
import org.sakaiproject.site.api.Group;
import org.sakaiproject.site.api.Site;
import org.sakaiproject.site.api.SiteService;
import org.sakaiproject.thread_local.api.ThreadLocalManager;
import org.sakaiproject.tool.api.SessionManager;
import org.sakaiproject.tool.api.Tool;
import org.sakaiproject.tool.api.ToolManager;
Expand All @@ -96,6 +97,7 @@
import org.sakaiproject.user.api.UserNotDefinedException;
import org.springframework.orm.hibernate5.support.HibernateDaoSupport;

import lombok.Setter;
import lombok.extern.slf4j.Slf4j;

/**
Expand Down Expand Up @@ -123,9 +125,9 @@ public class DiscussionForumManagerImpl extends HibernateDaoSupport implements
private MemoryService memoryService;
private Cache<String, Set<String>> allowedFunctionsCache;
private EventTrackingService eventTrackingService;
private ThreadLocalManager threadLocalManager;
private ToolManager toolManager;
private LearningResourceStoreService learningResourceStoreService;
@Setter private UIPermissionsManager uiPermissionsManager;

public static final int MAX_NUMBER_OF_SQL_PARAMETERS_IN_LIST = 1000;

Expand All @@ -151,10 +153,6 @@ public void setLearningResourceStoreService(LearningResourceStoreService service
learningResourceStoreService = service;
}

public void setThreadLocalManager(ThreadLocalManager threadLocalManager) {
this.threadLocalManager = threadLocalManager;
}

public void setToolManager(ToolManager toolManager) {
this.toolManager = toolManager;
}
Expand Down Expand Up @@ -1060,7 +1058,9 @@ public void setForumManager(MessageForumsForumManager forumManager)
public DiscussionForum createForum()
{
log.debug("createForum()");
return forumManager.createDiscussionForum();
DiscussionForum forum = forumManager.createDiscussionForum();
flagAreaCacheForClearing(forum);
return forum;
}

/*
Expand All @@ -1075,6 +1075,7 @@ public void deleteForum(DiscussionForum forum)
log.debug("setForumManager(DiscussionForum" + forum + ")");
}
forumManager.deleteDiscussionForum(forum);
flagAreaCacheForClearing(forum);
}

/*
Expand All @@ -1093,7 +1094,9 @@ public DiscussionTopic createTopic(DiscussionForum forum)
log.debug("Attempt to create topic with out forum");
return null;
}
return forumManager.createDiscussionForumTopic(forum);
DiscussionTopic topic = forumManager.createDiscussionForumTopic(forum);
flagAreaCacheForClearing(forum);
return topic;
}

/*
Expand Down Expand Up @@ -1153,13 +1156,27 @@ public DiscussionForum saveForum(DiscussionForum forum, boolean draft, String co
forumReturn.setSortIndex(0);
area.addDiscussionForum(forumReturn);
areaManager.saveArea(area, currentUser);
flagAreaCacheForClearing(area);
}
// set flag to true since permissions could have changed. This will force a clearing and resetting
// of the permissions cache for this area.
threadLocalManager.set("msgcntr_clear_permission_set#" + forumReturn.getArea().getId(), Boolean.TRUE);
return forumReturn;
}

private void flagAreaCacheForClearing(Object object) {
Area area = null;
if (object instanceof Topic) {
Topic topic = (Topic) object;
if (topic.getBaseForum() != null) area = topic.getBaseForum().getArea();
if (topic.getOpenForum() != null) area = topic.getOpenForum().getArea();
if (topic.getPrivateForum() != null) area = topic.getPrivateForum().getArea();
} else if (object instanceof BaseForum) {
BaseForum forum = (BaseForum) object;
area = forum.getArea();
} else if (object instanceof Area) {
area = (Area) object;
}
uiPermissionsManager.clearMembershipsFromCacheForArea(area);
}

@Override
public void saveTopicAsDraft(DiscussionTopic topic)
{
Expand Down Expand Up @@ -1205,6 +1222,7 @@ public DiscussionTopic saveTopic(DiscussionTopic topic, boolean draft, ForumsTop
forum = forumManager.saveDiscussionForum(forum, forum.getDraft(), false, currentUser); // event already logged by saveDiscussionForumTopic()
//sak-5146 forumManager.saveDiscussionForum(forum);
}
flagAreaCacheForClearing(forum);

if (params != null)
{
Expand All @@ -1227,6 +1245,7 @@ public void deleteTopic(DiscussionTopic topic)
log.debug("deleteTopic(DiscussionTopic " + topic + ")");
}
forumManager.deleteDiscussionForumTopic(topic);
flagAreaCacheForClearing(topic);
}

/*
Expand Down
19 changes: 5 additions & 14 deletions ...pl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

import org.apache.commons.lang3.StringUtils;
import org.sakaiproject.api.app.messageforums.Area;
import org.sakaiproject.api.app.messageforums.BaseForum;
import org.sakaiproject.api.app.messageforums.BulkPermission;
import org.sakaiproject.api.app.messageforums.DBMembershipItem;
import org.sakaiproject.api.app.messageforums.DiscussionForum;
Expand All @@ -48,7 +47,6 @@
import org.sakaiproject.site.api.Group;
import org.sakaiproject.site.api.Site;
import org.sakaiproject.site.api.SiteService;
import org.sakaiproject.thread_local.api.ThreadLocalManager;
import org.sakaiproject.tool.api.SessionManager;
import org.sakaiproject.tool.api.ToolManager;
import org.sakaiproject.user.api.User;
Expand Down Expand Up @@ -84,7 +82,6 @@ public class UIPermissionsManagerImpl implements UIPermissionsManager {
@Setter private SecurityService securityService;
@Setter private SessionManager sessionManager;
@Setter private SiteService siteService;
@Setter private ThreadLocalManager threadLocalManager;
@Setter private ToolManager toolManager;
@Setter private UserDirectoryService userDirectoryService;

Expand All @@ -95,6 +92,7 @@ public void init() {
log.info("init()");
userGroupMembershipCache = memoryService.getCache("org.sakaiproject.component.app.messageforums.ui.UIPermissionsManagerImpl.userGroupMembershipCache");
membershipItemCache = memoryService.getCache("org.sakaiproject.component.app.messageforums.ui.UIPermissionsManagerImpl.membershipItemCache");
forumManager.setUiPermissionsManager(this);
}

@Override
Expand Down Expand Up @@ -684,19 +682,12 @@ private boolean checkBaseConditions(DiscussionTopic topic, DiscussionForum forum
|| (topic != null && topic.getRestrictPermissionsForGroups() && isInstructorForAllowedGroup(topic.getId(), false));
}

private void clearMembershipsFromCacheForArea(Area area) {
public void clearMembershipsFromCacheForArea(Area area) {
if (area == null || area.getId() == null) return;
String areaId = area.getId().toString();
String areaCacheKey = "area_" + areaId;
String forumCacheKey = "forum_" + areaId;
String topicCacheKey = "topic_" + areaId;
Object obj = threadLocalManager.get("msgcntr_clear_permission_set#" + areaId);
if (obj instanceof Boolean && (Boolean) obj) {
membershipItemCache.remove(areaCacheKey);
membershipItemCache.remove(forumCacheKey);
membershipItemCache.remove(topicCacheKey);
threadLocalManager.set("msgcntr_clear_permission_set#" + areaId, Boolean.FALSE);
}
membershipItemCache.remove("area_" + areaId);
membershipItemCache.remove("forum_" + areaId);
membershipItemCache.remove("topic_" + areaId);
}

private Set<DBMembershipItem> getTopicMemberships(Area area) {
Expand Down
2 changes: 0 additions & 2 deletions msgcntr/messageforums-component-impl/src/webapp/WEB-INF/components.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@
<property name="securityService" ref="org.sakaiproject.authz.api.SecurityService"/>
<property name="sessionManager" ref="org.sakaiproject.tool.api.SessionManager" />
<property name="siteService" ref="org.sakaiproject.site.api.SiteService"/>
<property name="threadLocalManager" ref="org.sakaiproject.thread_local.api.ThreadLocalManager"/>
<property name="toolManager" ref="org.sakaiproject.tool.api.ActiveToolManager"/>
<property name="userDirectoryService" ref="org.sakaiproject.user.api.UserDirectoryService"/>
</bean>
Expand Down Expand Up @@ -301,7 +300,6 @@
<property name="contentHostingService" ref="org.sakaiproject.content.api.ContentHostingService"/>
<property name="memoryService" ref="org.sakaiproject.memory.api.MemoryService"/>
<property name="eventTrackingService" ref="org.sakaiproject.event.api.EventTrackingService"/>
<property name="threadLocalManager" ref="org.sakaiproject.thread_local.api.ThreadLocalManager"/>
<property name="toolManager" ref="org.sakaiproject.tool.api.ToolManager"/>
<property name="learningResourceStoreService" ref="org.sakaiproject.event.api.LearningResourceStoreService"/>
</bean>
Expand Down

0 comments on commit e6f1396

Please sign in to comment.