CF-1007: Download documents from sumsub#1134
Conversation
| return httpConnection; | ||
| } | ||
|
|
||
| private static String getContentType(HttpURLConnection httpConnection) { |
| return contentType; | ||
| } | ||
|
|
||
| private static void validateResponseCode(HttpURLConnection httpConnection, String imageId) throws IOException { |
| private final int retryDelaySeconds; // with increasing backoff (attemptNumber * retryDelaySeconds) | ||
|
|
||
| public SumsubDocumentDownloader(SumsubDocumentClient client, SumsubIdentityPieceCreator creator, | ||
| int maxDownloadRetries, int retryDelaySeconds) { |
There was a problem hiding this comment.
I would make maxDownloadRetries and retryDelaySeconds as constants in the class. It seems unnecessary to set them in the constructor.
There was a problem hiding this comment.
I refactored it to use constants in the calling class, but kept the constructor injection for easier testability.
|
|
||
| List<InspectionImage> mappableImages = images.stream() | ||
| .filter(img -> { | ||
| if (img.getImageId() == null || img.getIdDocDef() == null) { |
There was a problem hiding this comment.
I would divide it into two conditions and write a specific message - for possible easier debugging.
| * | ||
| * @param docType the Sumsub document type | ||
| * @param contentType the MIME type of the content | ||
| * @param content the raw content bytes |
| } | ||
|
|
||
| List<InspectionImage> mappableImages = images.stream() | ||
| .filter(img -> { |
There was a problem hiding this comment.
You could put this filtering logic into a method and give it a nice name to make ti more readable.
|
|
||
| if (!failedImages.isEmpty()) { | ||
| List<String> failedImageDetails = failedImages.stream() | ||
| .map(img -> img.getImageId() + " (" + (img.getIdDocDef() != null ? img.getIdDocDef().getIdDocType() : "unknown") + ")") |
There was a problem hiding this comment.
Maybe extract this into a method. It's hard to tell what it's supposed to do.
| @MethodSource("provideSelfieDocumentTypes") | ||
| void createIdentityPieceReturnsSelfieForSelfieVideoSelfie(SumSubDocumentType documentType) { | ||
| byte[] content = "selfie-data".getBytes(); | ||
| String contentType = "image/png"; |
There was a problem hiding this comment.
Just something to consider here. I'm sure the contentType will be different for videos in reality. It's not a problem in this test, but perhaps it would be a good idea to validate the content types somewhere? You are basically storing raw data without even checking what it is, or if it's valid for the given doc type.
| public SumsubDocumentClient(String token, String secret, String baseUrl) { | ||
| this.token = token; | ||
| this.baseUrl = baseUrl.endsWith("/") ? baseUrl.substring(0, baseUrl.length() - 1) : baseUrl; | ||
| try { |
There was a problem hiding this comment.
I'd put this try-catch into a private method
| return Hex.bytesToHexString(mac.doFinal()); | ||
| } | ||
|
|
||
| public record DownloadedDocument(byte[] content, String contentType) { |
There was a problem hiding this comment.
This is just an idea, but if you extracted this record, you could make it package-private. It's not needed anywhere outside this package anyway, if I'm not wrong.
| return failedImages; | ||
| } | ||
|
|
||
| private void addRetryDelay(int attempt) { |
There was a problem hiding this comment.
add could be confusing. Consider a better name like waitBeforeRetry or applyRetryDelay.
filipocelka
merged commit 842a36c
into
GENERALBYTESCOM:feature/1.17-snapshot
* CF-1007: Download documents from sumsub * CF-1007: Refactor constants, filters, formatting * CF-1007: Content type validation
4 participants
No description provided.