Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/refactor can modify vote to validator service #334

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package org.rulez.demokracia.pdengine.choice;

import org.rulez.demokracia.pdengine.dataobjects.VoteAdminInfo;
import org.rulez.demokracia.pdengine.vote.Vote;
import org.springframework.stereotype.Service;

@Service
public interface ChoiceModificationValidatorService {

void validateModification(
VoteAdminInfo voteAdminInfo, Vote vote, Choice choice
);

void validateDeletion(VoteAdminInfo voteAdminInfo, Vote vote, Choice choice);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package org.rulez.demokracia.pdengine.choice;

import org.rulez.demokracia.pdengine.authentication.AuthenticatedUserService;
import org.rulez.demokracia.pdengine.dataobjects.VoteAdminInfo;
import org.rulez.demokracia.pdengine.exception.ReportedException;
import org.rulez.demokracia.pdengine.vote.Vote;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;

@Service
public class ChoiceModificationValidatorServiceImpl
implements ChoiceModificationValidatorService {

private static final String MODIFICATION = "modification";
private static final String DELETION = "deletion";
private static final String CAN_ADDIN_IS_FALSE_MESSAGE =
"Choice %s disallowed: adminKey is user, but canAddin is false";
private static final String DIFFERENT_USER_MESSAGE =
"Choice %s disallowed: adminKey is user, and the choice was added by a different user";
@Autowired
private AuthenticatedUserService authenticationService;

@Override
public void validateModification(
final VoteAdminInfo voteAdminInfo, final Vote vote, final Choice choice
) {
validate(voteAdminInfo, vote, choice, MODIFICATION);
}

@Override
public void validateDeletion(
final VoteAdminInfo voteAdminInfo, final Vote vote, final Choice choice
) {
validate(voteAdminInfo, vote, choice, DELETION);
}

private void validate(
final VoteAdminInfo voteAdminInfo,
final Vote vote,
final Choice choice,
final String operation
) {
if (voteAdminInfo.isUserAdminKey()) {
validateAddinability(vote, operation);
validateMatchingUser(choice, operation);
}
}

private void
validateMatchingUser(final Choice choice, final String operation) {
if (!choice.getUserName().equals(getUserName()))
throw new ReportedException(
String.format(DIFFERENT_USER_MESSAGE, operation)
);
}

private void validateAddinability(final Vote vote, final String operation) {
if (!vote.getParameters().isAddinable())
throw new ReportedException(
String.format(CAN_ADDIN_IS_FALSE_MESSAGE, operation)
);
}

private String getUserName() {
return authenticationService.getAuthenticatedUserName();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,8 @@ public class ChoiceServiceImpl implements ChoiceService {

private static final String USER = "user";

private static final String MODIFY_NOT_SAME_USER_MESSAGE =
"Choice modification disallowed: adminKey is user, "
+ "and the choice was added by a different user";

private static final String MODIFY_CANT_ADDIN_MESSAGE =
"Choice modification disallowed: adminKey is user, but canAddin is false";

private static final String DELETE_CANT_ADDIN = "The adminKey is \"user\" but canAddin is false.";

private static final String DELETE_NOT_SAME_USER_MESSAGE =
"The adminKey is \"user\" but the user is not same with that user who added the choice.";
@Autowired
private ChoiceModificationValidatorService choiceModificationValidatorService;

@Autowired
private VoteService voteService;
Expand All @@ -32,8 +23,10 @@ public class ChoiceServiceImpl implements ChoiceService {
private AuthenticatedUserService authenticatedUserService;

@Override
public Choice addChoice(final VoteAdminInfo voteAdminInfo, final String choiceName,
final String user) {
public Choice addChoice(
final VoteAdminInfo voteAdminInfo, final String choiceName,
final String user
) {
final Vote vote = getModifiableVote(voteAdminInfo, voteService);
final Choice choice = new Choice(choiceName, user);
vote.addChoice(choice);
Expand All @@ -47,8 +40,10 @@ public Choice getChoice(final String voteId, final String choiceId) {
}

@Override
public void endorseChoice(final VoteAdminInfo voteAdminInfo, final String choiceId,
final String givenUserName) {
public void endorseChoice(
final VoteAdminInfo voteAdminInfo, final String choiceId,
final String givenUserName
) {
String userName = givenUserName;
final Vote vote = voteService.getVote(voteAdminInfo.voteId);
if (USER.equals(voteAdminInfo.adminKey)) {
Expand All @@ -60,39 +55,34 @@ public void endorseChoice(final VoteAdminInfo voteAdminInfo, final String choice
}

@Override
public void deleteChoice(final VoteAdminInfo voteAdminInfo, final String choiceId) {
public void
deleteChoice(final VoteAdminInfo voteAdminInfo, final String choiceId) {
final Vote vote = getModifiableVote(voteAdminInfo, voteService);
final Choice votesChoice = getChoice(voteAdminInfo.getVoteId(), choiceId);

if (voteAdminInfo.isUserAdminKey())
deleteChoiceAsUser(vote, votesChoice);
else
vote.getChoices().remove(votesChoice.getId());
choiceModificationValidatorService
.validateDeletion(voteAdminInfo, vote, votesChoice);

vote.getChoices().remove(votesChoice.getId());

voteService.saveVote(vote);
}

@Override
public void modifyChoice(final VoteAdminInfo adminInfo, final String choiceId,
final String choiceName) {
public void modifyChoice(
final VoteAdminInfo adminInfo, final String choiceId,
final String choiceName
) {
final Vote vote = getModifiableVote(adminInfo, voteService);

final Choice votesChoice = vote.getChoice(choiceId);
validateModifyInput(adminInfo, vote, votesChoice);
choiceModificationValidatorService
.validateModification(adminInfo, vote, votesChoice);

votesChoice.setName(choiceName);
voteService.saveVote(vote);
}

private void validateModifyInput(final VoteAdminInfo adminInfo, final Vote vote,
final Choice votesChoice) {
if (!USER.equals(adminInfo.adminKey))
return;
checkIfVoteIsAddinable(vote, new ReportedException(MODIFY_CANT_ADDIN_MESSAGE));
checkIfUserIsTheSame(votesChoice, getUserName(),
new ReportedException(MODIFY_NOT_SAME_USER_MESSAGE, votesChoice.getUserName()));
}

private String getUserName() {
return authenticatedUserService.getAuthenticatedUserName();
}
Expand All @@ -102,11 +92,4 @@ private void checkIfVoteIsEndorseable(final Vote vote) {
throw new ReportedException("user cannot endorse this vote");
}

private void deleteChoiceAsUser(final Vote vote, final Choice votesChoice) {
checkIfUserIsTheSame(votesChoice, getUserName(),
new IllegalArgumentException(DELETE_NOT_SAME_USER_MESSAGE));
checkIfVoteIsAddinable(vote, new IllegalArgumentException(DELETE_CANT_ADDIN));
vote.getChoices().remove(votesChoice.getId());
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.rulez.demokracia.pdengine.choice;

import static org.mockito.Mockito.*;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -9,12 +10,17 @@
import org.rulez.demokracia.pdengine.annotations.TestedFeature;
import org.rulez.demokracia.pdengine.annotations.TestedOperation;
import org.rulez.demokracia.pdengine.dataobjects.VoteAdminInfo;
import org.rulez.demokracia.pdengine.exception.ReportedException;

@TestedFeature("Manage votes")
@TestedOperation("delete choice")
@RunWith(MockitoJUnitRunner.class)
public class ChoiceDeleteAdminKeyIsUserTest extends ChoiceTestBase {

private static final String NOT_SAME_USER_MESSAGE =
"The adminKey is \"user\" but the user is not same with that user who added the choice.";
private static final String CAN_ADDIN_IS_FALSE_MESSAGE =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't those strings are constants in the production code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are. If you mean, we should use constants of the production code, I suggest to forget it.
Let's imagine the situation, when someone accidentally removes some words from this string or an unlucky regex overwrites it. We wouldn't realize it, since we compare this constant to itself.

"The adminKey is \"user\" but canAddin is false.";
private static final String IF_THE_VOTE_HAS_BALLOTS_ISSUED =
"if the vote has ballots issued, the choice cannot be deleted";
private static final String IF_USER_IS_USED_AS_ADMIN_KEY =
Expand All @@ -33,52 +39,84 @@ public void setUp() {
@Test
public void if_canAddin_is_false_then_other_users_cannot_delete_choices() {
final Choice choiceToDelete = createChoice(TEST_USER_NAME, false);
when(authService.getAuthenticatedUserName()).thenReturn(TEST_USER_NAME);
assertThrows(() -> choiceService.deleteChoice(new VoteAdminInfo(vote.getId(), USER),
choiceToDelete.getId())).assertMessageIs("The adminKey is \"user\" but canAddin is false.");
final VoteAdminInfo voteAdminInfo = new VoteAdminInfo(vote.getId(), USER);
doThrow(new ReportedException(CAN_ADDIN_IS_FALSE_MESSAGE))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Service stub, should be put into a stub provider.

.when(choiceModificationValidatorService)
.validateDeletion(voteAdminInfo, vote, choiceToDelete);
assertThrows(
() -> choiceService.deleteChoice(
voteAdminInfo,
choiceToDelete.getId()
)
).assertMessageIs("The adminKey is \"user\" but canAddin is false.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once a java constant, once an inline constant.

}

@TestedBehaviour(IF_USER_IS_USED_AS_ADMIN_KEY)
@Test
public void if_adminKey_is_user_and_the_user_is_not_the_one_who_added_the_choice_then_the_choice_cannot_be_deleted() {
public void
if_adminKey_is_user_and_the_user_is_not_the_one_who_added_the_choice_then_the_choice_cannot_be_deleted() {
final Choice choiceToDelete = createChoice(USER, true);
assertThrows(() -> choiceService.deleteChoice(new VoteAdminInfo(vote.getId(), USER),
choiceToDelete.getId())).assertMessageIs(
"The adminKey is \"user\" but the user is not same with that user who added the choice.");
final VoteAdminInfo voteAdminInfo = new VoteAdminInfo(vote.getId(), USER);

doThrow(new ReportedException(NOT_SAME_USER_MESSAGE))
.when(choiceModificationValidatorService)
.validateDeletion(voteAdminInfo, vote, choiceToDelete);

assertThrows(
() -> {
choiceService.deleteChoice(
voteAdminInfo,
choiceToDelete.getId()
);
}
).assertMessageIs(
NOT_SAME_USER_MESSAGE

);
}

@TestedBehaviour(IF_USER_IS_USED_AS_ADMIN_KEY)
@Test
public void if_adminKey_is_user_and_canAddin_is_true_then_the_user_who_added_the_choice_is_able_to_delete_it() {
public void
if_adminKey_is_user_and_canAddin_is_true_then_the_user_who_added_the_choice_is_able_to_delete_it() {
final Choice choiceToDelete = createChoice(TEST_USER_NAME, true);
when(authService.getAuthenticatedUserName()).thenReturn(TEST_USER_NAME);
choiceService.deleteChoice(new VoteAdminInfo(vote.getId(), USER), choiceToDelete.getId());
choiceService.deleteChoice(
new VoteAdminInfo(vote.getId(), USER), choiceToDelete.getId()
);

assertThrows(() -> choiceService.getChoice(vote.getId(), choiceToDelete.getId()))
assertThrows(
() -> choiceService.getChoice(vote.getId(), choiceToDelete.getId())
)
.assertMessageIs("Illegal choiceId");
}

@TestedBehaviour(IF_USER_IS_USED_AS_ADMIN_KEY)
@Test
public void deleteChoice_saves_vote_if_the_choice_is_deleted() {
final Choice choiceToDelete = createChoice(TEST_USER_NAME, true);
when(authService.getAuthenticatedUserName()).thenReturn(TEST_USER_NAME);
choiceService.deleteChoice(new VoteAdminInfo(vote.getId(), USER), choiceToDelete.getId());

choiceService.deleteChoice(
new VoteAdminInfo(vote.getId(), USER), choiceToDelete.getId()
);
verify(voteService).saveVote(vote);
}

@TestedBehaviour(IF_THE_VOTE_HAS_BALLOTS_ISSUED)
@Test
public void if_the_vote_has_issued_ballots_then_the_choice_cannot_be_deleted() {
public void
if_the_vote_has_issued_ballots_then_the_choice_cannot_be_deleted() {
final Choice choiceToDelete = createChoice(TEST_USER_NAME, true);
vote.getBallots().add("TestBallot");

assertThrows(() -> choiceService
.deleteChoice(new VoteAdminInfo(vote.getId(), vote.getAdminKey()), choiceToDelete.getId()))
.assertMessageIs("Vote modification disallowed: ballots already issued");
assertThrows(
() -> choiceService
.deleteChoice(new VoteAdminInfo(vote.getId(), vote.getAdminKey()), choiceToDelete.getId())
)
.assertMessageIs("Vote modification disallowed: ballots already issued");
}

private Choice createChoice(final String userName, final boolean isAddinable) {
private Choice
createChoice(final String userName, final boolean isAddinable) {
vote.getParameters().setAddinable(isAddinable);
final Choice choiceToDelete = new Choice(CHOICE1, userName);
vote.addChoice(choiceToDelete);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package org.rulez.demokracia.pdengine.choice;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.junit.MockitoJUnitRunner;
import org.rulez.demokracia.pdengine.annotations.TestedBehaviour;
import org.rulez.demokracia.pdengine.annotations.TestedFeature;
import org.rulez.demokracia.pdengine.annotations.TestedOperation;

@TestedFeature("Manage votes")
@TestedOperation("delete choice")
@RunWith(MockitoJUnitRunner.class)
public class ChoiceDeleteValidatorServiceTest extends ChoiceValidatorBaseTest {

@Test
@TestedBehaviour(
"if 'user' is used as adminKey, then the user must be the one who added the choice and canAddIn be true"
)
public void userAdmin_cannot_delete_choice_if_canAddin_is_false() {
assertFalseCanAddinCauseException(
() -> underTest.validateDeletion(getUserAdminInfo(), vote, choice)
);
}

@TestedBehaviour("modifies the string of the choice")
@Test
public void proper_voteId_choiceId_and_adminKey_does_modify_choice() {
assertNotThrows(
() -> {
underTest.validateDeletion(getAdminInfo(), vote, choice);
}
);
}

@TestedBehaviour(
"if 'user' is used as adminKey, then the user must be the one who added the choice and canAddIn be true"
)
@Test
public void
userAdmin_cannot_modify_choice_if_it_is_not_added_by_other_user() {
assertDifferentUserCauseException(
() -> underTest.validateDeletion(getUserAdminInfo(), vote, choice)
);
}

@TestedBehaviour(
"if 'user' is used as adminKey, then the user must be the one who added the choice and canAddIn be true"
)
@Test
public void
userAdmin_can_modify_the_choice_if_canAddin_is_true_and_he_is_the_choice_creator() {
assertNoExceptionWithCorrectParameters(
() -> underTest.validateDeletion(getUserAdminInfo(), vote, choice2)
);
}

@Override
protected String getOperation() {
return "deletion";
}

}
Loading