Skip to content
Snippets Groups Projects
Commit d8598fb6 authored by Daniel Gerhardt's avatar Daniel Gerhardt
Browse files

Improve creation/update implementation for answers

* Use EntityService for /v2 API
* Always use queue and bulk saving
* Implement access control for CRUD
parent 1426cfce
Branches
No related merge requests found
...@@ -32,6 +32,7 @@ import de.thm.arsnova.service.TimerService; ...@@ -32,6 +32,7 @@ import de.thm.arsnova.service.TimerService;
import de.thm.arsnova.util.PaginationListDecorator; import de.thm.arsnova.util.PaginationListDecorator;
import de.thm.arsnova.web.DeprecatedApi; import de.thm.arsnova.web.DeprecatedApi;
import de.thm.arsnova.web.Pagination; 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.ForbiddenException;
import de.thm.arsnova.web.exceptions.NoContentException; import de.thm.arsnova.web.exceptions.NoContentException;
import de.thm.arsnova.web.exceptions.NotFoundException; import de.thm.arsnova.web.exceptions.NotFoundException;
...@@ -513,11 +514,14 @@ public class ContentController extends PaginationController { ...@@ -513,11 +514,14 @@ public class ContentController extends PaginationController {
) { ) {
final de.thm.arsnova.model.Content content = contentService.get(contentId); final de.thm.arsnova.model.Content content = contentService.get(contentId);
final de.thm.arsnova.model.Answer answerV3 = fromV2Migrator.migrate(answer, content); 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) { if (answerV3 instanceof TextAnswer) {
return toV2Migrator.migrate((TextAnswer) answerService.saveAnswer(contentId, answerV3)); return toV2Migrator.migrate((TextAnswer) answerService.create(answerV3));
} else { } 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 { ...@@ -534,9 +538,9 @@ public class ContentController extends PaginationController {
final de.thm.arsnova.model.Answer answerV3 = fromV2Migrator.migrate(answer, content); final de.thm.arsnova.model.Answer answerV3 = fromV2Migrator.migrate(answer, content);
if (answerV3 instanceof TextAnswer) { if (answerV3 instanceof TextAnswer) {
return toV2Migrator.migrate((TextAnswer) answerService.updateAnswer(answerV3)); return toV2Migrator.migrate((TextAnswer) answerService.update(answerV3));
} else { } else {
return toV2Migrator.migrate((ChoiceAnswer) answerService.updateAnswer(answerV3), (ChoiceQuestionContent) content); return toV2Migrator.migrate((ChoiceAnswer) answerService.update(answerV3), (ChoiceQuestionContent) content);
} }
} }
......
/*
* 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;
}
}
...@@ -17,11 +17,13 @@ ...@@ -17,11 +17,13 @@
*/ */
package de.thm.arsnova.security; package de.thm.arsnova.security;
import de.thm.arsnova.model.Answer;
import de.thm.arsnova.model.Comment; import de.thm.arsnova.model.Comment;
import de.thm.arsnova.model.Content; import de.thm.arsnova.model.Content;
import de.thm.arsnova.model.Motd; import de.thm.arsnova.model.Motd;
import de.thm.arsnova.model.Room; import de.thm.arsnova.model.Room;
import de.thm.arsnova.model.UserProfile; import de.thm.arsnova.model.UserProfile;
import de.thm.arsnova.persistence.AnswerRepository;
import de.thm.arsnova.persistence.CommentRepository; import de.thm.arsnova.persistence.CommentRepository;
import de.thm.arsnova.persistence.ContentRepository; import de.thm.arsnova.persistence.ContentRepository;
import de.thm.arsnova.persistence.MotdRepository; import de.thm.arsnova.persistence.MotdRepository;
...@@ -58,6 +60,9 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator { ...@@ -58,6 +60,9 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator {
@Autowired @Autowired
private ContentRepository contentRepository; private ContentRepository contentRepository;
@Autowired
private AnswerRepository answerRepository;
@Autowired @Autowired
private MotdRepository motdRepository; private MotdRepository motdRepository;
...@@ -80,6 +85,8 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator { ...@@ -80,6 +85,8 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator {
&& hasRoomPermission(userId, ((Room) targetDomainObject), permission.toString())) && hasRoomPermission(userId, ((Room) targetDomainObject), permission.toString()))
|| (targetDomainObject instanceof Content || (targetDomainObject instanceof Content
&& hasContentPermission(userId, ((Content) targetDomainObject), permission.toString())) && hasContentPermission(userId, ((Content) targetDomainObject), permission.toString()))
|| (targetDomainObject instanceof Answer
&& hasAnswerPermission(userId, ((Answer) targetDomainObject), permission.toString()))
|| (targetDomainObject instanceof Comment || (targetDomainObject instanceof Comment
&& hasCommentPermission(userId, ((Comment) targetDomainObject), permission.toString())) && hasCommentPermission(userId, ((Comment) targetDomainObject), permission.toString()))
|| (targetDomainObject instanceof Motd || (targetDomainObject instanceof Motd
...@@ -113,6 +120,9 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator { ...@@ -113,6 +120,9 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator {
case "content": case "content":
final Content targetContent = contentRepository.findOne(targetId.toString()); final Content targetContent = contentRepository.findOne(targetId.toString());
return targetContent != null && hasContentPermission(userId, targetContent, permission.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": case "comment":
final Comment targetComment = commentRepository.findOne(targetId.toString()); final Comment targetComment = commentRepository.findOne(targetId.toString());
return targetComment != null && hasCommentPermission(userId, targetComment, permission.toString()); return targetComment != null && hasCommentPermission(userId, targetComment, permission.toString());
...@@ -178,6 +188,37 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator { ...@@ -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( private boolean hasCommentPermission(
final String userId, final String userId,
final Comment targetComment, final Comment targetComment,
......
...@@ -38,9 +38,9 @@ public interface AnswerService extends EntityService<Answer> { ...@@ -38,9 +38,9 @@ public interface AnswerService extends EntityService<Answer> {
void deleteAnswers(String contentId); 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); Map<String, Object> countAnswersAndAbstentionsInternal(String contentId);
......
...@@ -26,7 +26,6 @@ import de.thm.arsnova.model.ChoiceQuestionContent; ...@@ -26,7 +26,6 @@ import de.thm.arsnova.model.ChoiceQuestionContent;
import de.thm.arsnova.model.Content; import de.thm.arsnova.model.Content;
import de.thm.arsnova.model.Room; import de.thm.arsnova.model.Room;
import de.thm.arsnova.model.TextAnswer; import de.thm.arsnova.model.TextAnswer;
import de.thm.arsnova.model.transport.AnswerQueueElement;
import de.thm.arsnova.persistence.AnswerRepository; import de.thm.arsnova.persistence.AnswerRepository;
import de.thm.arsnova.security.User; import de.thm.arsnova.security.User;
import de.thm.arsnova.web.exceptions.NotFoundException; import de.thm.arsnova.web.exceptions.NotFoundException;
...@@ -36,7 +35,6 @@ import org.slf4j.Logger; ...@@ -36,7 +35,6 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.cache.annotation.CacheEvict;
import org.springframework.context.event.EventListener; import org.springframework.context.event.EventListener;
import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
import org.springframework.scheduling.annotation.Scheduled; import org.springframework.scheduling.annotation.Scheduled;
...@@ -58,7 +56,7 @@ import java.util.concurrent.ConcurrentLinkedQueue; ...@@ -58,7 +56,7 @@ import java.util.concurrent.ConcurrentLinkedQueue;
public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implements AnswerService { public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implements AnswerService {
private static final Logger logger = LoggerFactory.getLogger(ContentServiceImpl.class); 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 RoomService roomService;
private ContentService contentService; private ContentService contentService;
...@@ -88,21 +86,18 @@ public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implemen ...@@ -88,21 +86,18 @@ public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implemen
return; return;
} }
final List<Answer> answerList = new ArrayList<>(); final List<Answer> answers = new ArrayList<>();
final List<AnswerQueueElement> elements = new ArrayList<>(); Answer entry;
AnswerQueueElement entry;
while ((entry = this.answerQueue.poll()) != null) { while ((entry = this.answerQueue.poll()) != null) {
final Answer answer = entry.getAnswer(); answers.add(entry);
answerList.add(answer);
elements.add(entry);
} }
try { try {
for (AnswerQueueElement e : elements) { for (Answer e : answers) {
this.eventPublisher.publishEvent(new BeforeCreationEvent<>(this, e.getAnswer())); this.eventPublisher.publishEvent(new BeforeCreationEvent<>(this, e));
} }
answerRepository.saveAll(answerList); answerRepository.saveAll(answers);
for (AnswerQueueElement e : elements) { for (Answer e : answers) {
this.eventPublisher.publishEvent(new AfterCreationEvent<>(this, e.getAnswer())); this.eventPublisher.publishEvent(new AfterCreationEvent<>(this, e));
} }
} catch (final DbAccessException e) { } catch (final DbAccessException e) {
logger.error("Could not bulk save answers from queue.", e); logger.error("Could not bulk save answers from queue.", e);
...@@ -314,19 +309,24 @@ public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implemen ...@@ -314,19 +309,24 @@ public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implemen
} }
@Override @Override
@PreAuthorize("isAuthenticated()") @PreAuthorize("isAuthenticated() && hasPermission(#answer, 'create')")
@CacheEvict(value = "answerlists", key = "#contentId") public Answer create(final Answer answer) {
public Answer saveAnswer(final String contentId, 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 User user = userService.getCurrentUser();
final Content content = contentService.get(contentId); final Content content = contentService.get(answer.getContentId());
if (content == null) { if (content == null) {
throw new NotFoundException(); throw new NotFoundException();
} }
final Room room = roomService.get(content.getRoomId());
answer.setCreatorId(user.getId()); answer.setCreatorId(user.getId());
answer.setContentId(content.getId()); answer.setRoomId(content.getRoomId());
answer.setRoomId(room.getId());
/* FIXME: migrate /* FIXME: migrate
answer.setQuestionValue(content.calculateValue(answer)); answer.setQuestionValue(content.calculateValue(answer));
...@@ -349,17 +349,10 @@ public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implemen ...@@ -349,17 +349,10 @@ public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implemen
} else { } else {
answer.setRound(content.getState().getRound()); 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 @Override
@PreAuthorize("isAuthenticated()") protected void prepareUpdate(final Answer answer) {
@CacheEvict(value = "answerlists", allEntries = true)
public Answer updateAnswer(final Answer answer) {
final User user = userService.getCurrentUser(); final User user = userService.getCurrentUser();
final Answer realAnswer = this.getMyAnswer(answer.getContentId()); final Answer realAnswer = this.getMyAnswer(answer.getContentId());
if (user == null || realAnswer == null || !user.getId().equals(realAnswer.getCreatorId())) { if (user == null || realAnswer == null || !user.getId().equals(realAnswer.getCreatorId())) {
...@@ -377,9 +370,6 @@ public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implemen ...@@ -377,9 +370,6 @@ public class AnswerServiceImpl extends DefaultEntityServiceImpl<Answer> implemen
answer.setCreatorId(user.getId()); answer.setCreatorId(user.getId());
answer.setContentId(content.getId()); answer.setContentId(content.getId());
answer.setRoomId(room.getId()); answer.setRoomId(room.getId());
update(realAnswer);
return answer;
} }
/* /*
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment