Alon Bar-Lev has posted comments on this change.
Change subject: engine : Validate parameter in canDoAction
......................................................................
Patch Set 5:
(9 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
Line 144:
Line 145: private Map<Guid, CommandBase<?>> childCommandsMap = new
HashMap<>();
Line 146: private Map<Guid, Pair<VdcActionType, VdcActionParametersBase>>
childCommandInfoMap = new HashMap<>();
Line 147:
Line 148: @Parameters(type = Parameters.Type.common)
why can't these be required? what is special in common?
Line 149: final Parameter[] commonParameters = {
Line 150: CoreParameters.GUID_ID,
Line 151: CoreParameters.SESSION_ID,
Line 152: CoreParameters.PARAMETERS_CURRENT_USER,
Line 145: private Map<Guid, CommandBase<?>> childCommandsMap = new
HashMap<>();
Line 146: private Map<Guid, Pair<VdcActionType, VdcActionParametersBase>>
childCommandInfoMap = new HashMap<>();
Line 147:
Line 148: @Parameters(type = Parameters.Type.common)
Line 149: final Parameter[] commonParameters = {
private?
can these can be static? so you walk the classes objects based on inheritance?
Line 150: CoreParameters.GUID_ID,
Line 151: CoreParameters.SESSION_ID,
Line 152: CoreParameters.PARAMETERS_CURRENT_USER,
Line 153: CoreParameters.SHOULD_BE_LOGGED,
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/QueriesCommandBase.java
Line 38: private final DbUser user;
Line 39: private final P parameters;
Line 40: private boolean isInternalExecution = false;
Line 41: @Parameters(type = Parameters.Type.common)
Line 42: final Parameter[] commonParameters = {
CoreParameters.HTTP_SESSION_ID, CoreParameters.FILTERED, CoreParameters.REFRESH
};
each one on own line to allow future add without modifying entire line.
Line 43:
Line 44: public QueriesCommandBase(P parameters) {
Line 45: this.parameters = parameters;
Line 46: returnValue = new VdcQueryReturnValue();
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/CommandParameters.java
Line 5: import java.util.List;
Line 6:
Line 7: import org.ovirt.engine.core.common.utils.Parameter;
Line 8:
Line 9: public class CommandParameters {
can this be nested class within whatever use it?
Line 10: private List<Parameter> requiredParameters = new
ArrayList<Parameter>();
Line 11: private List<Parameter> optionalParameters = new
ArrayList<Parameter>();
Line 12: private List<Parameter> commonParameters = new
ArrayList<Parameter>();
Line 13:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/Parameters.java
Line 7:
Line 8: @Target(ElementType.FIELD)
Line 9: @Retention(RetentionPolicy.RUNTIME)
Line 10: public @interface Parameters {
Line 11: public enum Type {optional, required, common};
why do you need the common?
Line 12: Type type() default Type.required;
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/ParametersValidator.java
Line 20: public static void validateParameters(QueriesCommandBase cmd) {
Line 21: validateParameters(getCommandParameters(cmd),
cmd.getParameters());
Line 22: }
Line 23:
Line 24: public static void validateParameters(CommandBase cmd) {
why not take object and map?
public static void validateParameters(Object o, ParametersMap map) {
# get @Parameters annotation
# check aginst map
}
this can be generic and not related to anything in application.
please see it as (Parameters, ParametersMap, @Parameters, ParametersValidator)
can be used in other projects as well.
Line 25: validateParameters(getCommandParameters(cmd),
cmd.getParameters());
Line 26: }
Line 27:
Line 28: private static CommandParameters getCommandParameters(Object cmd) {
Line 48: private static Parameter[] getParameterValues(Object cmd, Field
field) throws IllegalAccessException {
Line 49: if (cmd instanceof QueriesCommandBase) {
Line 50: return (Parameter[]) ((QueriesCommandBase)
cmd).getFieldValue(field);
Line 51: }
Line 52: return (Parameter[]) ((CommandBase) cmd).getFieldValue(field);
why is this needed? why can't we only walk the parameters map?
Line 53: }
Line 54:
Line 55: private static void validateParameters(CommandParameters
cmdParams, ParametersMap paramsMap) {
Line 56: if (cmdParams.getRequiredParameters().isEmpty()) {
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/serialization/json/JsonObjectDeserializer.java
Line 49: JsonVmManagementParametersBaseMixIn.class);
Line 50:
formattedMapper.getDeserializationConfig().addMixInAnnotations(VmBase.class,
JsonVmBaseMixIn.class);
Line 51:
formattedMapper.getDeserializationConfig().addMixInAnnotations(VmStatic.class,
JsonVmStaticMixIn.class);
Line 52: formattedMapper.configure(Feature.FAIL_ON_UNKNOWN_PROPERTIES,
false);
Line 53: SimpleModule module = new SimpleModule("ParameterModule", new
Version(1, 0, 0, "M"));
this does not belong here
Line 54: module.addKeyDeserializer(Parameter.class, new
ParameterDeserializer());
Line 55: formattedMapper.registerModule(module);
Line 56: formattedMapper.enableDefaultTyping();
Line 57: }
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/serialization/json/JsonObjectSerializer.java
Line 52:
formattedMapper.getSerializationConfig().addMixInAnnotations(VmBase.class,
JsonVmBaseMixIn.class);
Line 53:
formattedMapper.getSerializationConfig().addMixInAnnotations(VmStatic.class,
JsonVmStaticMixIn.class);
Line 54:
formattedMapper.getSerializationConfig().addMixInAnnotations(VmPayload.class,
JsonVmPayloadMixIn.class);
Line 55: formattedMapper.configure(Feature.INDENT_OUTPUT, true);
Line 56: SimpleModule module = new SimpleModule("ParameterModule", new
Version(1, 0, 0, "M"));
this does not belong here
Line 57: module.addKeySerializer(Parameter.class, new
ParameterSerializer());
Line 58: formattedMapper.registerModule(module);
Line 59: formattedMapper.enableDefaultTyping();
Line 60: }
--
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: 5
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
