Maor Lipchuk has posted comments on this change.
Change subject: core: auto generate Handler fields from annotations
......................................................................
Patch Set 3: (9 inline comments)
Partial reviewed, change look Awesome!
see questions/comments inside
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsHandler.java
Line 40: mUpdateVdsStatic =
Line 41: new ObjectIdentityChecker(VdsHandler.class,
Arrays.asList(inspectedClasses), VDSStatus.class);
Line 42:
Line 43:
Line 44: for (Pair<EditableField,String> pair :
extractyAnnotatedFields(EditableField.class, inspectedClasses)) {
extracty should be extract (extractyAnnotatedFields)
Line 45: mUpdateVdsStatic.AddPermittedFields(pair.getSecond());
Line 46: }
Line 47:
Line 48: for (Pair<EditableOnVdsStatus,String> pair :
extractyAnnotatedFields(EditableOnVdsStatus.class, inspectedClasses)) {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
Line 82: * @see Backend#InitHandlers
Line 83: */
Line 84: public static void Init() {
Line 85: Class<?>[] inspectedClassNames = new Class<?>[] {
Line 86: VmBase.class,
Why adding VmBase?
Line 87: VM.class,
Line 88: VmStatic.class,
Line 89: VmDynamic.class };
Line 90:
Line 91: mUpdateVmsStatic =
Line 92: new ObjectIdentityChecker(VmHandler.class,
Arrays.asList(inspectedClassNames), VMStatus.class);
Line 93:
Line 94: for (Pair<EditableField,String> pair :
BaseHandler.extractyAnnotatedFields(EditableField.class,
(inspectedClassNames))) {
Line 95: mUpdateVmsStatic.AddPermittedFields(pair.getSecond());
You can use here AddPermittedField instead AddPermittedFields since it is only
a String and avoid the for loop inside
Line 96: }
Line 97:
Line 98: for (Pair<EditableOnVmStatusField, String> pair :
BaseHandler.extractyAnnotatedFields(EditableOnVmStatusField.class,
Line 99: inspectedClassNames)) {
Line 94: for (Pair<EditableField,String> pair :
BaseHandler.extractyAnnotatedFields(EditableField.class,
(inspectedClassNames))) {
Line 95: mUpdateVmsStatic.AddPermittedFields(pair.getSecond());
Line 96: }
Line 97:
Line 98: for (Pair<EditableOnVmStatusField, String> pair :
BaseHandler.extractyAnnotatedFields(EditableOnVmStatusField.class,
I would use a different name then pair here (It's a bit confusing since we
already use it in the previous for loop)
Line 99: inspectedClassNames)) {
Line 100:
mUpdateVmsStatic.AddField(Arrays.asList(pair.getFirst().statuses()),
Line 101: pair.getSecond());
Line 102: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/backendinterfaces/BaseHandler.java
Line 38: * @param clz
Line 39: * class array of the scanned types.
Line 40: * @return array of pairs of a the annotation instance -> field
name
Line 41: */
Line 42: public static <A extends Annotation> List<Pair<A , String>>
extractyAnnotatedFields(Class<A> annotation, Class<?>... clz) {
Maybe switch extractyAnnotatedFields with extractAnnotatedFields
Line 43: List<Pair<A, String>> pairList = new ArrayList<Pair<A,
String>>();
Line 44: for (Class<?> clazz : clz) {
Line 45: for (Field field : clazz.getDeclaredFields()) {
Line 46: A fieldAnnotation = field.getAnnotation(annotation);
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java
Line 86: @EditableField
Line 87: private BootSequence defaultBootSequence = BootSequence.C;
Line 88:
Line 89: @EditableOnVmStatusField
Line 90: private int niceLevel;
Isn't that field should have EditableField annotation
Line 91:
Line 92: @EditableField
Line 93: private boolean autoSuspend;
Line 94:
Line 89: @EditableOnVmStatusField
Line 90: private int niceLevel;
Line 91:
Line 92: @EditableField
Line 93: private boolean autoSuspend;
Not related to this patch, but is this field used today?
Line 94:
Line 95: @EditableField
Line 96: private int priority;
Line 97:
Line 98: @EditableField
Line 99: private boolean autoStartup;
Line 100:
Line 101: @EditableOnVmStatusField
Line 102: private boolean stateless;
Isn't that field should have EditableField annotation
Line 103:
Line 104: @EditableField
Line 105: private boolean deleteProtected;
Line 106:
Line 219: this.vdsGroupId = vdsGroupId;
Line 220: this.os = os;
Line 221: this.creationDate = creationDate;
Line 222: this.description = description;
Line 223: this.memSizeMb = memSizeMB;
Not sure why changing memSizeMb, at any case it should be correlated with the
argument in the method.
Line 224: this.numOfSockets = numOfSockets;
Line 225: this.cpuPerSocket = cpusPerSocket;
Line 226: this.numOfMonitors = numOfMonitors;
Line 227: this.domain = domain;
--
To view, visit http://gerrit.ovirt.org/11916
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8441f030c161c99c630a945b8b0228114665aa17
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
