From 6b5d3215dcddf76d55180f957ab88722a0140d15 Mon Sep 17 00:00:00 2001 From: srosse <none@none> Date: Mon, 11 Feb 2013 10:22:35 +0100 Subject: [PATCH] OO-538: review use of @Transactional --- .../olat/catalog/ui/CatalogController.java | 1 + .../org/olat/group/BusinessGroupService.java | 4 +- .../manager/BusinessGroupServiceImpl.java | 57 +++---------------- .../manager/InstantMessagingServiceImpl.java | 9 --- .../olat/repository/RepositoryEntry.hbm.xml | 3 +- .../org/olat/repository/RepositoryEntry.java | 28 ++------- .../olat/repository/RepositoryManager.java | 11 ++-- 7 files changed, 23 insertions(+), 90 deletions(-) diff --git a/src/main/java/org/olat/catalog/ui/CatalogController.java b/src/main/java/org/olat/catalog/ui/CatalogController.java index f4dd4dbfdc9..6ba3953c371 100644 --- a/src/main/java/org/olat/catalog/ui/CatalogController.java +++ b/src/main/java/org/olat/catalog/ui/CatalogController.java @@ -1167,6 +1167,7 @@ public class CatalogController extends BasicController implements Activateable2 ContextEntry catCe = entries.remove(0); Long catId = catCe.getOLATResourceable().getResourceableId(); + if(catId == null || catId.longValue() == 0) return; CatalogEntry ce = CatalogManager.getInstance().loadCatalogEntry(catId); switch(ce.getType()) { case CatalogEntry.TYPE_NODE: { diff --git a/src/main/java/org/olat/group/BusinessGroupService.java b/src/main/java/org/olat/group/BusinessGroupService.java index 55c2c7a8e8f..f0171d2e343 100644 --- a/src/main/java/org/olat/group/BusinessGroupService.java +++ b/src/main/java/org/olat/group/BusinessGroupService.java @@ -459,9 +459,7 @@ public interface BusinessGroupService { public BusinessGroupAddResponse moveIdentityFromWaitingListToParticipant(Identity ureqIdentity, List<Identity> identities, BusinessGroup currBusinessGroup, MailPackage mailing); - - public BusinessGroupAddResponse addToSecurityGroupAndFireEvent(Identity ureqIdentity, List<Identity> addIdentities, SecurityGroup secGroup); - + public void removeAndFireEvent(Identity ureqIdentity, List<Identity> addIdentities, SecurityGroup secGroup); /** diff --git a/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java b/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java index 359a6431a2b..bdf2aca9546 100644 --- a/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java +++ b/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java @@ -231,6 +231,7 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override + @Transactional public BusinessGroup updateBusinessGroup(Identity ureqIdentity, BusinessGroup group, String name, String description, Integer minParticipants, Integer maxParticipants, Boolean waitingList, Boolean autoCloseRanks) { @@ -321,25 +322,21 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override - @Transactional public BusinessGroup loadBusinessGroup(BusinessGroup group) { return businessGroupDAO.load(group.getKey()); } @Override - @Transactional public BusinessGroup loadBusinessGroup(Long key) { return businessGroupDAO.load(key); } @Override - @Transactional public BusinessGroup loadBusinessGroup(OLATResource resource) { return businessGroupDAO.load(resource.getResourceableId()); } @Override - @Transactional public List<BusinessGroup> loadBusinessGroups(Collection<Long> keys) { return businessGroupDAO.load(keys); } @@ -444,6 +441,7 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override + @Transactional public BusinessGroup mergeBusinessGroups(final Identity ureqIdentity, BusinessGroup targetGroup, final List<BusinessGroup> groupsToMerge, MailPackage mailing) { groupsToMerge.remove(targetGroup);//to be sure @@ -626,20 +624,17 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override - @Transactional public BusinessGroup findBusinessGroup(SecurityGroup secGroup) { return businessGroupDAO.findBusinessGroup(secGroup); } @Override - @Transactional(readOnly=true) public List<BusinessGroup> findBusinessGroupsOwnedBy(Identity identity, OLATResource resource) { SearchBusinessGroupParams params = new SearchBusinessGroupParams(identity, true, false); return businessGroupDAO.findBusinessGroups(params, resource, 0, -1); } @Override - @Transactional(readOnly=true) public List<BusinessGroup> findBusinessGroupsAttendedBy(Identity identity, OLATResource resource) { SearchBusinessGroupParams params = new SearchBusinessGroupParams(identity, false, true); return businessGroupDAO.findBusinessGroups(params, resource, 0, -1); @@ -651,7 +646,6 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override - @Transactional(readOnly=true) public int countBusinessGroups(SearchBusinessGroupParams params, OLATResource resource) { if(params == null) { params = new SearchBusinessGroupParams(); @@ -660,7 +654,6 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override - @Transactional(readOnly=true) public List<BusinessGroup> findBusinessGroups(SearchBusinessGroupParams params, OLATResource resource, int firstResult, int maxResults, BusinessGroupOrder... ordering) { if(params == null) { @@ -670,7 +663,6 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override - @Transactional(readOnly=true) public int countBusinessGroupViews(SearchBusinessGroupParams params, OLATResource resource) { if(params == null) { params = new SearchBusinessGroupParams(); @@ -679,7 +671,6 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override - @Transactional(readOnly=true) public List<BusinessGroupView> findBusinessGroupViews(SearchBusinessGroupParams params, OLATResource resource, int firstResult, int maxResults, BusinessGroupOrder... ordering) { if(params == null) { @@ -689,30 +680,27 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override - @Transactional(readOnly=true) public List<BusinessGroupView> findBusinessGroupViewsWithAuthorConnection(Identity author) { return businessGroupDAO.findBusinessGroupWithAuthorConnection(author); } @Override - @Transactional(readOnly=true) public List<Long> toGroupKeys(String groupNames, OLATResource resource) { return businessGroupRelationDAO.toGroupKeys(groupNames, resource); } @Override - @Transactional(readOnly=true) public int countContacts(Identity identity) { return contactDao.countContacts(identity); } @Override - @Transactional(readOnly=true) public List<Identity> findContacts(Identity identity, int firstResult, int maxResults) { return contactDao.findContacts(identity, firstResult, maxResults); } @Override + @Transactional public void deleteBusinessGroup(BusinessGroup group) { try{ OLATResourceableJustBeforeDeletedEvent delEv = new OLATResourceableJustBeforeDeletedEvent(group); @@ -782,6 +770,7 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override + @Transactional public MailerResult deleteBusinessGroupWithMail(BusinessGroup businessGroupTodelete, String businessPath, Identity deletedBy, Locale locale) { Codepoint.codepoint(this.getClass(), "deleteBusinessGroupWithMail"); @@ -829,6 +818,7 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override + @Transactional public BusinessGroupAddResponse addOwners(Identity ureqIdentity, Roles ureqRoles, List<Identity> addIdentities, BusinessGroup group, MailPackage mailing) { BusinessGroupAddResponse response = new BusinessGroupAddResponse(); @@ -1022,6 +1012,7 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override + @Transactional public void removeMembers(Identity ureqIdentity, List<Identity> identities, OLATResource resource, MailPackage mailing) { if(identities == null || identities.isEmpty() || resource == null) return;//nothing to do @@ -1186,6 +1177,7 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override + @Transactional public BusinessGroupAddResponse moveIdentityFromWaitingListToParticipant(Identity ureqIdentity, List<Identity> identities, BusinessGroup currBusinessGroup, MailPackage mailing) { @@ -1209,30 +1201,6 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD return response; } - - @Override - public BusinessGroupAddResponse addToSecurityGroupAndFireEvent(Identity ureqIdentity, List<Identity> addIdentities, SecurityGroup secGroup) { - BusinessGroupAddResponse response = new BusinessGroupAddResponse(); - for (Identity identity : addIdentities) { - if (securityManager.isIdentityPermittedOnResourceable(identity, Constants.PERMISSION_HASROLE, Constants.ORESOURCE_GUESTONLY)) { - response.getIdentitiesWithoutPermission().add(identity); - } - // Check if identity is already in group. make a db query in case - // someone in another workflow already added this user to this group. if - // found, add user to model - else if (securityManager.isIdentityInSecurityGroup(identity, secGroup)) { - response.getIdentitiesAlreadyInGroup().add(identity); - } else { - // identity has permission and is not already in group => add it - securityManager.addIdentityToSecurityGroup(identity, secGroup); - ThreadLocalUserActivityLogger.log(GroupLoggingAction.GROUP_OWNER_ADDED, getClass(), LoggingResourceable.wrap(identity)); - - response.getAddedIdentities().add(identity); - log.audit("added identity '" + identity.getName() + "' to securitygroup with key " + secGroup.getKey()); - } - } - return response; - } @Override public void removeAndFireEvent(Identity ureqIdentity, List<Identity> identities, SecurityGroup secGroup) { @@ -1384,13 +1352,11 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override - @Transactional(readOnly=true) public boolean hasResources(BusinessGroup group) { return businessGroupRelationDAO.countResources(group) > 0; } @Override - @Transactional(readOnly=true) public boolean hasResources(List<BusinessGroup> groups) { return businessGroupRelationDAO.countResources(groups) > 0; } @@ -1479,7 +1445,7 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override - @Transactional(readOnly=true) + @Transactional public int countDuplicateMembers(RepositoryEntry entry, boolean coaches, boolean participants) { return dedupSingleRepositoryentry(null, entry, coaches, participants, true); } @@ -1567,31 +1533,26 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override - @Transactional(readOnly=true) public List<OLATResource> findResources(Collection<BusinessGroup> groups, int firstResult, int maxResults) { return businessGroupRelationDAO.findResources(groups, firstResult, maxResults); } @Override - @Transactional(readOnly=true) public List<RepositoryEntry> findRepositoryEntries(Collection<BusinessGroup> groups, int firstResult, int maxResults) { return businessGroupRelationDAO.findRepositoryEntries(groups, firstResult, maxResults); } @Override - @Transactional(readOnly=true) public List<RepositoryEntryShort> findShortRepositoryEntries(Collection<BusinessGroupShort> groups, int firstResult, int maxResults) { return businessGroupRelationDAO.findShortRepositoryEntries(groups, firstResult, maxResults); } @Override - @Transactional(readOnly=true) public List<BGRepositoryEntryRelation> findRelationToRepositoryEntries(Collection<Long> groupKeys, int firstResult, int maxResults) { return businessGroupRelationDAO.findRelationToRepositoryEntries(groupKeys, firstResult, maxResults); } @Override - @Transactional(readOnly=true) public boolean isIdentityInBusinessGroup(Identity identity, BusinessGroup businessGroup) { SecurityGroup participants = businessGroup.getPartipiciantGroup(); if (participants != null && securityManager.isIdentityInSecurityGroup(identity, participants)) { @@ -1605,7 +1566,6 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override - @Transactional(readOnly=true) public List<BusinessGroupMembership> getBusinessGroupMembership(Collection<Long> businessGroups, Identity... identity) { List<BusinessGroupMembershipViewImpl> views = businessGroupDAO.getMembershipInfoInBusinessGroups(businessGroups, identity); @@ -1651,7 +1611,6 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD } @Override - @Transactional(readOnly=true) public boolean isIdentityInBusinessGroup(Identity identity, Long groupKey, boolean ownedById, boolean attendedById, OLATResource resource) { return businessGroupRelationDAO.isIdentityInBusinessGroup(identity, groupKey, ownedById, attendedById, resource); diff --git a/src/main/java/org/olat/instantMessaging/manager/InstantMessagingServiceImpl.java b/src/main/java/org/olat/instantMessaging/manager/InstantMessagingServiceImpl.java index 342508b207c..e4463413f51 100644 --- a/src/main/java/org/olat/instantMessaging/manager/InstantMessagingServiceImpl.java +++ b/src/main/java/org/olat/instantMessaging/manager/InstantMessagingServiceImpl.java @@ -110,7 +110,6 @@ public class InstantMessagingServiceImpl extends BasicManager implements Instant } @Override - @Transactional(readOnly=true) public String getStatus(Long identityKey) { return prefsDao.getStatus(identityKey); } @@ -146,7 +145,6 @@ public class InstantMessagingServiceImpl extends BasicManager implements Instant } @Override - @Transactional public InstantMessage getMessageById(Identity identity, Long messageId, boolean markedAsRead) { InstantMessageImpl msg = imDao.loadMessageById(messageId); if(markedAsRead && msg != null) { @@ -157,7 +155,6 @@ public class InstantMessagingServiceImpl extends BasicManager implements Instant } @Override - @Transactional public List<InstantMessage> getMessages(Identity identity, OLATResourceable chatResource, Date from, int firstResult, int maxResults, boolean markedAsRead) { List<InstantMessage> msgs = imDao.getMessages(chatResource, from, firstResult, maxResults); @@ -222,13 +219,11 @@ public class InstantMessagingServiceImpl extends BasicManager implements Instant } @Override - @Transactional(readOnly=true) public List<InstantMessageNotification> getNotifications(Identity identity) { return imDao.getNotifications(identity); } @Override - @Transactional(readOnly=true) public Buddy getBuddyById(Long identityKey) { IdentityShort identity = securityManager.loadIdentityShortByKey(identityKey); String fullname = userManager.getUserDisplayName(identity); @@ -237,7 +232,6 @@ public class InstantMessagingServiceImpl extends BasicManager implements Instant } @Override - @Transactional(readOnly=true) public List<BuddyGroup> getBuddyGroups(Identity me, boolean offlineUsers) { List<BuddyGroup> groups = new ArrayList<BuddyGroup>(25); Map<Long,BuddyGroup> groupMap = new HashMap<Long,BuddyGroup>(); @@ -280,7 +274,6 @@ public class InstantMessagingServiceImpl extends BasicManager implements Instant } @Override - @Transactional(readOnly=true) public List<Buddy> getOnlineBuddies() { Collection<Long> ids = sessionManager.getUsersOnline(); List<IdentityShort> contacts = securityManager.loadIdentityShortByKeys(ids); @@ -294,7 +287,6 @@ public class InstantMessagingServiceImpl extends BasicManager implements Instant } @Override - @Transactional(readOnly=true) public BuddyStats getBuddyStats(Identity me) { BuddyStats stats = new BuddyStats(); @@ -306,7 +298,6 @@ public class InstantMessagingServiceImpl extends BasicManager implements Instant } @Override - @Transactional(readOnly=true) public List<Buddy> getBuddiesListenTo(OLATResourceable chatResource) { List<RosterEntryView> roster = rosterDao.getRosterView(chatResource, 0, -1); List<Buddy> buddies = new ArrayList<Buddy>(); diff --git a/src/main/java/org/olat/repository/RepositoryEntry.hbm.xml b/src/main/java/org/olat/repository/RepositoryEntry.hbm.xml index 4e7fa2974ee..54c42d5b5e5 100644 --- a/src/main/java/org/olat/repository/RepositoryEntry.hbm.xml +++ b/src/main/java/org/olat/repository/RepositoryEntry.hbm.xml @@ -96,7 +96,7 @@ </property> <property name="statusCode" column="statuscode" unique="false" type="int"/> - + <!-- <list name="metaDataElements" table="o_repositorymetadata" lazy="true"> <key column="fk_repositoryentry"/> <index column="metadataelement_id"/> @@ -111,6 +111,7 @@ </property> </composite-element> </list> + --> </class> diff --git a/src/main/java/org/olat/repository/RepositoryEntry.java b/src/main/java/org/olat/repository/RepositoryEntry.java index e06f793f440..af2adecd9c0 100644 --- a/src/main/java/org/olat/repository/RepositoryEntry.java +++ b/src/main/java/org/olat/repository/RepositoryEntry.java @@ -25,9 +25,7 @@ package org.olat.repository; -import java.util.ArrayList; import java.util.Date; -import java.util.List; import org.olat.basesecurity.IdentityImpl; import org.olat.basesecurity.SecurityGroup; @@ -83,7 +81,7 @@ public class RepositoryEntry extends PersistentObject implements ModifiedInfo, O private boolean canDownload; private boolean membersOnly;//fxdiff VCRP-1,2: access control of resources private int statusCode; - private List<MetaDataElement> metaDataElements; + //private List<MetaDataElement> metaDataElements; private long launchCounter; private long downloadCounter; private Date lastUsage; @@ -100,7 +98,7 @@ public class RepositoryEntry extends PersistentObject implements ModifiedInfo, O */ RepositoryEntry() { softkey = CodeHelper.getGlobalForeverUniqueID(); - metaDataElements = new ArrayList<MetaDataElement>(); + //metaDataElements = new ArrayList<MetaDataElement>(); access = ACC_OWNERS; } @@ -160,16 +158,16 @@ public class RepositoryEntry extends PersistentObject implements ModifiedInfo, O } /** * @return Returns the metaDataElements. - */ + *//* public List<MetaDataElement> getMetaDataElements() { return metaDataElements; - } + }*/ /** * @param metaDataElements The metaDataElements to set. - */ + *//* public void setMetaDataElements(List<MetaDataElement> metaDataElements) { this.metaDataElements = metaDataElements; - } + }*/ /** * @return Returns the statusCode. */ @@ -372,20 +370,6 @@ public class RepositoryEntry extends PersistentObject implements ModifiedInfo, O public void setLaunchCounter(long l) { launchCounter = l; } - - /** - * Increment launch counter. - */ - public void incrementLaunchCounter() { - launchCounter++; - } - - /** - * Increment download counter. - */ - public void incrementDownloadCounter() { - downloadCounter++; - } /** * @return Returns the displayname. diff --git a/src/main/java/org/olat/repository/RepositoryManager.java b/src/main/java/org/olat/repository/RepositoryManager.java index 42f078233c5..9a658d7897d 100644 --- a/src/main/java/org/olat/repository/RepositoryManager.java +++ b/src/main/java/org/olat/repository/RepositoryManager.java @@ -33,6 +33,7 @@ import java.util.Collections; import java.util.Date; import java.util.List; +import javax.persistence.EntityManager; import javax.persistence.LockModeType; import javax.persistence.TypedQuery; @@ -496,7 +497,6 @@ public class RepositoryManager extends BasicManager { return entries.get(0); } - @Transactional(readOnly=true) public List<RepositoryEntry> lookupRepositoryEntries(Collection<Long> keys) { if (keys == null || keys.isEmpty()) { return Collections.emptyList(); @@ -522,7 +522,6 @@ public class RepositoryManager extends BasicManager { * @return the RepositorEntry or null if strict=false * @throws AssertException if the softkey could not be found (strict=true) */ - @Transactional(readOnly=true) public RepositoryEntry lookupRepositoryEntry(OLATResourceable resourceable, boolean strict) { OLATResource ores = (resourceable instanceof OLATResource) ? (OLATResource)resourceable : OLATResourceManager.getInstance().findResourceable(resourceable); @@ -730,7 +729,7 @@ public class RepositoryManager extends BasicManager { private RepositoryEntry loadForUpdate(RepositoryEntry re) { //first remove it from caches - DBFactory.getInstance().getCurrentEntityManager().detach(re); + dbInstance.getCurrentEntityManager().detach(re); StringBuilder query = new StringBuilder(); query.append("select v from ").append(RepositoryEntry.class.getName()).append(" as v ") @@ -739,8 +738,8 @@ public class RepositoryManager extends BasicManager { .append(" inner join fetch v.participantGroup as participantGroup") .append(" inner join fetch v.tutorGroup as tutorGroup")*/ .append(" where v.key=:repoKey"); - - RepositoryEntry entry = DBFactory.getInstance().getCurrentEntityManager().createQuery(query.toString(), RepositoryEntry.class) + + RepositoryEntry entry = dbInstance.getCurrentEntityManager().createQuery(query.toString(), RepositoryEntry.class) .setParameter("repoKey", re.getKey()) .setLockMode(LockModeType.PESSIMISTIC_WRITE) .getSingleResult(); @@ -781,7 +780,7 @@ public class RepositoryManager extends BasicManager { RepositoryEntry reloadedRe = loadForUpdate(re); if(reloadedRe == null) return null;//deleted - reloadedRe.incrementDownloadCounter(); + reloadedRe.setDownloadCounter(reloadedRe.getDownloadCounter() + 1); reloadedRe.setLastUsage(new Date()); updateLifeCycle(reloadedRe); RepositoryEntry updatedRe = DBFactory.getInstance().getCurrentEntityManager().merge(reloadedRe); -- GitLab