From 1a7a2ed5a2820c71cc4493a90eaace23b472b665 Mon Sep 17 00:00:00 2001 From: Daniel Gerhardt <code@dgerhardt.net> Date: Mon, 26 Nov 2018 16:55:51 +0100 Subject: [PATCH] Fix handling of content groups Partially reverts changes of 9d0255e771a91f60c450dd65e7fd8693b22bb584. Added helper methods to Room which transform the ContentGroup collection from Set to Map. Content IDs are now removed from groups when Contents are deleted. --- src/main/java/de/thm/arsnova/model/Room.java | 12 ++- .../arsnova/service/ContentServiceImpl.java | 91 ++++++++----------- .../thm/arsnova/service/RoomServiceImpl.java | 18 ++-- 3 files changed, 57 insertions(+), 64 deletions(-) diff --git a/src/main/java/de/thm/arsnova/model/Room.java b/src/main/java/de/thm/arsnova/model/Room.java index 652cf181c..d603c1c51 100644 --- a/src/main/java/de/thm/arsnova/model/Room.java +++ b/src/main/java/de/thm/arsnova/model/Room.java @@ -8,6 +8,8 @@ import java.util.HashSet; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; public class Room extends Entity { public static class ContentGroup { @@ -471,17 +473,25 @@ public class Room extends Entity { @JsonView({View.Persistence.class, View.Public.class}) public Set<ContentGroup> getContentGroups() { if (contentGroups == null) { - contentGroups = new HashSet<ContentGroup>(); + contentGroups = new HashSet<>(); } return contentGroups; } + public Map<String, ContentGroup> getContentGroupsAsMap() { + return getContentGroups().stream().collect(Collectors.toMap(ContentGroup::getName, Function.identity())); + } + @JsonView({View.Persistence.class, View.Public.class}) public void setContentGroups(final Set<ContentGroup> contentGroups) { this.contentGroups = contentGroups; } + public void setContentGroupsFromMap(final Map<String, ContentGroup> groups) { + this.contentGroups = new HashSet<>(groups.values()); + } + @JsonView({View.Persistence.class, View.Public.class}) public Settings getSettings() { if (settings == null) { diff --git a/src/main/java/de/thm/arsnova/service/ContentServiceImpl.java b/src/main/java/de/thm/arsnova/service/ContentServiceImpl.java index 1f31288f1..6bf706dc9 100644 --- a/src/main/java/de/thm/arsnova/service/ContentServiceImpl.java +++ b/src/main/java/de/thm/arsnova/service/ContentServiceImpl.java @@ -19,6 +19,7 @@ package de.thm.arsnova.service; import de.thm.arsnova.model.Content; import de.thm.arsnova.model.Room; +import de.thm.arsnova.model.Room.ContentGroup; import de.thm.arsnova.persistence.AnswerRepository; import de.thm.arsnova.persistence.ContentRepository; import de.thm.arsnova.persistence.LogEntryRepository; @@ -78,27 +79,16 @@ public class ContentServiceImpl extends DefaultEntityServiceImpl<Content> implem this.userService = userService; } - @Cacheable("contents") @Override - public Content get(final String id) { - try { - final Content content = super.get(id); - if (content.getFormat() != Content.Format.TEXT && 0 == content.getState().getRound()) { + protected void modifyRetrieved(final Content content) { + if (content.getFormat() != Content.Format.TEXT && 0 == content.getState().getRound()) { /* needed for legacy questions whose piRound property has not been set */ - content.getState().setRound(1); - } - //content.setSessionKeyword(roomRepository.getSessionFromId(content.getRoomId()).getKeyword()); - - Room room = roomService.get(content.getRoomId()); - content.setGroups(room.getContentGroups().stream() - .map(Room.ContentGroup::getName).collect(Collectors.toSet())); - - return content; - } catch (final DocumentNotFoundException e) { - logger.error("Could not get content {}.", id, e); + content.getState().setRound(1); } - return null; + Room room = roomService.get(content.getRoomId()); + content.setGroups(room.getContentGroups().stream() + .map(Room.ContentGroup::getName).filter(g -> !g.isEmpty()).collect(Collectors.toSet())); } /* FIXME: caching */ @@ -177,30 +167,23 @@ public class ContentServiceImpl extends DefaultEntityServiceImpl<Content> implem } @Override - public void finalizeCreate(final Content content) { + protected void finalizeCreate(final Content content) { /* Update content groups of room */ final Room room = roomService.get(content.getRoomId()); - final Set<Room.ContentGroup> contentGroups = room.getContentGroups(); - for (final Room.ContentGroup cg : contentGroups) { - if (content.getGroups().contains(cg.getName())) { - cg.getContentIds().add(content.getId()); - content.getGroups().remove(cg.getName()); - } - } - for (final String newContentGroups : content.getGroups()) { - Room.ContentGroup newGroup = new Room.ContentGroup(); - Set<String> newContentIds = new HashSet<String>(); - newContentIds.add(content.getId()); - newGroup.setName(newContentGroups); - newGroup.setAutoSort(true); - newGroup.setContentIds(newContentIds); - room.getContentGroups().add(newGroup); + final Map<String, Room.ContentGroup> groups = room.getContentGroupsAsMap(); + for (final String groupName : content.getGroups()) { + Room.ContentGroup group = groups.getOrDefault(groupName, new Room.ContentGroup()); + groups.put(groupName, group); + group.getContentIds().add(content.getId()); + group.setName(groupName); + group.setAutoSort(true); } + room.setContentGroupsFromMap(groups); roomService.update(room); } @Override - public void prepareUpdate(final Content content) { + protected void prepareUpdate(final Content content) { final User user = userService.getCurrentUser(); final Content oldContent = get(content.getId()); if (null == oldContent) { @@ -223,29 +206,24 @@ public class ContentServiceImpl extends DefaultEntityServiceImpl<Content> implem } @Override - public void finalizeUpdate(final Content content) { + protected void finalizeUpdate(final Content content) { /* Update content groups of room */ final Room room = roomService.get(content.getRoomId()); - for (final Room.ContentGroup cg : room.getContentGroups()) { - if (content.getGroups().contains(cg.getName())) { - cg.getContentIds().add(content.getId()); - content.getGroups().remove(cg.getName()); + final Set<String> contentsGroupNames = content.getGroups(); + final Set<String> allGroupNames = new HashSet<>(contentsGroupNames); + final Map<String, Room.ContentGroup> groups = room.getContentGroupsAsMap(); + allGroupNames.addAll(groups.keySet()); + for (final String groupName : allGroupNames) { + Room.ContentGroup group = groups.getOrDefault(groupName, new Room.ContentGroup()); + if (contentsGroupNames.contains(groupName)) { + group.getContentIds().add(content.getId()); + group.setName(groupName); + group.setAutoSort(true); } else { - cg.getContentIds().remove(content.getId()); - if (cg.getContentIds().isEmpty()) { - room.getContentGroups().remove(cg); - } + group.getContentIds().remove(content.getId()); } } - for (final String newContentGroups : content.getGroups()) { - Room.ContentGroup newGroup = new Room.ContentGroup(); - Set<String> newContentIds = new HashSet<String>(); - newContentIds.add(content.getId()); - newGroup.setName(newContentGroups); - newGroup.setAutoSort(true); - newGroup.setContentIds(newContentIds); - room.getContentGroups().add(newGroup); - } + room.setContentGroupsFromMap(groups); roomService.update(room); /* TODO: not sure yet how to refactor this code - we need access to the old and new entity @@ -259,6 +237,15 @@ public class ContentServiceImpl extends DefaultEntityServiceImpl<Content> implem */ } + @Override + protected void prepareDelete(Content content) { + final Room room = roomService.get(content.getRoomId()); + for (ContentGroup group : room.getContentGroups()) { + group.getContentIds().remove(content.getId()); + } + roomService.update(room); + } + /* TODO: Only evict cache entry for the content's session. This requires some refactoring. */ @Override @PreAuthorize("hasPermission(#contentId, 'content', 'owner')") diff --git a/src/main/java/de/thm/arsnova/service/RoomServiceImpl.java b/src/main/java/de/thm/arsnova/service/RoomServiceImpl.java index 6d9b4fef6..7aae870b9 100644 --- a/src/main/java/de/thm/arsnova/service/RoomServiceImpl.java +++ b/src/main/java/de/thm/arsnova/service/RoomServiceImpl.java @@ -174,33 +174,29 @@ public class RoomServiceImpl extends DefaultEntityServiceImpl<Room> implements R } /** - * Fetches the room. - * Adds content from this room that is in no content group to a no-name group. - * @param id room id to fetch - * @return Room with no-name content group + * Adds a default content group with contents that have no other content group assigned. + * + * @param room The Room to be modified */ @Override - public Room get(final String id) { - Room r = super.get(id); + protected void modifyRetrieved(final Room room) { // creates a set from all room content groups - Set<String> cIdsWithGroup = r.getContentGroups().stream() + Set<String> cIdsWithGroup = room.getContentGroups().stream() .map(Room.ContentGroup::getContentIds) .flatMap(ids -> ids.stream()) .collect(Collectors.toSet()); - Set<String> cIds = new HashSet<String>(contentRepository.findIdsByRoomId(id)); + Set<String> cIds = new HashSet<>(contentRepository.findIdsByRoomId(room.getId())); cIds.removeAll(cIdsWithGroup); if (!cIds.isEmpty()) { - Set<Room.ContentGroup> cgs = r.getContentGroups(); + Set<Room.ContentGroup> cgs = room.getContentGroups(); Room.ContentGroup defaultGroup = new Room.ContentGroup(); defaultGroup.setContentIds(cIds); defaultGroup.setAutoSort(true); defaultGroup.setName(""); cgs.add(defaultGroup); } - - return r; } @Override -- GitLab