Alon Bar-Lev has posted comments on this change. Change subject: utils: pki-resource: cleanup: levarage enum methods for content type ......................................................................
Patch Set 5: (4 comments) Hi, I am very sorry... but it does not makes it any simpler, just more complex. It should be exact opposite... having the resources as enum not the formats, and allow get the format based on specific format. You also created dependency between servlet interface and internal representation of enum, which is bad especially in java environment in which every programmer can just rename enums and breaks application. So although I appreciate your effort, I will leave this as-is for now. Thanks! I will reuse the servlet utils though. Alon .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetCACertificateQuery.java Line 12: protected void executeQueryCommand() { Line 13: try { Line 14: getQueryReturnValue().setSucceeded(false); Line 15: getQueryReturnValue().setReturnValue( Line 16: PKIResources.X509_PEM.getAsString() how do you select which certificate? Line 17: ); Line 18: getQueryReturnValue().setSucceeded(true); Line 19: } catch (Exception e) { Line 20: getQueryReturnValue().setExceptionString(e.getMessage()); .................................................... File backend/manager/modules/root/src/main/webapp/WEB-INF/web.xml Line 19: <param-value>/pki-resource</param-value> Line 20: </init-param> Line 21: <init-param> Line 22: <param-name>attr-resource</param-name> Line 23: <param-value>X509_PEM_CA</param-value> you cannot change this, you break the interface of installer. there cannot be dependency between interface and internal enum names. Line 24: </init-param> Line 25: </servlet> Line 26: <servlet-mapping> Line 27: <servlet-name>PKIResourceServlet.ca</servlet-name> .................................................... File backend/manager/modules/services/src/main/java/org/ovirt/engine/core/services/PKIResourceServlet.java Line 42: } Line 43: PKIResources r = PKIResources.valueOf(resourceString); Line 44: Format format = r.getFormat(); Line 45: if (formatString != null) { Line 46: format = Format.valueOf(formatString); if someone "refactor" the java, it will break the entire application, as the servlet interface will change and the current issued resources will be broken. Line 47: } Line 48: Line 49: if (alias == null) { Line 50: alias = r.getAlias(); .................................................... File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/PKIResources.java Line 25: * </pre> Line 26: */ Line 27: public enum PKIResources { Line 28: Line 29: X509_PEM_CA("application/x-x509-cert", null, getCertificate(EngineLocalConfig.getInstance().getPKICACert()), Format.PEM), the X509_PEM_CA is format it is not specific certificate... I should be able to get any certificate in any format. you can have entry for engine-ca in X509_PEM format, engine-ca in X509_PEM_CA format, etc... but this only makes it more complex. Line 30: X509_PEM("application/x-x509-ca-cert", null, getCertificate(EngineLocalConfig.getInstance().getPKIEngineCert()), Format.PEM), Line 31: OPENSSH_PUBKEY("text/plain", "ovirt-engine", getCertificate(EngineLocalConfig.getInstance().getPKIEngineCert()), Format.OPENSSH); Line 32: Line 33: private final String contentType; -- To view, visit http://gerrit.ovirt.org/21073 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I24da0ff174599ffdeabbf5846eab429bf0d6510d Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> 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