From d8598fb6ef80ed2a4d23b6adfc4ed25178beb46d Mon Sep 17 00:00:00 2001
From: Daniel Gerhardt <code@dgerhardt.net>
Date: Tue, 22 Jan 2019 11:03:12 +0100
Subject: [PATCH] Improve creation/update implementation for answers
* Use EntityService for /v2 API
* Always use queue and bulk saving
* Implement access control for CRUD
---
.../controller/v2/ContentController.java | 12 ++--
.../model/transport/AnswerQueueElement.java | 60 -------------------
.../ApplicationPermissionEvaluator.java | 41 +++++++++++++
.../de/thm/arsnova/service/AnswerService.java | 4 +-
.../arsnova/service/AnswerServiceImpl.java | 54 +++++++----------
5 files changed, 73 insertions(+), 98 deletions(-)
delete mode 100644 src/main/java/de/thm/arsnova/model/transport/AnswerQueueElement.java
diff --git a/src/main/java/de/thm/arsnova/controller/v2/ContentController.java b/src/main/java/de/thm/arsnova/controller/v2/ContentController.java
index d5c7c1db6..d600d8205 100644
--- a/src/main/java/de/thm/arsnova/controller/v2/ContentController.java
+++ b/src/main/java/de/thm/arsnova/controller/v2/ContentController.java
@@ -32,6 +32,7 @@ import de.thm.arsnova.service.TimerService;
import de.thm.arsnova.util.PaginationListDecorator;
import de.thm.arsnova.web.DeprecatedApi;
import de.thm.arsnova.web.Pagination;
+import de.thm.arsnova.web.exceptions.BadRequestException;
import de.thm.arsnova.web.exceptions.ForbiddenException;
import de.thm.arsnova.web.exceptions.NoContentException;
import de.thm.arsnova.web.exceptions.NotFoundException;
@@ -513,11 +514,14 @@ public class ContentController extends PaginationController {
) {
final de.thm.arsnova.model.Content content = contentService.get(contentId);
final de.thm.arsnova.model.Answer answerV3 = fromV2Migrator.migrate(answer, content);
+ if (!contentId.equals(answerV3.getContentId())) {
+ throw new BadRequestException("Mismatching content IDs.");
+ }
if (answerV3 instanceof TextAnswer) {
- return toV2Migrator.migrate((TextAnswer) answerService.saveAnswer(contentId, answerV3));
+ return toV2Migrator.migrate((TextAnswer) answerService.create(answerV3));
} else {
- return toV2Migrator.migrate((ChoiceAnswer) answerService.saveAnswer(contentId, answerV3), (ChoiceQuestionContent) content);
+ return toV2Migrator.migrate((ChoiceAnswer) answerService.create(answerV3), (ChoiceQuestionContent) content);
}
}
@@ -534,9 +538,9 @@ public class ContentController extends PaginationController {
final de.thm.arsnova.model.Answer answerV3 = fromV2Migrator.migrate(answer, content);
if (answerV3 instanceof TextAnswer) {
- return toV2Migrator.migrate((TextAnswer) answerService.updateAnswer(answerV3));
+ return toV2Migrator.migrate((TextAnswer) answerService.update(answerV3));
} else {
- return toV2Migrator.migrate((ChoiceAnswer) answerService.updateAnswer(answerV3), (ChoiceQuestionContent) content);
+ return toV2Migrator.migrate((ChoiceAnswer) answerService.update(answerV3), (ChoiceQuestionContent) content);
}
}
diff --git a/src/main/java/de/thm/arsnova/model/transport/AnswerQueueElement.java b/src/main/java/de/thm/arsnova/model/transport/AnswerQueueElement.java
deleted file mode 100644
index dfe4c2b8b..000000000
--- a/src/main/java/de/thm/arsnova/model/transport/AnswerQueueElement.java
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * This file is part of ARSnova Backend.
- * Copyright (C) 2012-2018 The ARSnova Team and Contributors
- *
- * ARSnova Backend is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * ARSnova Backend is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- */
-package de.thm.arsnova.model.transport;
-
-import de.thm.arsnova.model.Answer;
-import de.thm.arsnova.model.Content;
-import de.thm.arsnova.model.Room;
-
-/**
- * An answer that is about to get saved in the database. Answers are not saved immediately, they are instead stored
- * in a queue that is cleared at specific intervals.
- */
-public class AnswerQueueElement {
-
- private final Room room;
-
- private final Content content;
-
- private final Answer answer;
-
- private final String userId;
-
- public AnswerQueueElement(Room room, Content content, Answer answer, String userId) {
- this.room = room;
- this.content = content;
- this.answer = answer;
- this.userId = userId;
- }
-
- public Room getRoom() {
- return room;
- }
-
- public Content getQuestion() {
- return content;
- }
-
- public Answer getAnswer() {
- return answer;
- }
-
- public String getUserId() {
- return userId;
- }
-}
diff --git a/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java b/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java
index acf6a9e92..028fab785 100644
--- a/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java
+++ b/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java
@@ -17,11 +17,13 @@
*/
package de.thm.arsnova.security;
+import de.thm.arsnova.model.Answer;
import de.thm.arsnova.model.Comment;
import de.thm.arsnova.model.Content;
import de.thm.arsnova.model.Motd;
import de.thm.arsnova.model.Room;
import de.thm.arsnova.model.UserProfile;
+import de.thm.arsnova.persistence.AnswerRepository;
import de.thm.arsnova.persistence.CommentRepository;
import de.thm.arsnova.persistence.ContentRepository;
import de.thm.arsnova.persistence.MotdRepository;
@@ -58,6 +60,9 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator {
@Autowired
private ContentRepository contentRepository;
+ @Autowired
+ private AnswerRepository answerRepository;
+
@Autowired
private MotdRepository motdRepository;
@@ -80,6 +85,8 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator {
&& hasRoomPermission(userId, ((Room) targetDomainObject), permission.toString()))
|| (targetDomainObject instanceof Content
&& hasContentPermission(userId, ((Content) targetDomainObject), permission.toString()))
+ || (targetDomainObject instanceof Answer
+ && hasAnswerPermission(userId, ((Answer) targetDomainObject), permission.toString()))
|| (targetDomainObject instanceof Comment
&& hasCommentPermission(userId, ((Comment) targetDomainObject), permission.toString()))
|| (targetDomainObject instanceof Motd
@@ -113,6 +120,9 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator {
case "content":
final Content targetContent = contentRepository.findOne(targetId.toString());
return targetContent != null && hasContentPermission(userId, targetContent, permission.toString());
+ case "answer":
+ final Answer targetAnswer = answerRepository.findOne(targetId.toString());
+ return targetAnswer != null && hasAnswerPermission(userId, targetAnswer, permission.toString());
case "comment":
final Comment targetComment = commentRepository.findOne(targetId.toString());
return targetComment != null && hasCommentPermission(userId, targetComment, permission.toString());
@@ -178,6 +188,37 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator {
}
}
+ private boolean hasAnswerPermission(
+ final String userId,
+ final Answer targetAnswer,
+ final String permission) {
+ final Content content = contentRepository.findOne(targetAnswer.getContentId());
+ if (!hasContentPermission(userId, content, "read")) {
+ return false;
+ }
+ Room room;
+ switch (permission) {
+ case "read":
+ if (targetAnswer.getCreatorId().equals(userId) || content.getState().isResponsesVisible()) {
+ return true;
+ }
+ room = roomRepository.findOne(targetAnswer.getRoomId());
+ return room != null && hasRoomPermission(userId, room, "owner");
+ case "create":
+ return content.getState().isResponsesEnabled();
+ case "owner":
+ return targetAnswer.getCreatorId().equals(userId);
+ case "update":
+ /* TODO */
+ return false;
+ case "delete":
+ room = roomRepository.findOne(targetAnswer.getRoomId());
+ return room != null && hasRoomPermission(userId, room, "owner");
+ default:
+ return false;
+ }
+ }
+
private boolean hasCommentPermission(
final String userId,
final Comment targetComment,
diff --git a/src/main/java/de/thm/arsnova/service/AnswerService.java b/src/main/java/de/thm/arsnova/service/AnswerService.java
index 01a1a6fc3..cbfb4fa0f 100644
--- a/src/main/java/de/thm/arsnova/service/AnswerService.java
+++ b/src/main/java/de/thm/arsnova/service/AnswerService.java
@@ -38,9 +38,9 @@ public interface AnswerService extends EntityService<Answer> {
void deleteAnswers(String contentId);
- Answer saveAnswer(String contentId, Answer answer);
+ Answer create(Answer answer);
- Answer updateAnswer(Answer answer);
+ Answer update(Answer answer);
Map<String, Object> countAnswersAndAbstentionsInternal(String contentId);
diff --git a/src/main/java/de/thm/arsnova/service/AnswerServiceImpl.java b/src/main/java/de/thm/arsnova/service/AnswerServiceImpl.java
index 070bebb45..ceccd9b48 100644
--- a/src/main/java/de/thm/arsnova/service/AnswerServiceImpl.java
+++ b/src/main/java/de/thm/arsnova/service/AnswerServiceImpl.java
@@ -26,7 +26,6 @@ import de.thm.arsnova.model.ChoiceQuestionContent;
import de.thm.arsnova.model.Content;
import de.thm.arsnova.model.Room;
import de.thm.arsnova.model.TextAnswer;
-import de.thm.arsnova.model.transport.AnswerQueueElement;
import de.thm.arsnova.persistence.AnswerRepository;
import de.thm.arsnova.security.User;
import de.thm.arsnova.web.exceptions.NotFoundException;
@@ -36,7 +35,6 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
-import org.springframework.cache.annotation.CacheEvict;
import org.springframework.context.event.EventListener;
import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
import org.springframework.scheduling.annotation.Scheduled;
@@ -58,7 +56,7 @@ import java.util.concurrent.ConcurrentLinkedQueue;
public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implements AnswerService {
private static final Logger logger = LoggerFactory.getLogger(ContentServiceImpl.class);
- private final Queue<AnswerQueueElement> answerQueue = new ConcurrentLinkedQueue<>();
+ private final Queue<Answer> answerQueue = new ConcurrentLinkedQueue<>();
private RoomService roomService;
private ContentService contentService;
@@ -88,21 +86,18 @@ public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implemen
return;
}
- final List<Answer> answerList = new ArrayList<>();
- final List<AnswerQueueElement> elements = new ArrayList<>();
- AnswerQueueElement entry;
+ final List<Answer> answers = new ArrayList<>();
+ Answer entry;
while ((entry = this.answerQueue.poll()) != null) {
- final Answer answer = entry.getAnswer();
- answerList.add(answer);
- elements.add(entry);
+ answers.add(entry);
}
try {
- for (AnswerQueueElement e : elements) {
- this.eventPublisher.publishEvent(new BeforeCreationEvent<>(this, e.getAnswer()));
+ for (Answer e : answers) {
+ this.eventPublisher.publishEvent(new BeforeCreationEvent<>(this, e));
}
- answerRepository.saveAll(answerList);
- for (AnswerQueueElement e : elements) {
- this.eventPublisher.publishEvent(new AfterCreationEvent<>(this, e.getAnswer()));
+ answerRepository.saveAll(answers);
+ for (Answer e : answers) {
+ this.eventPublisher.publishEvent(new AfterCreationEvent<>(this, e));
}
} catch (final DbAccessException e) {
logger.error("Could not bulk save answers from queue.", e);
@@ -314,19 +309,24 @@ public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implemen
}
@Override
- @PreAuthorize("isAuthenticated()")
- @CacheEvict(value = "answerlists", key = "#contentId")
- public Answer saveAnswer(final String contentId, final Answer answer) {
+ @PreAuthorize("isAuthenticated() && hasPermission(#answer, 'create')")
+ public Answer create(final Answer answer) {
+ this.answerQueue.offer(answer);
+
+ return answer;
+ }
+
+ @Override
+ protected void prepareCreate(final Answer answer) {
+ /* TODO: prevent multiple answers from one user */
final User user = userService.getCurrentUser();
- final Content content = contentService.get(contentId);
+ final Content content = contentService.get(answer.getContentId());
if (content == null) {
throw new NotFoundException();
}
- final Room room = roomService.get(content.getRoomId());
answer.setCreatorId(user.getId());
- answer.setContentId(content.getId());
- answer.setRoomId(room.getId());
+ answer.setRoomId(content.getRoomId());
/* FIXME: migrate
answer.setQuestionValue(content.calculateValue(answer));
@@ -349,17 +349,10 @@ public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implemen
} else {
answer.setRound(content.getState().getRound());
}
-
- this.answerQueue.offer(new AnswerQueueElement(room, content, answer, user.getId()));
-
- return answer;
}
- /* FIXME: Remove, this should be handled by EntityService! */
@Override
- @PreAuthorize("isAuthenticated()")
- @CacheEvict(value = "answerlists", allEntries = true)
- public Answer updateAnswer(final Answer answer) {
+ protected void prepareUpdate(final Answer answer) {
final User user = userService.getCurrentUser();
final Answer realAnswer = this.getMyAnswer(answer.getContentId());
if (user == null || realAnswer == null || !user.getId().equals(realAnswer.getCreatorId())) {
@@ -377,9 +370,6 @@ public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implemen
answer.setCreatorId(user.getId());
answer.setContentId(content.getId());
answer.setRoomId(room.getId());
- update(realAnswer);
-
- return answer;
}
/*
--
GitLab