Alexander Wels has posted comments on this change.
Change subject: utils: pki-resource: cleanup: levarage enum methods for content
type
......................................................................
Patch Set 5:
(4 comments)
....................................................
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()
X509_PEM vs X509_PEM_CA
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>
vs a dependency between the interface and some hard coded string? How is that
any better?
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 refactors this it will break the entire application as well:
resources = new HashMap<String, PKIResources.Resource>();
resources.put("ca-certificate", PKIResources.Resource.CACertificate);
resources.put("engine-certificate", PKIResources.Resource.EngineCertificate);
Here it is just some random string vs a stronly typed enumeration.
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),
You really only have 2 certificates.
The CA,
The engine
Each of which can have 2 formats. PEM or open ssh.
This ENUMERATION, simply enumerates all the possible values that are valid
right now. This enumeration is just a short cut to have all related information
together.
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
