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

ApiListingResource doesn't respect pretty print support #2320

Closed
alexandrebrasil opened this issue Jul 19, 2017 · 4 comments
Closed

ApiListingResource doesn't respect pretty print support #2320

alexandrebrasil opened this issue Jul 19, 2017 · 4 comments

Comments

@alexandrebrasil
Copy link

I configured Swagger using BeanConfig on my Application start up to use pretty print, but when I retrieve the API from swagger.json endpoint it's all on a single line.

I've tracked the issue down to the ApiListingResource. When this resource processes the API, it doesn't return a Swagger object on the Response (that would be then converted by SwaggerSerializers), but returns a text string instead.

If ApiListingResource responds with the Swagger object instead, the serialization works fine and respect the pretty print request. Could the getListing() code be changed from

        if (StringUtils.isNotBlank(type) && type.trim().equalsIgnoreCase("yaml")) {
            return getListingYamlResponse(app, context, sc, headers, uriInfo);
        } else {
            return getListingJsonResponse(app, context, sc, headers, uriInfo);
        }

to

		Swagger swagger = process(app, context, sc, headers, uriInfo);
		if(swagger == null) {
			return Response.status(Status.NOT_FOUND).build();
		} else {
			return Response.ok(swagger, "json".equals(type) ? MediaType.APPLICATION_JSON_TYPE : new MediaType("application", "yaml")).build();
		}

or would it affect some functionality I'm not aware of?

@ChristianCiach
Copy link

I can confirm this using Swagger 1.5.16. As BaseApiListingResource.java calls Json.mapper().writeValueAsString(swagger) directly, there is absolutely no point in registering SwaggerSerializers.class to the JAX-RS implementation, as SwaggerSerializers isn't used anyway.

@ChristianCiach
Copy link

ChristianCiach commented Aug 3, 2017

I now use my own listing resource instead of ApiListingResource as a workaround:

/**
 *
 * Hack-class to use until https://github.com/swagger-api/swagger-core/issues/2320 is fixed.
 *
 * @author christianc
 *
 */
@Path("/swagger.{type:json|yaml}")
public class HackApiListingResource extends BaseApiListingResource {

	@Context
	ServletContext context;

	@GET
	@Produces({ MediaType.APPLICATION_JSON, "application/yaml" })
	@ApiOperation(value = "The swagger definition in either JSON or YAML", hidden = true)
	public Response getListing(@Context Application app, @Context ServletConfig sc, @Context HttpHeaders headers,
			@Context UriInfo uriInfo, @PathParam("type") String type) throws JsonProcessingException {
		Swagger swagger = process(app, context, sc, headers, uriInfo);
		if (swagger == null) {
			return Response.status(Status.NOT_FOUND).build();
		}
		if ("json".equals(type)) {
			return Response.ok(swagger, MediaType.APPLICATION_JSON).build();
		}
		if ("yaml".equals(type)) {
			return Response.ok(swagger, "application/yaml").build();
		}
		return Response.status(Status.NOT_FOUND).build();
	}
}

I propose this as a fix for the current ApiListingResource.java (Not anymore, see two comments below). Of course, this only works if SwaggerSerializers is also registered as a JAX-RS resource, but according to the official documentation this needs to be done anyway (but wasn't necessary before because of this bug).

@ChristianCiach
Copy link

Wtf is this anyway?

                String yaml = Yaml.mapper().writeValueAsString(swagger);
                StringBuilder b = new StringBuilder();
                String[] parts = yaml.split("\n");
                for (String part : parts) {
                    b.append(part);
                    b.append("\n");
                }
                return Response.ok().entity(b.toString()).type("application/yaml").build();

Why first split the yaml and then reconstruct it again?

@ChristianCiach
Copy link

The bug of this issue has been introduced with 3ad7595. I propose a complete revert of this commit, as I don't see the point of it. The commit message is "use custom object mapper to serialize json in order to avoid null values", but I don't see any null handling that would differ from the current implementation of SwaggerSerializers.java. Please revert this commit!

ChristianCiach pushed a commit to ChristianCiach/swagger-core that referenced this issue Aug 3, 2017
null values"

This reverts commit 3ad7595.

This fixes issue swagger-api#2320. The original commit is pointless and dangerous,
as it surprisingly overrides the behavior of SwaggerSerializers.java,
but does essentially the same (but without supporting pretty printing).
ChristianCiach pushed a commit to ChristianCiach/swagger-core that referenced this issue Aug 3, 2017
This fixes issue swagger-api#2320 for YAML documents. The original code
surprisingly overrides the behavior of SwaggerSerializers.java to
needlessly split and join a YAML document. This was originally done to
remove some kind of comment line from the YAML, but this has been
removed long ago, making split/join-code pointless.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants