Skip to content
This repository has been archived by the owner on Dec 19, 2023. It is now read-only.

Replacement for binary content of multipart requests #369

Merged
merged 2 commits into from
Jan 27, 2020

Conversation

MikeSafonov
Copy link
Contributor

This PR adds MultipartContentOperationPreprocessor for replacing binary content of multipart/form-data requests

@fbenz fbenz requested review from jmisur and fbenz January 21, 2020 09:02
Copy link
Contributor

@fbenz fbenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. An alternative to creating a separate processor would be to just add the media type to the set of media types in BinaryReplacementContentModifier. But I also prefer a separate processor because in some cases it makes sense that the content of multipart requests ends up in the document. This is very unlikely for images and PDFs (covered by the existing BinaryReplacementContentModifier). With keeping them separate, every user can decide to go with both, either one or none.

import org.springframework.restdocs.operation.preprocess.OperationPreprocessor;
import org.springframework.restdocs.operation.preprocess.OperationResponsePreprocessor;
import org.springframework.restdocs.operation.preprocess.Preprocessors;
import org.springframework.restdocs.operation.preprocess.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please expand the imports again. We are avoiding * imports. But I also know that we neither have this documented nor enforced at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;

public class MultipartContentOperationPreprocessor extends OperationPreprocessorAdapter {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why you are extending OperationPreprocessorAdapter instead of implementing ContentModifier like the BinaryReplacementContentModifier? Implementing ContentModifier makes the code a bit simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I extending OperationPreprocessorAdapter because ContentModifier takes original content (look at ContentModifyingOperationPreprocessor. In case of multipart/form-data content is empty byte array and all binary data holds in parts field.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this makes sense.

@@ -0,0 +1,44 @@
package capital.scalable.restdocs.response;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The headers above the new files are missing but I can add them as well. We have a Maven plugin configured that adds them automatically.

import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;

public class MultipartContentOperationPreprocessor extends OperationPreprocessorAdapter {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this makes sense.

@fbenz fbenz merged commit 2a23c31 into ScaCap:master Jan 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants