Skip to content

Commit 47d72d0

Browse files
committed
JC-2409 Error 500 when saving PM with invalid "To" field
- Fixed hibernate lazy initialization exception by moving userService.getCurrentUser() to service layer.
1 parent cdf589c commit 47d72d0

7 files changed

Lines changed: 27 additions & 13 deletions

File tree

jcommune-service/src/main/java/org/jtalks/jcommune/service/PrivateMessageService.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,8 @@ public interface PrivateMessageService extends EntityService<PrivateMessage> {
7575
* @param userTo receiver of the message
7676
* @param title the title of the message.
7777
* @param body the body of the message.
78-
* @param userFrom sender.
7978
*/
80-
void saveDraft(long id, JCUser userTo, String title, String body, JCUser userFrom);
79+
void saveDraft(long id, JCUser userTo, String title, String body);
8180

8281
/**
8382
* Get count of new messages for current user.

jcommune-service/src/main/java/org/jtalks/jcommune/service/UserService.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,13 @@ JCUser saveEditedUserNotifications(long editedUserId, UserNotificationsContainer
152152
*/
153153
void checkPermissionToCreateAndEditSimplePage(Long userId);
154154

155+
/**
156+
* This method checks a permission of user to send private messages.
157+
*
158+
* @param userId an identified of user, for which we check permission.
159+
*/
160+
void checkPermissionToSendPrivateMessages(Long userId);
161+
155162
/**
156163
* Searches for the common user, meaning that she might or might not be registered in JCommune, she can also be
157164
* registered by some other JTalks component. This might be required to search through all the users of JTalks.

jcommune-service/src/main/java/org/jtalks/jcommune/service/transactional/TransactionalPrivateMessageService.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,9 @@ public Page<PrivateMessage> getDraftsForCurrentUser(String page) {
147147
* {@inheritDoc}
148148
*/
149149
@Override
150-
@PreAuthorize("hasPermission(#userFrom.id, 'USER', 'ProfilePermission.SEND_PRIVATE_MESSAGES')")
151-
public void saveDraft(long id, JCUser userTo, String title, String body, JCUser userFrom) {
150+
public void saveDraft(long id, JCUser userTo, String title, String body) {
151+
JCUser userFrom = userService.getCurrentUser();
152+
userService.checkPermissionToSendPrivateMessages(userFrom.getId());
152153
PrivateMessage pm;
153154
if (this.getDao().isExist(id)) {
154155
pm = this.getDao().get(id);

jcommune-service/src/main/java/org/jtalks/jcommune/service/transactional/TransactionalUserService.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,12 @@ public void checkPermissionToCreateAndEditSimplePage(Long userId) {
298298
LOGGER.debug("Check permission to create or edit simple(static) pages - " + userId);
299299
}
300300

301+
@Override
302+
@PreAuthorize("hasPermission(#userId, 'USER', 'ProfilePermission.SEND_PRIVATE_MESSAGES')")
303+
public void checkPermissionToSendPrivateMessages(Long userId) {
304+
LOGGER.debug("Check permission to send private messages");
305+
}
306+
301307
/**
302308
* {@inheritDoc}
303309
*/

jcommune-service/src/test/java/org/jtalks/jcommune/service/transactional/TransactionalPrivateMessageServiceTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,12 +181,13 @@ public void testGetDraftsForCurrentUser() {
181181
}
182182

183183
@Test
184-
public void testSaveDraft() throws NotFoundException {
184+
public void testSaveDraft() {
185185
JCUser recipient = new JCUser("name", "example@example.com", "pwd");
186186

187187
when(securityService.<User>createAclBuilder()).thenReturn(aclBuilder);
188+
when(userService.getCurrentUser()).thenReturn(JC_USER);
188189

189-
pmService.saveDraft(PM_ID, recipient, "title", "body", JC_USER);
190+
pmService.saveDraft(PM_ID, recipient, "title", "body");
190191

191192
verify(pmDao).saveOrUpdate(any(PrivateMessage.class));
192193
verify(aclBuilder).grant(GeneralPermission.WRITE);

jcommune-view/jcommune-web-controller/src/main/java/org/jtalks/jcommune/web/controller/PrivateMessageController.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
import org.springframework.web.servlet.ModelAndView;
3434

3535
import javax.validation.Valid;
36-
import java.util.Arrays;
36+
import java.util.Collections;
3737
import java.util.List;
3838

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

jcommune-view/jcommune-web-controller/src/test/java/org/jtalks/jcommune/web/controller/PrivateMessageControllerTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ public void saveDraftPost() throws Exception {
332332
.andExpect(model().hasNoErrors())
333333
.andExpect(status().isMovedTemporarily())
334334
.andExpect(redirectedUrl("/drafts"));
335-
verify(pmService).saveDraft(PM_ID, JC_USER, TITLE, BODY, JC_USER);
335+
verify(pmService).saveDraft(PM_ID, JC_USER, TITLE, BODY);
336336
}
337337

338338
@Test
@@ -347,7 +347,7 @@ public void testSaveDraftGet() throws Exception {
347347
.andExpect(model().hasNoErrors())
348348
.andExpect(status().isMovedTemporarily())
349349
.andExpect(redirectedUrl("/drafts"));
350-
verify(pmService).saveDraft(0, JC_USER, TITLE, BODY, JC_USER);
350+
verify(pmService).saveDraft(0, JC_USER, TITLE, BODY);
351351
}
352352

353353
@Test

0 commit comments

Comments
 (0)