Store message-context in ErrorStorage #6339
Conversation
| public MessageContext(MessageContext base) { | ||
| this(base.data); | ||
| } |
There was a problem hiding this comment.
Is dit handig? Wanneer wordt dit gebruikt? Misschien dat het handiger is om een copy methode te hebben. Anders hebben we straks mogelijk 2 pointers naar de zelfde context.
There was a problem hiding this comment.
De data wordt wel gekopieerd in die andere constructor, dus we hebben niet 2 pointers naar dezelfde. Ik zal even opzoeken waar het gebruikt wordt, om te kijken of het nodig / wenselijk is.
There was a problem hiding this comment.
Deze constructor wordt 2x gebruikt, in Message.copyContext() en in de constructor van PathMessage.
De constructor die een map als parameter neemt wordt ook 2x gebruikt: in FileSystemUtils.getContext() en in de constructor van UrlMessage.
Ik zou deze constructors kunnen weghalen en vervangen door .withAllFrom(...)?
| } | ||
|
|
||
| private PartMessage(MessageContext context, Part part) throws MessagingException { | ||
| public PartMessage(MessageContext context, Part part) throws MessagingException { |
There was a problem hiding this comment.
Ik vind het misschien mooier om de context (als optionele) 2e param mee te geven.
There was a problem hiding this comment.
Op zich heb je gelijk. Dit was ooit nodig denk ik om overlappende constructors uit elkaar te halen, maar dat is nu geen excuus meer.
| public void putAll(Map<String, ?> base) { | ||
| if (base!=null) { | ||
| base.forEach((key, value) -> { | ||
| if (value instanceof Serializable) data.put(key, (Serializable) value); |
There was a problem hiding this comment.
En anders? Ten minste een log melding denk ik, of als transient toevoegen oid.
There was a problem hiding this comment.
Goed punt. Ik denk dat we in de context wel een map van Map<String, Object> moeten houden maar dat we in writeObject() dan alles eruit moeten filteren wat niet Serializable is, zodat er dan in ieder geval geen onverwachtte fouten optreden bij het schrijven naar de error-store, en op dat moment een warning loggen voor al die keys.
|
Ik vind hem nu al goed! |
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| Copyright 2020-2022 WeAreFrank! | |||
| Copyright 2020-2024 WeAreFrank! | |||
There was a problem hiding this comment.
Always a good time to update copyrights 😜
nielsm5
changed the title
No description provided.