Alon Bar-Lev has posted comments on this change.
Change subject: engine : Validate parameter in canDoAction
......................................................................
Patch Set 6:
(3 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/ParametersValidator.java
Line 66: clz = clz.getSuperclass();
Line 67: }
Line 68:
Line 69: return cmdParams;
Line 70: }
Hmmm... I really do not think we need the CommandParameters class... not that
it is that important.
A Map<Boolean, List<Parameter>> can be returned by this function...
map.put(false, new ArrayList<Parameters>());
map.put(frue, new ArrayList<Parameters>());
while() {
map.get(fieldAnnotation.optional()).addAll(Arrays.asList(parameters));
}
Not that I against writing english in code... but I find it much more confusing.
Line 71:
Line 72: private static class CommandParameters {
Line 73: private List<Parameter> requiredParameters = new
ArrayList<Parameter>();
Line 74: private List<Parameter> optionalParameters = new
ArrayList<Parameter>();
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/InvalidParameterValueException.java
Line 10: ". Expected value of type " + expected.getName()
Line 11: + " but actual class was " + actual.getName());
Line 12: }
Line 13:
Line 14: public InvalidParameterValueException(String paramName, Class clz)
{
can this change be pushed higher in chain to where
InvalidParameterValueException was introduced?
Line 15: super("Invalid value for parameter "+ paramName +
Line 16: ". Could not load class " + clz.getName());
Line 17: }
Line 18:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/ParametersMap.java
Line 19: }
Line 20:
Line 21: public ParametersMap put(Parameter param, Object value) {
Line 22: if (!isAssignableFrom(param.getJavaType(), value.getClass())) {
Line 23: throw new InvalidParameterValueException(param.getName(),
param.getJavaType(), value.getClass());
I think this should be pushed higher in patch chain.
Line 24: }
Line 25: paramsMap.put(param, (Serializable) value);
Line 26: return this;
Line 27: }
--
To view, visit http://gerrit.ovirt.org/21485
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I15566c9177da28b2d47bbb6018fbfb61defcf3da
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@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
