From ofrenkel at redhat.com Thu Apr 12 03:50:38 2012 Content-Type: multipart/mixed; boundary="===============6840676888591997515==" MIME-Version: 1.0 From: Omer Frenkel To: devel at ovirt.org Subject: Re: [Engine-devel] Query regarding ValidationUtils#validateInputs Date: Thu, 12 Apr 2012 03:50:37 -0400 Message-ID: <15a5329c-4579-4ef6-b1c8-be0b260e859b@zmail07.collab.prod.int.phx2.redhat.com> In-Reply-To: 4F8677AD.2090304@redhat.com --===============6840676888591997515== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable --=3D_e21defe9-d4a0-4579-ac6e-9f383c6664d2 Content-Type: text/plain; charset=3Dutf-8 Content-Transfer-Encoding: 7bit ----- Original Message ----- > From: "Shireesh Anjal" > To: engine-devel(a)ovirt.org > Sent: Thursday, April 12, 2012 9:35:25 AM > Subject: [Engine-devel] Query regarding > ValidationUtils#validateInputs > Hi, > This is regarding the following validation method we have in > ValidationUtils: > public static ArrayList > validateInputs(List> validationGroupList, T parameters); > I there any particular reason for supporting the validations only on > objects of classes derived from VdcActionParametersBase? I guess > this was done because this method is primarily intended to validate > the action parameters passed to a BLL action, using the validation > annotations on the parameter class. However I think this method can > be useful for general use as well. e.g. I cannot add a "@Valid" > annotation on a "list" or a "map" in a parameter class. So I need to > iterate over the list/map, and validate each element inside the > loop. The validation inside the loop can also utilize the above > method if the restriction "extends VdcActionParametersBase" is > removed. This will allow me to do the following in the canDoAction > method: > protected boolean canDoAction() { > ... > for(GlusterBrickEntity brick : > getParameters().getGlusterVolume().getBricks()) { > List errors =3D > ValidationUtils.validateInputs(getValidationGroups(), brick); > if(errors !=3D null) { > for(String error : errors) { > addCanDoActionMessage(error); > } > } > } > ... > } > Regards, > Shireesh > _______________________________________________ > Engine-devel mailing list > Engine-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/engine-devel i don't think there is a reason to restrict only for VdcActionParametersBas= e, roy what do you think? = also you can use here = getReturnValue().getCanDoActionMessages().addAll(errors); = instead of going over on all errors. = --=3D_e21defe9-d4a0-4579-ac6e-9f383c6664d2 Content-Type: text/html; charset=3Dutf-8 Content-Transfer-Encoding: 7bit <= div style=3D'font-family: Times New Roman; font-size: 12pt; color: #000000'= >

From: "Shireesh Anjal" <sanjal(a)redhat.com>= ;
To: engine-devel(a)ovirt.org
Sent: Thursday, April 12= , 2012 9:35:25 AM
Subject: [Engine-devel] Query regarding Validat= ionUtils#validateInputs

= = = = Hi,

This is regarding the following validation method we have in ValidationUtils:

public static <T extends VdcActionParametersBase> ArrayList<String> validateInputs(List<Class<?>> validationGroupList, T parameters);

I there any particular reason for supporting the validations only on objects of classes derived from VdcActionParametersBase? I guess this was done because this method is primarily intended to validate the action parameters passed to a BLL action, using the validation annotations on the parameter class. However I think this method can be useful for general use as well. e.g. I cannot add a "@Valid" annotation on a "list" or a "map" in a parameter class. So I need to iterate over the list/map, and validate each element inside the loop. The validation inside the loop can also utilize the above method if the restriction "extends VdcActionParametersBase" is removed. This will allow me to do the following in the canDoAction method:

protected boolean canDoAction() {
...
    for(GlusterBrickEntity brick : getParameters().getGlusterVolume().getBricks()) {
        List<String> errors =3D ValidationUtils.validateInputs(getValidationGroups(), brick);
        if(errors !=3D null) {
            for(= String error : errors) {
            = ;    addCanDoActionMessage(error);
            }
        }
    }
...
}

Regards,
Shireesh
=
_______________________________________________
Engine-devel mailing= list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinf= o/engine-devel
i don't think there is a reason to restrict = only for VdcActionParametersBase, roy what do you think?

