Cleanup MediaType and implement in Core #2790
Conversation
...va/nl/nn/adapterframework/extensions/aspose/services/conv/impl/CisConversionServiceImpl.java
Outdated
Show resolved
Hide resolved
core/src/main/java/nl/nn/adapterframework/util/MessageUtils.java
Outdated
Show resolved
Hide resolved
core/src/main/java/nl/nn/adapterframework/util/MessageUtils.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Gerrit van Brakel <g.vanbrakel@flux-it.nl>
| * If no charset was provided and the requested charset is <code>auto</auto>, try to parse the charset. | ||
| * If unsuccessful return the default; <code>UTF-8</code>. | ||
| */ | ||
| private String computeCharset(String defaultCharset) throws IOException { |
There was a problem hiding this comment.
computeCharset moet volgens mij de volgende precedence hebben:
- de charset uit context METADATA_CHARSET
- als 'defaultCharset==auto', dan MessageUtils.computeCharset(this)
- defaultCharset
- StreamUtil.DEFAULT_INPUT_STREAM_ENCODING
Optie 1 zit er nu niet in
There was a problem hiding this comment.
Dat zit als het goed is in de MessageUtils
There was a problem hiding this comment.
Ik vind het nog steeds heel verwarrend.
- dat deze computeCharset een argument mee krijgt dat hij ook zelf op zou kunnen halen
- dat hij niet het argument defaultCharset van asReader(defaultCharset) mee krijgt,
- maar dat z'n argument wel defaultCharset heet
- dat hij maar een deel van de charset berekent, namelijk dat stuk wat er zou zijn als er zonder defaultCharset van asReader(defaultCharset)
- dat er in MessageUtils ook nog een computeCharset is die bijna alles doet, maar niet alles.
Ik zou voor willen stellen:
- MessageUtils.computeCharset te hernoemen in MessageUtils.computeDecodingCharset
- daar de defaultCharset van asReader(defaultCharset) aan mee te geven
- de method computeCharset() in Message op te doeken.
core/src/main/java/nl/nn/adapterframework/util/MessageUtils.java
Outdated
Show resolved
Hide resolved
| * If no charset was provided and the requested charset is <code>auto</auto>, try to parse the charset. | ||
| * If unsuccessful return the default; <code>UTF-8</code>. | ||
| */ | ||
| private String computeCharset(String defaultCharset) throws IOException { |
There was a problem hiding this comment.
Ik vind het nog steeds heel verwarrend.
- dat deze computeCharset een argument mee krijgt dat hij ook zelf op zou kunnen halen
- dat hij niet het argument defaultCharset van asReader(defaultCharset) mee krijgt,
- maar dat z'n argument wel defaultCharset heet
- dat hij maar een deel van de charset berekent, namelijk dat stuk wat er zou zijn als er zonder defaultCharset van asReader(defaultCharset)
- dat er in MessageUtils ook nog een computeCharset is die bijna alles doet, maar niet alles.
Ik zou voor willen stellen:
- MessageUtils.computeCharset te hernoemen in MessageUtils.computeDecodingCharset
- daar de defaultCharset van asReader(defaultCharset) aan mee te geven
- de method computeCharset() in Message op te doeken.
core/src/main/java/nl/nn/adapterframework/util/MessageUtils.java
Outdated
Show resolved
Hide resolved
| Message messageNullCharset = new Message((String) null) { //NullMessage, charset cannot be determined | ||
| @Override |
There was a problem hiding this comment.
Is eigenlijk raar dat je hier een 'String' messag gebruikt. Die zou geen charset moeten hebben, en een empty message ook niet. Je test nu wel iets, maar computeDecodingCharset zou hier ook null op mogen leveren.
There was a problem hiding this comment.
Het is raar dat een String message een charset veld heeft.
Hier wordt in puur de 'flow' van charset bepalen getest, en daar moet de 'compute' null teruggeven.
| * Only works for binary messages | ||
| * @param readLimit amount of bytes to read. | ||
| */ | ||
| public byte[] getMagic(int readLimit) throws IOException { |
There was a problem hiding this comment.
Wordt deze getMagic nog ergens anders gebruikt dan in MessageUtils? Moet hij niet daar heen dan?
There was a problem hiding this comment.
Nee, alleen in de MessageUtils. Hij kan niet verplaatst worden omdat de InputStream gewrapped moet worden wanneer dit geen markSupported stream is.
| LOG.info("unable to detect charset for message [{}] closest match [{}] did not meet confidence level [{}/{}]", message, charset, match.getConfidence(), confidence); | ||
| return updateMessageCharset(message, null); //return NULL so calling method can fall back to the default charset. | ||
| } | ||
|
|
There was a problem hiding this comment.
Ik denk dat je hier ook ergens wil regelen dat als er op een repeatable message meerdere malen getReader(auto) gedaan wordt, er dan maar één keer gedetecteerd wordt, ook als dat niet succesvol is.
No description provided.