Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MockClientHttpResponse should not return null body [SPR-16367] #20914

Closed
spring-projects-issues opened this issue Jan 11, 2018 · 6 comments
Closed
Assignees
Labels
in: test type: bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jan 11, 2018

Arthur Gavlyukovskiy opened SPR-16367 and commented

According to HttpInputMessage documentation body can never be null, although this is not true when you create mock response using MockRestResponseCreators without body, e.g. withBadRequest() or withSuccess().

Looks like changes should be done in MockHttpInputMessage to set ByteArrayInputStream with empty byte array if passed content is null. As well null check before closing stream in MockClientHttpResponse may be removed.


Affects: 4.3.12

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 12, 2018

Rossen Stoyanchev commented

I don't follow. MockRestResponseCreators creates a DefaultResponseCreator which initializes the body to byte[0]. Then it passes that into MockHttpInputMessage which has an Assert.notNull check on the body passed in.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 12, 2018

Arthur Gavlyukovskiy commented

withStatus(HttpStatus.TOO_MANY_REQUESTS) uses new DefaultResponseCreator(HttpStatus) and DefaultResponseCreator#content stays null. Then in DefaultResponseCreator#createResponse it goes to the branch where this.contentResource is null:

response = new MockClientHttpResponse(this.content, this.statusCode);

here this.content is null and then constructor of MockHttpInputMessage

public MockHttpInputMessage(byte[] contents) {
	this.body = (contents != null ? new ByteArrayInputStream(contents) : null);
}

sets body to null if contents was null.

As workaround I use

withStatus(HttpStatus.TOO_MANY_REQUESTS).body("")

But to match documentation it should be empty by default or, which is better, make changes in constructor to something like

public MockHttpInputMessage(byte[] contents) {
	this.body = new ByteArrayInputStream(contents != null ? contents : new byte[0]);
}

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 12, 2018

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 12, 2018

Rossen Stoyanchev commented

Okay that makes sense now. We can make this adjustment in 4.3.x.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 12, 2018

Arthur Gavlyukovskiy commented

Thanks for a quick fix. BTW about copyright, it is 2018 already :)

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 15, 2018

Rossen Stoyanchev commented

Yes it is :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test type: bug
Projects
None yet
Development

No branches or pull requests

2 participants