[
https://issues.apache.org/jira/browse/OAK-3510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14954587#comment-14954587
]
angela edited comment on OAK-3510 at 10/13/15 8:07 AM:
-------------------------------------------------------
updated patch which also includes some tests. while writing those i found it
strange that an instance with an empty provider name is equal to one with
{{null}} as provider name. i therefore decided to change the constructor
converting empty provider name to null. this would also be consistent with the
static {{fromString}} method, which always creates instances with {{null}}
provider name if it is missing in the string representation.
in addition: the private {{escape}} can be made static and i also added
annotations to the params.
was (Author: anchela):
updated patch which also includes some tests. while writing those i found it
strange that an instance with an empty provider name is equal to one with
{{null}} as provider name. i therefore decided to change the constructor
converting empty provider name to null. this would also be consistent with the
static {{fromString}} method, which always creates instances with {{null}}
provider name if it is missing in the string representation.
in addition: the private {{escape}} can be made static and i also added
annotations to the params.
and finall
> Troublesome ExternalIdentityRef.equals(Object) implementation
> -------------------------------------------------------------
>
> Key: OAK-3510
> URL: https://issues.apache.org/jira/browse/OAK-3510
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: auth-external
> Reporter: angela
> Attachments: OAK-3510.patch, OAK-3510_2.patch
>
>
> in the light of OAK-3508 i looked at the {{ExternalIdentifyRef}} class and
> found the following implementation of {{Object.equals(Object)}}:
> {code}
> public boolean equals(Object o) {
> try {
> // assuming that we never compare other types of classes
> return this == o || string.equals(((ExternalIdentityRef)
> o).string);
> } catch (Exception e) {
> return false;
> }
> }
> {code}
> since this class is public and exported as part of a public API, i don't
> think the assumption made in the code is justified. also i would argue that
> catching {{Exception}} is bad style as is exception driven development. in
> this particular case it was IMHO perfectly trivial to just get rid of the
> catch clause altogether.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
