Juan Hernandez has posted comments on this change. Change subject: restApi : RestApi to export volume profile to pdf ......................................................................
Patch Set 20: (7 comments) I miss the changes in the .spec to use the FOP version provided by the distribution. http://gerrit.ovirt.org/#/c/28340/20/backend/manager/dependencies/pom.xml File backend/manager/dependencies/pom.xml: Line 240: <dependency> Line 241: <groupId>org.apache.xmlgraphics</groupId> Line 242: <artifactId>fop</artifactId> Line 243: <version>${fop.version}</version> Line 244: <scope>provided</scope> No need for "provided" here, please remove it. Line 245: </dependency> Line 246: Line 247: </dependencies> Line 248: http://gerrit.ovirt.org/#/c/28340/20/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/gluster/GlusterVolumeResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/gluster/GlusterVolumeResource.java: Line 101: Line 102: @GET Line 103: @Produces("application/pdf") Line 104: @Path("profilestatistics") Line 105: public ByteArrayOutputStream getProfileStatisticsAsPdf(); Add an empty line after the method declaration. Line 106: /** Line 107: * Sub-resource locator method, returns GlusterBricksResource on which the remainder of the URI is dispatched. Line 108: * Line 109: * @return matching subresource if found http://gerrit.ovirt.org/#/c/28340/20/backend/manager/modules/restapi/jaxrs/pom.xml File backend/manager/modules/restapi/jaxrs/pom.xml: Line 110: <dependency> Line 111: <groupId>org.apache.xmlgraphics</groupId> Line 112: <artifactId>fop</artifactId> Line 113: <version>${fop.version}</version> Line 114: <scope>provided</scope> Fix indentation: two spaces per level (no tabs). Line 115: </dependency> Line 116: Line 117: </dependencies> Line 118: http://gerrit.ovirt.org/#/c/28340/20/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/gluster/BackendGlusterVolumeResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/gluster/BackendGlusterVolumeResource.java: Line 218: } Line 219: } Line 220: Line 221: @Override Line 222: public ByteArrayOutputStream getProfileStatisticsAsPdf() { Instead of putting all this logic here I think it is better to externalize it to a custom message body writer. If you do so you will need to put the @Produces("application/pdf") annotation in the original "getProfileStatistcs" method: @GET @Formatted @Path("profilestatistics") @Produces({ ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML, ApiMediaType.APPLICATION_PDF }) public GlusterVolumeProfileDetails getProfileStatistics(); (You will have to add APPLICATION_PDF to ApiMediaType as well). That tells the JAX-RS framework (Resteasy) that this resource can produce PDF output. Now you need to tell JAX-RS how to actually produce PDF from GlusterVolumeProfileDetails. You can do that with a custom message body writer (an implementation of the MessageBodyWriter interface) annotated with @Produces("application/pdf". In that message body writer you can do create the JAXB context only once, as it is expensive, in the constructor: public class FOPMessageBodyWriter ... { private JAXBContext jaxbContext; public FOPMessageBodywriter() { String modelPackage = API.class.getPackage().getName(); jaxbContext = JAXBContext.newInstance(modelPackage); } } For the transformation you should try to avoid an intermediate buffer, and an intermediate file. You can use the JAXBSource class for that, to convert the object directly to a sequence of XML parsing events, that will be consumed by the transformer: Source source = new JAXBSource(jaxbContext, profileDetails); Putting all this PDF generation logic in the message body writer simplifies the resource, and makes the PDF generation potentially reusable. Line 223: GlusterVolumeProfileDetails profileDeatils = getProfileStatistics(); Line 224: try { Line 225: JAXBContext context = JAXBContext.newInstance(GlusterVolumeProfileDetails.class); Line 226: ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); Line 239: public static final String EXTENSION = ".pdf"; Line 240: public String PRESCRIPTION_URL = "template.xsl"; Line 241: Line 242: public String createPDFFile(ByteArrayOutputStream xmlSource, URL templateFilePath) throws IOException { Line 243: File file = File.createTempFile("" + System.currentTimeMillis(), EXTENSION); It is better to avoid using a temporary file, but if you really need it then it is vital that you remember to close and remove it. This should be done with a try-finally: File file = ...; try { file = ...; } finally { if (file != null) { file.delete(); } } Line 244: StreamSource transformSource = new StreamSource(templateFilePath.openStream()); Line 245: FopFactory fopFactory = FopFactory.newInstance(); Line 246: FOUserAgent foUserAgent = fopFactory.newFOUserAgent(); Line 247: ByteArrayOutputStream pdfoutStream = new ByteArrayOutputStream(); Line 265: str.close(); Line 266: out.close(); Line 267: Line 268: } catch (TransformerException e) { Line 269: e.printStackTrace(); Exceptions should be handled or sent to the log, never printed to standard output: throw new IOException(e); Line 270: } Line 271: } catch (FOPException e) { Line 272: e.printStackTrace(); Line 273: } http://gerrit.ovirt.org/#/c/28340/20/ear/src/main/resources/META-INF/MANIFEST.MF File ear/src/main/resources/META-INF/MANIFEST.MF: Line 4: org.apache.commons.beanutils, Line 5: org.apache.commons.codec, Line 6: org.apache.commons.collections, Line 7: org.apache.commons.httpclient, Line 8: org.apache.commons.lang, To keep this list alphabetically sorted the new dependency should go after this ^line. Line 9: org.apache.log4j, Line 10: org.apache.xmlrpc, Line 11: org.codehaus.jackson.jackson-core-asl, Line 12: org.codehaus.jackson.jackson-mapper-asl, -- To view, visit http://gerrit.ovirt.org/28340 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie3ff28f5cf18bd7a2bcb53a169873fe6ae3541ed Gerrit-PatchSet: 20 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: anmolbabu <anb...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Ramesh N <rnach...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: Shubhendu Tripathi <shtri...@redhat.com> Gerrit-Reviewer: anmolbabu <anb...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
