search

marcinadd / projecty-web

Project management software based on spring
GNU General Public License v3.0
76 stars 76 forks source link

Fix failing test case for MessageControllerTests - givenRequestOnDown… #23

Closed anirban-a closed 5 years ago

anirban-a commented 5 years ago

…loadFile_shouldReturnFileToDownload

Handle null pointer exception in MessageController

marcinadd commented 5 years ago

Thanks for your help. Sorry, but I can't accept your pull request. It will be nice to use aspects here. I've already finished MessageAspect.

  1. It isn't necessary to check if message != null since if optionalMessage.isPresent() so message can't be null.
  2. In this case, downloadFile is unprotected. Every user can have access to someone else message attachments.
  3. If you can you should avoid nested ifs. You check if message.getAttachments() is not null but you don't check if message.getAttachments.get(...) isn't null. if (optionalMessage.isPresent() && optionalMessage.get().getAttachments().get(Math.toIntExact(fileId))) sounds better for me. And you don't have to set attachment to null and set in one more time.

Thanks Hope you contribute one more time :)

anirban-a commented 5 years ago

Ok, so for now I reverted the changed made in the controller, kept as it was, but fixed the specific test case that was failing. Also, @marcinadd can you provide some other contact details of yours where I can discuss any doubts and confusions with you, say Skype, Slack, wherever you're active?