Merge CheckSumPipe and HashPipe and add hashEncoding #7456
Conversation
| } | ||
| } | ||
|
|
||
| protected static class ZipChecksumGenerator implements ChecksumGenerator { | ||
|
|
||
| static class BasicGenerator implements ChecksumGenerator { |
There was a problem hiding this comment.
Ik twijfelde nog een beetje over deze naam, maar 'zip' moest er iig vanaf wat mij betreft
| checksum.reset(); | ||
| } | ||
|
|
||
| @Override | ||
| public void update(int b){ |
There was a problem hiding this comment.
Deze werd nergens gebruikt
| @@ -80,7 +80,7 @@ | |||
| * <pipe name="make unique message" className="org.frankframework.pipes.FixedResultPipe" | |||
| * returnString="<msg mid="MID" action="ACTION" />" replaceFixedParams="true"> | |||
| * <param name="MID" sessionKey="mid" /> | |||
| * <param name="ACTION" xpathExpression="request/@action" /> | |||
| * <param name="ACTION" xpathExpression="request/@action" /> | |||
There was a problem hiding this comment.
Formatting fix voor wat we vanochtend zagen
| @@ -21,41 +25,6 @@ public ChecksumPipe createPipe() { | |||
| return new ChecksumPipe(); | |||
| } | |||
|
|
|||
| @Test | |||
| public void rightChecksumGeneratedMD5() throws Exception { | |||
There was a problem hiding this comment.
Deze testen waren vrij zinloos: check of je een md5 checksumgenerator aanmaakt als je een md5 generator aanmaakt. Qua coverage verliezen we hier ook niets. De enige relevant is cantCalculate hieronder wat mij betreft.
| URL file = TestFileUtils.getTestFileURL("/Pipes/2.txt"); | ||
| assertEquals(expected, calculateChecksum(file.getPath(), ChecksumType.MD5, true)); | ||
| assertEquals(expected, calculateChecksum(file.openStream(), ChecksumType.MD5)); | ||
| public static Stream<Arguments> values() { |
There was a problem hiding this comment.
Mooier als ParameterizedTest vond ik
There was a problem hiding this comment.
Alleen formatting
| * need to prove the authenticity of the message as well, please use the {@link HashPipe} which uses an algorithm and a secret to prove both | ||
| * integrity and authenticity. | ||
| * <p> | ||
| * The hash is generated based on the bytes of the given input message or on the bytes read from the file path if @{inputIsFile} is @{code true} |
There was a problem hiding this comment.
| * The hash is generated based on the bytes of the given input message or on the bytes read from the file path if @{inputIsFile} is @{code true} | |
| * The hash is generated based on the bytes of the given input message or on the bytes read from the file path if {@code inputIsFile} is {@code true} |
| * | ||
| * CRC32 and Adler32 are {@link Checksum} implementations, the others are {@link MessageDigest} implementations. |
There was a problem hiding this comment.
| * CRC32 and Adler32 are {@link Checksum} implementations, the others are {@link MessageDigest} implementations. |
Dit is veel te technisch.
| * Example usage: | ||
| * <pre>{@code | ||
| * <pipe className="org.frankframework.pipes.ChecksumPipe" | ||
| * name="SHA" | ||
| * algorithm="SHA"> | ||
| * <forward name="success" path="READY"/> | ||
| * </pipe> | ||
| * }</pre> |
There was a problem hiding this comment.
Dit gaan we later automatisch genereren, en de syntax klopt niet, dus dit kan ook beter weggelaten worden.
| * <p> | ||
| * Example usage: | ||
| * <pre>{@code | ||
| * <pipe className="org.frankframework.pipes.HashPipe" | ||
| * name="SHA" | ||
| * algorithm="HmacSHA1" | ||
| * secret="privateSecretString"> | ||
| * <forward name="success" path="READY"/> | ||
| * </pipe> | ||
| * }</pre> |
| * <li>MD5</li> | ||
| * <li>SHA</li> | ||
| * <li>SHA256</li> | ||
| * <li>SHA384</li> | ||
| * <li>SHA512</li> |
There was a problem hiding this comment.
Als ik dit zo terugzie is het eigenlijk helemaal niet zo lastig om van deze 2 pipes 1 te maken. Misschien toch maandag even in de groep gooien?
There was a problem hiding this comment.
Sure, ik heb de rest aangepast
| public boolean isRequiresSecret() { | ||
| return requiresSecret; | ||
| } |
There was a problem hiding this comment.
Stiekem vind ik dan requiresSecret mooier zonder de is er voor 😶🌫️
There was a problem hiding this comment.
na overleg aangepast naar isSecretRequired.
nielsm5
changed the title
|
And add clear documentation and unit tests for HashPipe and ChecksumPipe