Skip to content

Commit

Permalink
JC-2409 Error 500 when saving PM with invalid "To" field
Browse files Browse the repository at this point in the history
- Fixed hibernate lazy initialization exception by moving
userService.getCurrentUser() to service layer.
  • Loading branch information
evgeniycheban committed Sep 16, 2018
1 parent cdf589c commit 8566f46
Show file tree
Hide file tree
Showing 10 changed files with 160 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,8 @@ public interface PrivateMessageService extends EntityService<PrivateMessage> {
* @param userTo receiver of the message
* @param title the title of the message.
* @param body the body of the message.
* @param userFrom sender.
*/
void saveDraft(long id, JCUser userTo, String title, String body, JCUser userFrom);
void saveDraft(long id, JCUser userTo, String title, String body);

/**
* Get count of new messages for current user.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,13 @@ JCUser saveEditedUserNotifications(long editedUserId, UserNotificationsContainer
*/
void checkPermissionToCreateAndEditSimplePage(Long userId);

/**
* This method checks a permission of user to send private messages.
*
* @param userId an identified of user, for which we check permission.
*/
void checkPermissionToSendPrivateMessages(Long userId);

/**
* Searches for the common user, meaning that she might or might not be registered in JCommune, she can also be
* registered by some other JTalks component. This might be required to search through all the users of JTalks.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,9 @@ public Page<PrivateMessage> getDraftsForCurrentUser(String page) {
* {@inheritDoc}
*/
@Override
@PreAuthorize("hasPermission(#userFrom.id, 'USER', 'ProfilePermission.SEND_PRIVATE_MESSAGES')")
public void saveDraft(long id, JCUser userTo, String title, String body, JCUser userFrom) {
public void saveDraft(long id, JCUser userTo, String title, String body) {
JCUser userFrom = userService.getCurrentUser();
userService.checkPermissionToSendPrivateMessages(userFrom.getId());
PrivateMessage pm;
if (this.getDao().isExist(id)) {
pm = this.getDao().get(id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,12 @@ public void checkPermissionToCreateAndEditSimplePage(Long userId) {
LOGGER.debug("Check permission to create or edit simple(static) pages - " + userId);
}

@Override
@PreAuthorize("hasPermission(#userId, 'USER', 'ProfilePermission.SEND_PRIVATE_MESSAGES')")
public void checkPermissionToSendPrivateMessages(Long userId) {
LOGGER.debug("Check permission to send private messages");
}

/**
* {@inheritDoc}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,13 @@ public void testGetDraftsForCurrentUser() {
}

@Test
public void testSaveDraft() throws NotFoundException {
public void testSaveDraft() {
JCUser recipient = new JCUser("name", "[email protected]", "pwd");

when(securityService.<User>createAclBuilder()).thenReturn(aclBuilder);
when(userService.getCurrentUser()).thenReturn(JC_USER);

pmService.saveDraft(PM_ID, recipient, "title", "body", JC_USER);
pmService.saveDraft(PM_ID, recipient, "title", "body");

verify(pmDao).saveOrUpdate(any(PrivateMessage.class));
verify(aclBuilder).grant(GeneralPermission.WRITE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import org.springframework.web.servlet.ModelAndView;

import javax.validation.Valid;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

/**
Expand Down Expand Up @@ -242,10 +242,10 @@ public ModelAndView editDraftPage(@PathVariable(PM_ID) Long id) throws NotFoundE
* @return redirect to "drafts" folder if saved successfully or show form with error message
*/
@RequestMapping(value = "/pm/save", method = {RequestMethod.POST, RequestMethod.GET})
public ModelAndView saveDraft(@Valid @ModelAttribute("privateMessageDto") PrivateMessageDraftDto pmDto, BindingResult result) {
public ModelAndView saveDraft(@Valid @ModelAttribute("privateMessageDto") PrivateMessageDraftDto pmDto,
BindingResult result) {
String targetView = "redirect:/drafts";
long pmDtoId = pmDto.getId();
JCUser userFrom = userService.getCurrentUser();
JCUser userTo = null;
if (pmDto.getRecipient() != null) {
try {
Expand All @@ -258,7 +258,7 @@ public ModelAndView saveDraft(@Valid @ModelAttribute("privateMessageDto") Privat
// The case when field "To:" filled incorrectly and fields "Title:" and "Body" are both empty .
if (pmDtoId != 0) { //means that we try to edit existing draft
try {
pmService.delete(Arrays.asList(pmDtoId));
pmService.delete(Collections.singletonList(pmDtoId));
} catch (NotFoundException e) {
// Catch block is empty because we don't need any additional logic in case if user removed
// draft in separate browser tab. We should just redirect him to list of drafts
Expand All @@ -269,7 +269,7 @@ public ModelAndView saveDraft(@Valid @ModelAttribute("privateMessageDto") Privat
if (result.hasFieldErrors()){
return new ModelAndView(PM_FORM).addObject(DTO,pmDto);
}
pmService.saveDraft(pmDtoId, userTo, pmDto.getTitle(), pmDto.getBody(), userFrom);
pmService.saveDraft(pmDtoId, userTo, pmDto.getTitle(), pmDto.getBody());
return new ModelAndView(targetView);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ public void saveDraftPost() throws Exception {
.andExpect(model().hasNoErrors())
.andExpect(status().isMovedTemporarily())
.andExpect(redirectedUrl("/drafts"));
verify(pmService).saveDraft(PM_ID, JC_USER, TITLE, BODY, JC_USER);
verify(pmService).saveDraft(PM_ID, JC_USER, TITLE, BODY);
}

@Test
Expand All @@ -347,7 +347,7 @@ public void testSaveDraftGet() throws Exception {
.andExpect(model().hasNoErrors())
.andExpect(status().isMovedTemporarily())
.andExpect(redirectedUrl("/drafts"));
verify(pmService).saveDraft(0, JC_USER, TITLE, BODY, JC_USER);
verify(pmService).saveDraft(0, JC_USER, TITLE, BODY);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/**
* Copyright (C) 2011 JTalks.org Team
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 2.1 of the License, or (at your option) any later version.
* This library 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
* Lesser General Public License for more details.
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
package org.jtalks.jcommune.test

import org.jtalks.common.model.permissions.ProfilePermission
import org.jtalks.jcommune.service.security.AdministrationGroup
import org.jtalks.jcommune.test.model.User
import org.jtalks.jcommune.test.service.GroupsService
import org.jtalks.jcommune.test.utils.PrivateMessages
import org.jtalks.jcommune.test.utils.Users
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.mock.web.MockHttpSession
import org.springframework.test.context.ContextConfiguration
import org.springframework.test.context.transaction.TransactionConfiguration
import org.springframework.test.context.web.WebAppConfiguration
import org.springframework.test.web.servlet.MockMvc
import org.springframework.transaction.annotation.Transactional
import spock.lang.Specification

import static org.jtalks.jcommune.test.utils.assertions.Assert.assertView
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post

/**
* @author Evgeniy Cheban
*/
@WebAppConfiguration
@ContextConfiguration(locations = 'classpath:/org/jtalks/jcommune/web/view/test-configuration.xml')
@TransactionConfiguration(transactionManager = 'transactionManager', defaultRollback = true)
@Transactional
class PrivateMessageControllerTest extends Specification {
@Autowired
Users users
@Autowired
GroupsService groupsService
@Autowired
MockMvc mockMvc

def setup() {
groupsService.createPredefinedGroups()
}

def 'should save draft private message when "to" invalid'() {
given: 'User logged in'
def user = User.newInstance()
def group = groupsService.getGroupByName(AdministrationGroup.USER.name)
users.created(user).withPermissionOn(group, ProfilePermission.SEND_PRIVATE_MESSAGES)

def session = users.signIn(user)
when: 'User save draft private message with invalid "to"'
def pmDto = PrivateMessages.randomDto()
def mvcResult = mockMvc.perform(post('/pm/save')
.session(session as MockHttpSession)
.param('title', pmDto.title)
.param('body', pmDto.body)
.param('recipient', pmDto.recipient)
).andReturn()
then:
assertView(mvcResult, 'redirect:/drafts')
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* Copyright (C) 2011 JTalks.org Team
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 2.1 of the License, or (at your option) any later version.
* This library 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
* Lesser General Public License for more details.
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
package org.jtalks.jcommune.test.model

/**
* @author Evgeniy Cheban
*/
class PrivateMessageDto {
String title
String body
String recipient
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Copyright (C) 2011 JTalks.org Team
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 2.1 of the License, or (at your option) any later version.
* This library 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
* Lesser General Public License for more details.
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
package org.jtalks.jcommune.test.utils

import org.jtalks.common.model.entity.User
import org.jtalks.jcommune.model.entity.PrivateMessage
import org.jtalks.jcommune.test.model.PrivateMessageDto

import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic

/**
* @author Evgeniy Cheban
*/
class PrivateMessages {

static PrivateMessageDto randomDto(Map<String, Object> overrideDefaults = [:]) {
Map<String, Object> defaults = [
'title': randomAlphabetic(PrivateMessage.MAX_TITLE_LENGTH).toString(),
'body': randomAlphabetic(PrivateMessage.MAX_MESSAGE_LENGTH).toString(),
'recipient': randomAlphabetic(User.USERNAME_MAX_LENGTH).toString()
]
defaults.putAll(overrideDefaults)
return new PrivateMessageDto(defaults)
}
}

0 comments on commit 8566f46

Please sign in to comment.