Grouper Developers Forum

[Gagné Sébastien <>]

Here is the fixed LdapComparator in the subject API.

 

I tested it using my dev Box and it works, I can now see subjects with the same description.

 

De : [mailto:] De la part de Gagné Sébastien
Envoyé : 24 septembre 2012 14:36
À :
Objet : [grouper-dev] Inconsistencies with subject comparison and equality

 

Hi Devs,

I found the “dropped subject” problem :

 

The method “searchHelper” in the LdapSourceAdapter is using a TreeSet to hold the subjects, which uses the LdapComparator class (package edu.internet2.middleware.subject.provider). This class provides a faulty (or inconsistent) compare to method. If 2 objects are equals according to the comparator, the TreeSet will ignore it.

 

LdapComparator compares the description (which is not unique) :

 

String s0d = s0.getDescription();

String s1d = s1.getDescription();

 

// log.debug("comparing " + s0d + " to " + s1d);

return s0d.compareTo(s1d);

 

While the SubjectImpl ‘s equals method uses the ID and sourceID :

return StringUtils.equals(subject.getId(), otherObj.getId())

&& StringUtils.equals(subject.getSourceId(), otherObj.getSourceId());

 

This means that you can have subjects that are equal in one case and not in the other, e.g. :

gsh 3% s1 = SubjectFinder.findById("marchanr4")

gsh 6% s2 = SubjectFinder.findById("marchanr3")

 

gsh 8% s2.equals(s1)

false

 

gsh 12% comp = new LdapComparator()

gsh 15% comp.compare(s1,s2)

0

 

I think the LdapComparator should be changed to use the Subject’s ID and Source ID just like the equals method. Using the description doesn’t make sense when using the totality of the subjects.

 

Something like :

 

      public int compare(Subject so0, Subject so1) {

 

            try {

 

                  Subject s0 = (Subject) so0;

                  Subject s1 = (Subject) so1;

                 

                  int src = "s0.getSourceId().compareTo(s1.getSourceId());

                  if (src == 0) {

                        return s0. getId().compareTo(s1.getId());

 

                  } else {

                        return src;

                  }

 

            } catch (Exception e) {

                  log.debug("exception " + e);

            }

            return (1);

      }

 

 

 

Another thing to fix :

In SubjectImpl : according to Java, equals should return a NulPointerException if obj == null, but there might be a reason why you didn’t do it

 

 

Thanks for your time

 

 

Sébastien Gagné,     | Analyste en informatique

514-343-6111 x33844  | Université de Montréal,

                     | Pavillon Roger-Gaudry, local X-100-11

 

Attachment: LdapComparator.java
Description: LdapComparator.java