anmolbabu has posted comments on this change. Change subject: restapi : RestApi to export volume profile to pdf ......................................................................
Patch Set 24: (16 comments) http://gerrit.ovirt.org/#/c/28340/24/backend/manager/dependencies/pom.xml File backend/manager/dependencies/pom.xml: Line 265: <groupId>commons-io</groupId> Line 266: <artifactId>commons-io</artifactId> Line 267: <version>${commons-io.version}</version> Line 268: </dependency> Line 269: > Remember to fix this whitespace problem before merging. Done Line 270: <dependency> Line 271: <groupId>org.apache.xmlgraphics</groupId> Line 272: <artifactId>batik-util</artifactId> Line 273: <version>${batik.version}</version> http://gerrit.ovirt.org/#/c/28340/24/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/pdf/FOPMessageBodyWriter.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/pdf/FOPMessageBodyWriter.java: Line 58: return true; Line 59: } Line 60: Line 61: @Override Line 62: public void writeTo(final Object data, Class<?> dataClass, Type arg2, Annotation[] annotation, MediaType mediaType, MultivaluedMap<String, Object> map, OutputStream outputStream) throws IOException, WebApplicationException { > Name the parameters as in the definition of the interface: Done Line 63: try { Line 64: new FOPMessageBodyWriter(); Line 65: Line 66: Source source = new JAXBSource(jaxbContext, data); Line 60: Line 61: @Override Line 62: public void writeTo(final Object data, Class<?> dataClass, Type arg2, Annotation[] annotation, MediaType mediaType, MultivaluedMap<String, Object> map, OutputStream outputStream) throws IOException, WebApplicationException { Line 63: try { Line 64: new FOPMessageBodyWriter(); > Don't need to create an instance here. Done Line 65: Line 66: Source source = new JAXBSource(jaxbContext, data); Line 67: Line 68: String xslName = "/" + dataClass.getSimpleName() + "AsPdf.xsl"; Line 70: StreamSource transformSource = new StreamSource(templateStream); Line 71: Line 72: FopFactory fopFactory = FopFactory.newInstance(); Line 73: FOUserAgent foUserAgent = fopFactory.newFOUserAgent(); Line 74: TransformerFactory transfact = TransformerFactory.newInstance(); > Is it possible to create these objects in the constructor and reuse them? C Done Line 75: Line 76: final ByteArrayOutputStream pdfoutStream = new ByteArrayOutputStream(); Line 77: Transformer xslfoTransformer = transfact.newTransformer(transformSource); Line 78: Fop fop = fopFactory.newFop(MimeConstants.MIME_PDF, foUserAgent, pdfoutStream); Line 74: TransformerFactory transfact = TransformerFactory.newInstance(); Line 75: Line 76: final ByteArrayOutputStream pdfoutStream = new ByteArrayOutputStream(); Line 77: Transformer xslfoTransformer = transfact.newTransformer(transformSource); Line 78: Fop fop = fopFactory.newFop(MimeConstants.MIME_PDF, foUserAgent, pdfoutStream); > No need for the pdfoutStream, you can generate the PDF document directly to Done Line 79: Result res = new SAXResult(fop.getDefaultHandler()); Line 80: xslfoTransformer.transform(source, res); Line 81: Line 82: StreamingOutput stream = new StreamingOutput() { Line 82: StreamingOutput stream = new StreamingOutput() { Line 83: public void write(OutputStream output) throws IOException, WebApplicationException { Line 84: output.write(pdfoutStream.toByteArray()); Line 85: } Line 86: }; > This stream isn't necessary. Done Line 87: Response.ok(stream).type("application/pdf").header("content-disposition", "attachment; filename=\"stats.pdf\"").build(); Line 88: } catch(Exception e) { Line 89: log.error(e.toString()); Line 90: } Line 83: public void write(OutputStream output) throws IOException, WebApplicationException { Line 84: output.write(pdfoutStream.toByteArray()); Line 85: } Line 86: }; Line 87: Response.ok(stream).type("application/pdf").header("content-disposition", "attachment; filename=\"stats.pdf\"").build(); > This is completely ignored, as you aren't doing anything with the created R Done Line 88: } catch(Exception e) { Line 89: log.error(e.toString()); Line 90: } Line 91: } http://gerrit.ovirt.org/#/c/28340/24/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 100: ApiMediaType.APPLICATION_XML, Line 101: ApiMediaType.APPLICATION_JSON, Line 102: ApiMediaType.APPLICATION_X_YAML, Line 103: ApiMediaType.APPLICATION_PDF Line 104: }) > Fix the indentation of the }) , should start in the same column that @Produ Done Line 105: public GlusterVolumeProfileDetails getProfileStatistics(); Line 106: Line 107: /** Line 108: * Sub-resource locator method, returns GlusterBricksResource on which the remainder of the URI is dispatched. http://gerrit.ovirt.org/#/c/28340/24/ovirt-engine.spec.in File ovirt-engine.spec.in: Line 192: Requires: %{name}-dbscripts = %{version}-%{release} Line 193: Requires: %{name}-restapi = %{version}-%{release} Line 194: Requires: %{name}-tools = %{version}-%{release} Line 195: Requires: %{name}-userportal = %{version}-%{release} Line 196: Requires: %{name}-webadmin-portal = %{version}-%{release} > The "Requires: fop" should go here, as it is available in all the distribut Done Line 197: Requires: %{name}-websocket-proxy >= %{version}-%{release} Line 198: Requires: java Line 199: Requires: java-1.7.0-openjdk >= 1:1.7.0.9-2.3.3.2 Line 200: Requires: jpackage-utils Line 223: Requires: postgresql-jdbc Line 224: Requires: quartz Line 225: Requires: snmp4j Line 226: Requires: spring-ldap Line 227: Requires: fop > Use a tab to indent, no spaces, to be consistent with the rest of the "Requ Done Line 228: Requires: springframework-aop Line 229: Requires: springframework-beans Line 230: Requires: springframework-context Line 231: Requires: springframework-expression Line 689: dst="%{engine_jboss_modules}/${dst}" Line 690: src="%{_javadir}/${src}" Line 691: rm -f "%{buildroot}${dst}" Line 692: ln -s "${src}" "%{buildroot}${dst}" Line 693: done << __EOF__ > The line for fop should go here, as it is available in all the distribution Done Line 694: org/apache/ws/commons/main/ws-commons-util.jar Line 695: org/ovirt/otopi/main/otopi.jar otopi/otopi.jar Line 696: org/ovirt/ovirt-host-deploy/main/ovirt-host-deploy.jar ovirt-host-deploy/ovirt-host-deploy.jar Line 697: org/ovirt/vdsm-jsonrpc-java/main/vdsm-jsonrpc-java-client.jar vdsm-jsonrpc-java/vdsm-jsonrpc-java-client.jar Line 705: com/woorea/openstack/sdk/main/openstack-client.jar openstack-java-sdk/openstack-client.jar Line 706: com/woorea/openstack/sdk/main/quantum-client.jar openstack-java-sdk/quantum-client.jar Line 707: com/woorea/openstack/sdk/main/quantum-model.jar openstack-java-sdk/quantum-model.jar Line 708: com/woorea/openstack/sdk/main/resteasy-connector.jar openstack-java-sdk/resteasy-connector.jar Line 709: org/apache/fop/main/fop.jar > This is not specific to Fedora, should go in the previous section. Done Line 710: org/aopalliance/main/aopalliance.jar Line 711: org/apache/commons/compress/main/commons-compress.jar Line 712: org/apache/commons/configuration/main/commons-configuration.jar commons-configuration.jar Line 713: org/apache/commons/httpclient/main/commons-httpclient.jar http://gerrit.ovirt.org/#/c/28340/24/pom.xml File pom.xml: Line 95: <fop.version>1.0</fop.version> Line 96: <xmlgraphics-commons.version>1.4</xmlgraphics-commons.version> Line 97: <batik.version>1.7</batik.version> Line 98: <apache-avalon-api.version>4.3.1</apache-avalon-api.version> Line 99: <apache-avalon-impl.version>4.3.1</apache-avalon-impl.version> > Use only one property for avalon: Done Line 100: <apache-batick-all.version>1.7</apache-batick-all.version> Line 101: <xalan.version>2.6.0</xalan.version> Line 102: <servlet-api.version>2.5</servlet-api.version> Line 103: <commons-io.version>1.3</commons-io.version> Line 96: <xmlgraphics-commons.version>1.4</xmlgraphics-commons.version> Line 97: <batik.version>1.7</batik.version> Line 98: <apache-avalon-api.version>4.3.1</apache-avalon-api.version> Line 99: <apache-avalon-impl.version>4.3.1</apache-avalon-impl.version> Line 100: <apache-batick-all.version>1.7</apache-batick-all.version> > I think this isn't needed, you already "have bakit.version". Done Line 101: <xalan.version>2.6.0</xalan.version> Line 102: <servlet-api.version>2.5</servlet-api.version> Line 103: <commons-io.version>1.3</commons-io.version> Line 104: <c3p0.version>0.9.1.1</c3p0.version> Line 97: <batik.version>1.7</batik.version> Line 98: <apache-avalon-api.version>4.3.1</apache-avalon-api.version> Line 99: <apache-avalon-impl.version>4.3.1</apache-avalon-impl.version> Line 100: <apache-batick-all.version>1.7</apache-batick-all.version> Line 101: <xalan.version>2.6.0</xalan.version> > This isn't needed. Done Line 102: <servlet-api.version>2.5</servlet-api.version> Line 103: <commons-io.version>1.3</commons-io.version> Line 104: <c3p0.version>0.9.1.1</c3p0.version> Line 105: <aopalliance.version>1.0</aopalliance.version> Line 98: <apache-avalon-api.version>4.3.1</apache-avalon-api.version> Line 99: <apache-avalon-impl.version>4.3.1</apache-avalon-impl.version> Line 100: <apache-batick-all.version>1.7</apache-batick-all.version> Line 101: <xalan.version>2.6.0</xalan.version> Line 102: <servlet-api.version>2.5</servlet-api.version> > This isn't needed. Done Line 103: <commons-io.version>1.3</commons-io.version> Line 104: <c3p0.version>0.9.1.1</c3p0.version> Line 105: <aopalliance.version>1.0</aopalliance.version> Line 106: <snmp4j.version>2.2.2</snmp4j.version> -- 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: 24 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
