Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
package org.jenkinsci.plugins.docker.traceability.core;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;
import com.github.dockerjava.api.command.InspectContainerResponse;
import com.github.dockerjava.api.command.InspectImageResponse;
import com.github.dockerjava.api.model.Event;
Expand Down Expand Up @@ -345,16 +346,16 @@ public void doImage(StaplerRequest req, StaplerResponse rsp,
//TODO: filtering by container ID, imageID, containerName, imageName, hostName, hostID, environment
/**
* Retrieves the latest container status via API.
* The output will be retrieved in JSON. Supports filers. Missing
* "since" and "until"
* Supports filers. Missing "since" and "until"
* @param id ID of the container, for which the info should be retrieved.
* Short container IDs are not supported.
* @param format Format used in the response
* @throws IOException Processing error
* @throws ServletException Servlet error
* @return Raw JSON output compatible with docker inspect
* @return Response available in different formats, including a JSON output compatible with docker inspect
*/
public HttpResponse doRawContainerInfo(@QueryParameter(required = true) String id)
throws IOException, ServletException {
public HttpResponse doRawContainerInfo(@QueryParameter(required = true) String id, @QueryParameter(required = false) final String format)
throws IOException, ServletException {
checkPermission(DockerTraceabilityPlugin.READ_DETAILS);

//TODO: check containerID format
Expand All @@ -368,9 +369,8 @@ public HttpResponse doRawContainerInfo(@QueryParameter(required = true) String i
return HttpResponses.error(500, "Cannot retrieve the container's status");
}

// Return raw JSON in the response
InspectContainerResponse[] out = {inspectInfo};
return toJSONResponse(out);
return toFormattedResponse(out, format);
}

//TODO: More filtering
Expand All @@ -384,15 +384,17 @@ public HttpResponse doRawContainerInfo(@QueryParameter(required = true) String i
* If the value equals to 0, the filter will be ignored (default in {@link QueryParameter}).
* @param until End time.
* If the value equals to 0, the filter will be ignored (default in {@link QueryParameter}).
* @param format Format used in the response.
* @throws IOException Processing error
* @throws ServletException Servlet error
* @return Response containing the output JSON. may be an error if something breaks.
* @return Response available in different formats. may be an error if something breaks.
*/
public HttpResponse doQueryContainer(
@QueryParameter(required = true) String id,
@QueryParameter(required = false) String mode,
@QueryParameter(required = false) long since,
@QueryParameter(required = false) long until)
@QueryParameter(required = false) long until,
@QueryParameter(required = false) String format)
throws IOException, ServletException {
checkPermission(DockerTraceabilityPlugin.READ_DETAILS);

Expand Down Expand Up @@ -443,31 +445,32 @@ public HttpResponse doQueryContainer(
}
}

// Return raw JSON in the response
return toJSONResponse(result);
return toFormattedResponse(result, format);
}

/**
* Retrieves the latest raw status via API.
* The output will be retrieved in JSON.
*
* @param id ID of the image, for which the info should be retrieved.
* Short container IDs are not supported.
* @param format Format used in the output response.
*
* @throws IOException Processing error
* @throws ServletException Servlet error
* @return {@link HttpResponse}
*
* @return Response available in different formats {@link HttpResponse}
Copy link
Member

Choose a reason for hiding this comment

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

Broken text

Copy link
Author

Choose a reason for hiding this comment

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

@oleg-nenashev I don't know what this means. I'm sorry.

Copy link
Member

Choose a reason for hiding this comment

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

The resulting text will be "Response available in different formats HttpResponse". The last word is not consistent with the sentence. "HttpResponse available in different formats" would be better

*/
public HttpResponse doRawImageInfo(@QueryParameter(required = true) String id)
throws IOException, ServletException {
public HttpResponse doRawImageInfo(@QueryParameter(required = true) String id, @QueryParameter(required = false) String format)
throws IOException, ServletException {
checkPermission(DockerTraceabilityPlugin.READ_DETAILS);

final InspectImageResponse report = DockerTraceabilityHelper.getLastInspectImageResponse(id);
if (report == null) {
return HttpResponses.error(404, "No info available for the imageId=" + id);
}

// Return raw JSON in the response
InspectImageResponse[] out = {report};
return toJSONResponse(out);
return toFormattedResponse(out, format);
}

/**
Expand Down Expand Up @@ -568,18 +571,69 @@ private synchronized void load() {
LOGGER.log(Level.SEVERE, "Failed to load the configuration from config path = "+config,e);
}
}


/**
* Represents the different response formats (JSON, pretty JSON, XML).
*/
private enum ResponseFormat {
JSON("json", "application/json;charset=UTF-8"),
PRETTYJSON("json-pretty", "application/json;charset=UTF-8"),
XML("xml", "application/xml;charset=UTF-8");
Copy link
Member

Choose a reason for hiding this comment

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

Non-standard spec. text/xml is a correct one

Copy link
Author

Choose a reason for hiding this comment

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

I'll change it but this RFC defines both as valid for "document entities" https://www.ietf.org/rfc/rfc3023.txt

Copy link
Member

Choose a reason for hiding this comment

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

Thanks a lot for checking it. In such case there's no need to change the MIME type


private String alias;

private String contentType;

private static final ResponseFormat DEFAULT = JSON;

private ResponseFormat(final String alias, final String contentType) {
this.alias = alias;
Copy link
Member

Choose a reason for hiding this comment

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

why store the alias when you are switching on the alias below anyways?

Copy link
Author

Choose a reason for hiding this comment

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

@rsandell Ok. I thought it would be useful in the future. Removed.

this.contentType = contentType;
}

public static ResponseFormat fromAlias(final String alias) {
if (alias == null) {
return DEFAULT;
}
if (alias.equals("json")) {
Copy link
Member

Choose a reason for hiding this comment

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

equalsIgnoreCase would be a tiny bit nicer?

Copy link
Author

Choose a reason for hiding this comment

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

@rsandell If you use equalsIgnoreCase these URLs will be the same:

  1. http://localhost:8080/jenkins/docker-traceability/rawContainerInfo?format=json-pretty&id=...
  2. http://localhost:8080/jenkins/docker-traceability/rawContainerInfo?format=JSON-PRETTY&id=...

But according to the specification QueryParams are case-sensitive:

The scheme and host are case-insensitive and normally provided in lowercase; all other components are compared in a case-sensitive manner.

IMHO, I prefer to use equals in this case.

return JSON;
} else if (alias.equals("json-pretty")) {
return PRETTYJSON;
} else if (alias.equals("xml")) {
// Related to https://issues.jenkins-ci.org/browse/JENKINS-28727
throw new IllegalStateException("Unsupported format: " + alias);
} else {
throw new IllegalStateException("Unsupported format: " + alias);
}
}

public String getAlias() {
return alias;
}

public String getContentType() {
return contentType;
}
}

/**
* Serves the JSON response.
* @param item Data to be serialized to JSON
* @return HTTP response with application/json MIME type
* Serves the response and manages its output format in the response.
*
* @param item Data to be serialized
* @param format Format used in the response
*
* @return HTTP response with MIME type
*/
private static HttpResponse toJSONResponse(final Object item) {
private static HttpResponse toFormattedResponse(final Object item, final String format) {
return new HttpResponse() {
@Override
public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException {
ObjectMapper mapper = new ObjectMapper();
rsp.setContentType("application/json;charset=UTF-8");
ObjectMapper mapper = new ObjectMapper();
ResponseFormat responseFormat = ResponseFormat.fromAlias(format);
rsp.setContentType(responseFormat.getContentType());
if (responseFormat.equals(ResponseFormat.PRETTYJSON)) {
Copy link
Member

Choose a reason for hiding this comment

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

I would also keep it in enum class, but it's optional at the current stage

mapper.enable(SerializationFeature.INDENT_OUTPUT);
}
mapper.writeValue(rsp.getWriter(), item);
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
package org.jenkinsci.plugins.docker.traceability.model;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;
import java.io.IOException;
import org.jenkinsci.plugins.docker.traceability.samples.JSONSamples;
import static org.junit.Assert.assertEquals;
Expand All @@ -40,6 +41,7 @@ public class DockerTraceabilityReportTest {
@Test
public void jsonRoundTrip() throws IOException {
ObjectMapper mapper = new ObjectMapper();
mapper.enable(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS);
Copy link
Member

Choose a reason for hiding this comment

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

Yes, it may help with the unstable test

DockerTraceabilityReport report = JSONSamples.submitReport.readObject(DockerTraceabilityReport.class);
assertNotNull(report.getContainer());
assertNotNull(report.getEvent());
Expand Down