ePages-de / restdocs-wiremock

Spring REST Docs WireMock Integration
Apache License 2.0
75 stars 27 forks source link

MockMvcRequestConverter wrong stub generation for DELETE method #67

Closed SergeyPirogov closed 1 year ago

SergeyPirogov commented 6 years ago

I found an issue at MockMvcRequestConverter. For example we have batch delete Method that uses HTTP DELETE example below:

DELETE /users?id=1&id=2&id=3

In such scenario document generator can't generate correct stub. I found the place where the problem exists. Below in convert method there is implicitly "GET" only method covered. What to do in case with DELETE?

@Override
    public OperationRequest convert(MockHttpServletRequest mockRequest) {
        try {
            HttpHeaders headers = extractHeaders(mockRequest);
            Parameters parameters = extractParameters(mockRequest);
            List<OperationRequestPart> parts = extractParts(mockRequest);
            Collection<RequestCookie> cookies = extractCookies(mockRequest, headers);
            String queryString = mockRequest.getQueryString();
            if (!StringUtils.hasText(queryString)
                    && "GET".equals(mockRequest.getMethod())) {
                queryString = parameters.toQueryString();
            }
            return new OperationRequestFactory().create(
                    URI.create(
                            getRequestUri(mockRequest) + (StringUtils.hasText(queryString)
                                    ? "?" + queryString : "")),
                    HttpMethod.valueOf(mockRequest.getMethod()),
                    FileCopyUtils.copyToByteArray(mockRequest.getInputStream()), headers,
                    parameters, parts, cookies);
        }
        catch (Exception ex) {
            throw new ConversionException(ex);
        }
    }
mduesterhoeft commented 6 years ago

Thanks for the request. Would you mind to create a PR?

otrosien commented 5 years ago

My understanding is that this is an issue of spring-restdocs, not restdocs-wiremock, as the query params assumption in MockMvcRequestConverter is part of spring-restdocs-mockmvc code [1], and as such we would have to open an issue upstream.

[1] https://github.com/spring-projects/spring-restdocs/blob/master/spring-restdocs-mockmvc/src/main/java/org/springframework/restdocs/mockmvc/MockMvcRequestConverter.java#L74

SergeyPirogov commented 5 years ago

@otrosien I think yes