-
Notifications
You must be signed in to change notification settings - Fork 1
Persistence of notifications sent from the hub #131
Conversation
@@ -78,45 +28,25 @@ public void setTransferInformation(CapitalTransferInformation transferInformatio | |||
this.transferInformation = transferInformation; | |||
} | |||
|
|||
@Override |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add here toString()
again. I think it could be useful to show the payment information as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely, an oversight. Fixed it.
@@ -29,7 +34,12 @@ | |||
} | |||
|
|||
private Optional<InsurantInformation> findMatchingEntry(Set<InsurantInformation> retirementFundEntries, InsurantInformation termination) { | |||
return retirementFundEntries.stream().filter(commencement -> isMatching(commencement, termination)).findAny(); | |||
final List<InsurantInformation> result = retirementFundEntries.stream().filter(commencement -> isMatching(commencement, termination)).collect(Collectors.toList()); | |||
if (result.size() > 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't you use count()
here on the stream so you don't have to collect it and stream it again when returning findAny()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I tried that initially, but it turned out that count() is already a terminal operation and the stream cannot be used afterwards. Java streams do not support forking, thus one has to buffer them in order to be able to reuse them.
} | ||
|
||
public void saveMatchForTermination(MatchForTermination notification) { | ||
final NotificationDTO dto = findByNotification(notification) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case should actually never happen?
import java.time.LocalDate; | ||
|
||
@Entity | ||
public class NotificationDTO { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me it is very confusing that the name ending DTO is used for an entity. I have only seen DTOs to be used as objects which are used by a REST API. Is that a common standard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is common, I have seen it in several occasions. In general DTOs are used to carry data between remote processes. The original intent was to use it in remote interfaces, e.g. a REST interface. One could argue that exchanging data between an application and the DB is also communication between two remote processes, therefore speaking about DTOs at this point makes sense.
Technically a DTO has two responsibilities: to aggregate data that can be send in a single call and to take care of serialization. Entities do not really aggregate data, but they do take care of serialization. Guess one could argue both ways.
…35_hub_persistence # Conflicts: # demo/build.gradle # encrypted-data-model/src/main/java/ch/prevo/open/encrypted/model/MatchForTermination.java # hub/src/main/java/ch/prevo/open/hub/nodes/NodeCaller.java # hub/src/main/java/ch/prevo/open/hub/nodes/NodeService.java # hub/src/main/java/ch/prevo/open/hub/repository/NotificationRepository.java # hub/src/test/java/ch/prevo/open/hub/nodes/NodeCallerTest.java # hub/src/test/java/ch/prevo/open/hub/repository/NotificationDAOTest.java # node/src/main/java/ch/prevo/open/node/api/MatchNotificationController.java # node/src/test/java/ch/prevo/open/node/api/MatchNotificationControllerTest.java
No description provided.