also you ca= n use here
getReturnValue().getCanDoActionMessages().addAll(errors);
= instead of going over on all errors.
--=3D_e21defe9-d4a0-4579-ac6e-9f383c6664d2-- --===============6840676888591997515== Content-Type: multipart/alternative MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="attachment.bin" LS09X2UyMWRlZmU5LWQ0YTAtNDU3OS1hYzZlLTlmMzgzYzY2NjRkMgpDb250ZW50LVR5cGU6IHRl eHQvcGxhaW47IGNoYXJzZXQ9dXRmLTgKQ29udGVudC1UcmFuc2Zlci1FbmNvZGluZzogN2JpdAoK LS0tLS0gT3JpZ2luYWwgTWVzc2FnZSAtLS0tLQoKPiBGcm9tOiAiU2hpcmVlc2ggQW5qYWwiIDxz YW5qYWxAcmVkaGF0LmNvbT4KPiBUbzogZW5naW5lLWRldmVsQG92aXJ0Lm9yZwo+IFNlbnQ6IFRo dXJzZGF5LCBBcHJpbCAxMiwgMjAxMiA5OjM1OjI1IEFNCj4gU3ViamVjdDogW0VuZ2luZS1kZXZl bF0gUXVlcnkgcmVnYXJkaW5nCj4gVmFsaWRhdGlvblV0aWxzI3ZhbGlkYXRlSW5wdXRzCgo+IEhp LAoKPiBUaGlzIGlzIHJlZ2FyZGluZyB0aGUgZm9sbG93aW5nIHZhbGlkYXRpb24gbWV0aG9kIHdl IGhhdmUgaW4KPiBWYWxpZGF0aW9uVXRpbHM6Cgo+IHB1YmxpYyBzdGF0aWMgPFQgZXh0ZW5kcyBW ZGNBY3Rpb25QYXJhbWV0ZXJzQmFzZT4gQXJyYXlMaXN0PFN0cmluZz4KPiB2YWxpZGF0ZUlucHV0 cyhMaXN0PENsYXNzPD8+PiB2YWxpZGF0aW9uR3JvdXBMaXN0LCBUIHBhcmFtZXRlcnMpOwoKPiBJ IHRoZXJlIGFueSBwYXJ0aWN1bGFyIHJlYXNvbiBmb3Igc3VwcG9ydGluZyB0aGUgdmFsaWRhdGlv bnMgb25seSBvbgo+IG9iamVjdHMgb2YgY2xhc3NlcyBkZXJpdmVkIGZyb20gVmRjQWN0aW9uUGFy YW1ldGVyc0Jhc2U/IEkgZ3Vlc3MKPiB0aGlzIHdhcyBkb25lIGJlY2F1c2UgdGhpcyBtZXRob2Qg aXMgcHJpbWFyaWx5IGludGVuZGVkIHRvIHZhbGlkYXRlCj4gdGhlIGFjdGlvbiBwYXJhbWV0ZXJz IHBhc3NlZCB0byBhIEJMTCBhY3Rpb24sIHVzaW5nIHRoZSB2YWxpZGF0aW9uCj4gYW5ub3RhdGlv bnMgb24gdGhlIHBhcmFtZXRlciBjbGFzcy4gSG93ZXZlciBJIHRoaW5rIHRoaXMgbWV0aG9kIGNh bgo+IGJlIHVzZWZ1bCBmb3IgZ2VuZXJhbCB1c2UgYXMgd2VsbC4gZS5nLiBJIGNhbm5vdCBhZGQg YSAiQFZhbGlkIgo+IGFubm90YXRpb24gb24gYSAibGlzdCIgb3IgYSAibWFwIiBpbiBhIHBhcmFt ZXRlciBjbGFzcy4gU28gSSBuZWVkIHRvCj4gaXRlcmF0ZSBvdmVyIHRoZSBsaXN0L21hcCwgYW5k IHZhbGlkYXRlIGVhY2ggZWxlbWVudCBpbnNpZGUgdGhlCj4gbG9vcC4gVGhlIHZhbGlkYXRpb24g aW5zaWRlIHRoZSBsb29wIGNhbiBhbHNvIHV0aWxpemUgdGhlIGFib3ZlCj4gbWV0aG9kIGlmIHRo ZSByZXN0cmljdGlvbiAiZXh0ZW5kcyBWZGNBY3Rpb25QYXJhbWV0ZXJzQmFzZSIgaXMKPiByZW1v dmVkLiBUaGlzIHdpbGwgYWxsb3cgbWUgdG8gZG8gdGhlIGZvbGxvd2luZyBpbiB0aGUgY2FuRG9B Y3Rpb24KPiBtZXRob2Q6Cgo+IHByb3RlY3RlZCBib29sZWFuIGNhbkRvQWN0aW9uKCkgewo+IC4u Lgo+IGZvcihHbHVzdGVyQnJpY2tFbnRpdHkgYnJpY2sgOgo+IGdldFBhcmFtZXRlcnMoKS5nZXRH bHVzdGVyVm9sdW1lKCkuZ2V0QnJpY2tzKCkpIHsKPiBMaXN0PFN0cmluZz4gZXJyb3JzID0KPiBW YWxpZGF0aW9uVXRpbHMudmFsaWRhdGVJbnB1dHMoZ2V0VmFsaWRhdGlvbkdyb3VwcygpLCBicmlj ayk7Cj4gaWYoZXJyb3JzICE9IG51bGwpIHsKPiBmb3IoU3RyaW5nIGVycm9yIDogZXJyb3JzKSB7 Cj4gYWRkQ2FuRG9BY3Rpb25NZXNzYWdlKGVycm9yKTsKPiB9Cj4gfQo+IH0KPiAuLi4KPiB9Cgo+ IFJlZ2FyZHMsCj4gU2hpcmVlc2gKCj4gX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX18KPiBFbmdpbmUtZGV2ZWwgbWFpbGluZyBsaXN0Cj4gRW5naW5lLWRldmVs QG92aXJ0Lm9yZwo+IGh0dHA6Ly9saXN0cy5vdmlydC5vcmcvbWFpbG1hbi9saXN0aW5mby9lbmdp bmUtZGV2ZWwKCmkgZG9uJ3QgdGhpbmsgdGhlcmUgaXMgYSByZWFzb24gdG8gcmVzdHJpY3Qgb25s eSBmb3IgVmRjQWN0aW9uUGFyYW1ldGVyc0Jhc2UsIHJveSB3aGF0IGRvIHlvdSB0aGluaz8gCgph bHNvIHlvdSBjYW4gdXNlIGhlcmUgCmdldFJldHVyblZhbHVlKCkuZ2V0Q2FuRG9BY3Rpb25NZXNz YWdlcygpLmFkZEFsbChlcnJvcnMpOyAKaW5zdGVhZCBvZiBnb2luZyBvdmVyIG9uIGFsbCBlcnJv cnMuIAoKLS09X2UyMWRlZmU5LWQ0YTAtNDU3OS1hYzZlLTlmMzgzYzY2NjRkMgpDb250ZW50LVR5 cGU6IHRleHQvaHRtbDsgY2hhcnNldD11dGYtOApDb250ZW50LVRyYW5zZmVyLUVuY29kaW5nOiA3 Yml0Cgo8aHRtbD48aGVhZD48c3R5bGUgdHlwZT0ndGV4dC9jc3MnPnAgeyBtYXJnaW46IDA7IH08 L3N0eWxlPjwvaGVhZD48Ym9keT48ZGl2IHN0eWxlPSdmb250LWZhbWlseTogVGltZXMgTmV3IFJv bWFuOyBmb250LXNpemU6IDEycHQ7IGNvbG9yOiAjMDAwMDAwJz48YnI+PGJyPjxociBpZD0iendj aHIiPjxibG9ja3F1b3RlIHN0eWxlPSJib3JkZXItbGVmdDoycHggc29saWQgcmdiKDE2LCAxNiwg MjU1KTttYXJnaW4tbGVmdDo1cHg7cGFkZGluZy1sZWZ0OjVweDtjb2xvcjojMDAwO2ZvbnQtd2Vp Z2h0Om5vcm1hbDtmb250LXN0eWxlOm5vcm1hbDt0ZXh0LWRlY29yYXRpb246bm9uZTtmb250LWZh bWlseTpIZWx2ZXRpY2EsQXJpYWwsc2Fucy1zZXJpZjtmb250LXNpemU6MTJwdDsiPjxiPkZyb206 IDwvYj4iU2hpcmVlc2ggQW5qYWwiICZsdDtzYW5qYWxAcmVkaGF0LmNvbSZndDs8YnI+PGI+VG86 IDwvYj5lbmdpbmUtZGV2ZWxAb3ZpcnQub3JnPGJyPjxiPlNlbnQ6IDwvYj5UaHVyc2RheSwgQXBy aWwgMTIsIDIwMTIgOTozNToyNSBBTTxicj48Yj5TdWJqZWN0OiA8L2I+W0VuZ2luZS1kZXZlbF0g UXVlcnkgcmVnYXJkaW5nIFZhbGlkYXRpb25VdGlscyN2YWxpZGF0ZUlucHV0czxicj48YnI+CiAg CgogICAgCiAgCiAgCiAgICBIaSw8YnI+CiAgICA8YnI+CiAgICBUaGlzIGlzIHJlZ2FyZGluZyB0 aGUgZm9sbG93aW5nIHZhbGlkYXRpb24gbWV0aG9kIHdlIGhhdmUgaW4KICAgIFZhbGlkYXRpb25V dGlsczo8YnI+CiAgICA8YnI+CiAgICA8aT5wdWJsaWMgc3RhdGljICZsdDtUIGV4dGVuZHMgVmRj QWN0aW9uUGFyYW1ldGVyc0Jhc2UmZ3Q7CiAgICAgIEFycmF5TGlzdCZsdDtTdHJpbmcmZ3Q7IHZh bGlkYXRlSW5wdXRzKExpc3QmbHQ7Q2xhc3MmbHQ7PyZndDsmZ3Q7CiAgICAgIHZhbGlkYXRpb25H cm91cExpc3QsIFQgcGFyYW1ldGVycyk7PC9pPjxicj4KICAgIDxicj4KICAgIEkgdGhlcmUgYW55 IHBhcnRpY3VsYXIgcmVhc29uIGZvciBzdXBwb3J0aW5nIHRoZSB2YWxpZGF0aW9ucyBvbmx5IG9u CiAgICBvYmplY3RzIG9mIGNsYXNzZXMgZGVyaXZlZCBmcm9tIFZkY0FjdGlvblBhcmFtZXRlcnNC YXNlPyBJIGd1ZXNzCiAgICB0aGlzIHdhcyBkb25lIGJlY2F1c2UgdGhpcyBtZXRob2QgaXMgcHJp bWFyaWx5IGludGVuZGVkIHRvIHZhbGlkYXRlCiAgICB0aGUgYWN0aW9uIHBhcmFtZXRlcnMgcGFz c2VkIHRvIGEgQkxMIGFjdGlvbiwgdXNpbmcgdGhlIHZhbGlkYXRpb24KICAgIGFubm90YXRpb25z IG9uIHRoZSBwYXJhbWV0ZXIgY2xhc3MuIEhvd2V2ZXIgSSB0aGluayB0aGlzIG1ldGhvZCBjYW4K ICAgIGJlIHVzZWZ1bCBmb3IgZ2VuZXJhbCB1c2UgYXMgd2VsbC4gZS5nLiBJIGNhbm5vdCBhZGQg YSAiQFZhbGlkIgogICAgYW5ub3RhdGlvbiBvbiBhICJsaXN0IiBvciBhICJtYXAiIGluIGEgcGFy YW1ldGVyIGNsYXNzLiBTbyBJIG5lZWQgdG8KICAgIGl0ZXJhdGUgb3ZlciB0aGUgbGlzdC9tYXAs IGFuZCB2YWxpZGF0ZSBlYWNoIGVsZW1lbnQgaW5zaWRlIHRoZQogICAgbG9vcC4gVGhlIHZhbGlk YXRpb24gaW5zaWRlIHRoZSBsb29wIGNhbiBhbHNvIHV0aWxpemUgdGhlIGFib3ZlCiAgICBtZXRo b2QgaWYgdGhlIHJlc3RyaWN0aW9uICJleHRlbmRzIFZkY0FjdGlvblBhcmFtZXRlcnNCYXNlIiBp cwogICAgcmVtb3ZlZC4gVGhpcyB3aWxsIGFsbG93IG1lIHRvIGRvIHRoZSBmb2xsb3dpbmcgaW4g dGhlIGNhbkRvQWN0aW9uCiAgICBtZXRob2Q6PGJyPgogICAgPGJyPgogICAgcHJvdGVjdGVkIGJv b2xlYW4gY2FuRG9BY3Rpb24oKSB7PGJyPgogICAgLi4uPGJyPgogICAgJm5ic3A7Jm5ic3A7Jm5i c3A7IGZvcihHbHVzdGVyQnJpY2tFbnRpdHkgYnJpY2sgOgogICAgZ2V0UGFyYW1ldGVycygpLmdl dEdsdXN0ZXJWb2x1bWUoKS5nZXRCcmlja3MoKSkgezxicj4KICAgICZuYnNwOyZuYnNwOyZuYnNw OyAmbmJzcDsmbmJzcDsmbmJzcDsgTGlzdCZsdDtTdHJpbmcmZ3Q7IGVycm9ycyA9CiAgICBWYWxp ZGF0aW9uVXRpbHMudmFsaWRhdGVJbnB1dHMoZ2V0VmFsaWRhdGlvbkdyb3VwcygpLCBicmljayk7 PGJyPgogICAgJm5ic3A7Jm5ic3A7Jm5ic3A7ICZuYnNwOyZuYnNwOyZuYnNwOyBpZihlcnJvcnMg IT0gbnVsbCkgezxicj4KICAgICZuYnNwOyZuYnNwOyZuYnNwOyZuYnNwOyZuYnNwOyZuYnNwOyZu YnNwOyZuYnNwOyZuYnNwOyZuYnNwOyZuYnNwOyBmb3IoU3RyaW5nIGVycm9yIDogZXJyb3JzKSB7 PGJyPgogICAgJm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7 Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7IGFkZENhbkRvQWN0aW9u TWVzc2FnZShlcnJvcik7PGJyPgogICAgJm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5i c3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7IH08YnI+CiAgICAmbmJzcDsmbmJzcDsm bmJzcDsmbmJzcDsmbmJzcDsmbmJzcDsmbmJzcDsgfTxicj4KICAgICZuYnNwOyZuYnNwOyZuYnNw OyB9PGJyPgogICAgLi4uPGJyPgogICAgfTxicj4KICAgIDxicj4KICAgIFJlZ2FyZHMsPGJyPgog ICAgU2hpcmVlc2g8YnI+CiAgCgo8YnI+X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX188YnI+RW5naW5lLWRldmVsIG1haWxpbmcgbGlzdDxicj5FbmdpbmUtZGV2 ZWxAb3ZpcnQub3JnPGJyPmh0dHA6Ly9saXN0cy5vdmlydC5vcmcvbWFpbG1hbi9saXN0aW5mby9l bmdpbmUtZGV2ZWw8YnI+PC9ibG9ja3F1b3RlPmkgZG9uJ3QgdGhpbmsgdGhlcmUgaXMgYSByZWFz b24gdG8gcmVzdHJpY3Qgb25seSBmb3IgVmRjQWN0aW9uUGFyYW1ldGVyc0Jhc2UsIHJveSB3aGF0 IGRvIHlvdSB0aGluaz88YnI+PGJyPmFsc28geW91IGNhbiB1c2UgaGVyZTxicj5nZXRSZXR1cm5W YWx1ZSgpLmdldENhbkRvQWN0aW9uTWVzc2FnZXMoKS5hZGRBbGwoZXJyb3JzKTs8YnI+aW5zdGVh ZCBvZiBnb2luZyBvdmVyIG9uIGFsbCBlcnJvcnMuPGJyPjwvZGl2PjwvYm9keT48L2h0bWw+Ci0t PV9lMjFkZWZlOS1kNGEwLTQ1NzktYWM2ZS05ZjM4M2M2NjY0ZDItLQo= --===============6840676888591997515==--