Ori Liel has posted comments on this change. Change subject: restapi: Add ability to extract rsdl.xml from jar ......................................................................
Patch Set 1: (8 comments) http://gerrit.ovirt.org/#/c/24385/1/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/rsdl/RsdlIOManager.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/rsdl/RsdlIOManager.java: Line 10: public class RsdlIOManager { Line 11: Line 12: private static final int DEFAULT_BUFFER_SIZE = 1024 * 4; Line 13: public static final String RSDL_FILE_NAME = "/rsdl.xml"; Line 14: public static final String GLUSTER_RSDL_FILE_NAME = "/rsdl_gluster.xml"; > These ^aren't file names, rather resource names. I suggest to name them _RE Done Line 15: Line 16: public static void main(String[] args) throws IOException { Line 17: System.out.println("Copying rsdl.xml, rsdl_gluster.xml to: " + args[0]); Line 18: copyRsdl(args[0]); Line 13: public static final String RSDL_FILE_NAME = "/rsdl.xml"; Line 14: public static final String GLUSTER_RSDL_FILE_NAME = "/rsdl_gluster.xml"; Line 15: Line 16: public static void main(String[] args) throws IOException { Line 17: System.out.println("Copying rsdl.xml, rsdl_gluster.xml to: " + args[0]); > We will probably run this from maven, and will use it to write to a "target Done Line 18: copyRsdl(args[0]); Line 19: } Line 20: Line 21: public static void copyRsdl(String outputDirectory) throws IOException { Line 18: copyRsdl(args[0]); Line 19: } Line 20: Line 21: public static void copyRsdl(String outputDirectory) throws IOException { Line 22: copy(loadAsStream(RSDL_FILE_NAME), new FileWriter(new File(outputDirectory + RSDL_FILE_NAME))); > It isn't good to create the File object concatenating the directory and the Done Line 23: copy(loadAsStream(GLUSTER_RSDL_FILE_NAME), new FileWriter(new File(outputDirectory + GLUSTER_RSDL_FILE_NAME))); Line 24: } Line 25: Line 26: public static InputStream loadAsStream(String fileName) { Line 19: } Line 20: Line 21: public static void copyRsdl(String outputDirectory) throws IOException { Line 22: copy(loadAsStream(RSDL_FILE_NAME), new FileWriter(new File(outputDirectory + RSDL_FILE_NAME))); Line 23: copy(loadAsStream(GLUSTER_RSDL_FILE_NAME), new FileWriter(new File(outputDirectory + GLUSTER_RSDL_FILE_NAME))); > This never closes the streams. Try to do something like this: Done Line 24: } Line 25: Line 26: public static InputStream loadAsStream(String fileName) { Line 27: return RsdlIOManager.class.getResourceAsStream(fileName); Line 22: copy(loadAsStream(RSDL_FILE_NAME), new FileWriter(new File(outputDirectory + RSDL_FILE_NAME))); Line 23: copy(loadAsStream(GLUSTER_RSDL_FILE_NAME), new FileWriter(new File(outputDirectory + GLUSTER_RSDL_FILE_NAME))); Line 24: } Line 25: Line 26: public static InputStream loadAsStream(String fileName) { > I would use resourceName instead of fileName. Done Line 27: return RsdlIOManager.class.getResourceAsStream(fileName); Line 28: } Line 29: Line 30: public static void copy(InputStream input, Writer output) throws IOException { Line 26: public static InputStream loadAsStream(String fileName) { Line 27: return RsdlIOManager.class.getResourceAsStream(fileName); Line 28: } Line 29: Line 30: public static void copy(InputStream input, Writer output) throws IOException { > If the first parameter is a input stream the second should be an output str Done Line 31: InputStreamReader in = new InputStreamReader(input); Line 32: char[] buffer = new char[DEFAULT_BUFFER_SIZE]; Line 33: int n = 0; Line 34: while (-1 != (n = in.read(buffer))) { Line 28: } Line 29: Line 30: public static void copy(InputStream input, Writer output) throws IOException { Line 31: InputStreamReader in = new InputStreamReader(input); Line 32: char[] buffer = new char[DEFAULT_BUFFER_SIZE]; > Use byte[], not char[], together with stream instead of writer. Done Line 33: int n = 0; Line 34: while (-1 != (n = in.read(buffer))) { Line 35: output.write(buffer, 0, n); Line 36: } http://gerrit.ovirt.org/#/c/24385/1/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/rsdl/RsdlManager.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/rsdl/RsdlManager.java: Line 54: String fileName = Line 55: applicationMode == ApplicationMode.AllModes ? RsdlIOManager.RSDL_FILE_NAME Line 56: : RsdlIOManager.GLUSTER_RSDL_FILE_NAME; Line 57: InputStream rsdlAsStrem = RsdlIOManager.loadAsStream(fileName); Line 58: return JAXB.unmarshal(rsdlAsStrem, RSDL.class); > The input stream is never closed here. Done Line 59: } Line 60: Line 61: private static void serializeRsdl(RSDL rsdl, String rsdlLocation) { Line 62: JAXB.marshal(rsdl, new File(rsdlLocation)); -- To view, visit http://gerrit.ovirt.org/24385 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I11834dac081e17c702c614ae923575a3debba6eb Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ori Liel <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Ori Liel <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
