Vojtech Szocs has posted comments on this change. Change subject: aaa: Fixing search to search by authz ......................................................................
Patch Set 5: (3 comments) http://gerrit.ovirt.org/#/c/28722/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAAAProfileListQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAAAProfileListQuery.java: Line 27: Collections.sort(names, new Comparator<ProfileEntry>() { Line 28: Line 29: @Override Line 30: public int compare(ProfileEntry lhs, ProfileEntry rhs) { Line 31: return lhs.getProfile().compareTo(rhs.getProfile()) == 0 ? > You're right about the comparisson, sorry for that. As for temp variables, it's up to you :-) Line 32: lhs.getProfile().compareTo(rhs.getProfile()) Line 33: : lhs.getAuthz().compareTo(rhs.getAuthz()); Line 34: Line 35: } http://gerrit.ovirt.org/#/c/28722/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/aaa/ProfileEntry.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/aaa/ProfileEntry.java: Line 23: public String getProfile() { Line 24: return profile; Line 25: } Line 26: Line 27: public void setProfile(String profile) { > but it's serializablle , shouldnt it have setters as well? Good question, I didn't think about it when I wrote my comment. According to [1,2] it should be perfectly OK to have a class like this: public class ProfileEntry implements Serializable { // serialVersionUID declaration goes here private final String profile; private final String authz; public ProfileEntry(String profile, String authz) { this.profile = profile; this.authz = authz; } public String getProfile() { return profile; } public String getAuthz() { return authz; } } It's because the first non-serializable superclass (java.lang.Object in this case, which doesn't implement Serializable interface) already has a no-arg constructor. Upon deserialization (which is "bottom-up"), if a class doesn't implement Serializable interface, no-arg constructor is invoked (along with readObject/writeObject method hooks). On the other hand, if the first non-serializable superclass does _not_ have a no-arg constructor, deserialization will fail. For example: // non-serializable, without no-arg constructor public class One { public One(int a) { } } // error on deserialization - class One doesn't have no-arg constructor public class Two extends One implements Serializable { // constructors don't matter as this class implements Serializable } [1] http://stackoverflow.com/questions/13351969/java-serialization-and-javabeans [2] http://www.jguru.com/faq/view.jsp?EID=251942 Line 28: this.profile = profile; Line 29: } Line 30: Line 31: public String getAuthz() { Line 31: public String getAuthz() { Line 32: return authz; Line 33: } Line 34: Line 35: public void setAuthz(String authz) { > see my comment for vojtech - don't i need that for serialization? See my response :-) I think that we do _not_ need to follow JavaBeans convention here (no-arg constructor, getter/setter methods). Line 36: this.authz = authz; Line 37: } Line 38: Line 39: public String toString() { -- To view, visit http://gerrit.ovirt.org/28722 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic1867577a900e2c7a815d443b771e2576bd8aea2 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Ondřej Macháček <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
