[OpenAFS-devel] Q's Java API (JAFS) in the 1.4.x build branch

Jim Doyle rockymtnmagic@yahoo.com
Fri, 11 Jul 2008 14:47:11 -0700 (PDT)


This is to address Marcus' questions:

Regarding hashCode()/equals(), lets look at a Case Study:

=========================================================================
public class Volume ... {
  ....
  public boolean equals( Volume otherVolume )
  {
    return ( name.equals(otherVolume.getName()) ) &&
           ( this.getPartition().equals(otherVolume.getPartition()) );
  }
} 
========================================================================

issues:

1.  The correct signature is public boolean equals(Object ref),  and not
the type narrowed signature currently implemented.   

2.  equals(...)  should never ever throw an Exception. You can get this
code to check a NullPtrExc by simply handing it a null volume reference.
You might also be able to get this throw an NPE if Volume::partition
or any of its dependent attributes are null!

3.  If you override equals(...), you must always override hashCode().
equals and hashCode need to operate on the same internal attributes 
that determine object identity. Furthermore, the following assertion
must be honored:
          X.equals(y)   ===> (implies)     X.hashCode() == Y.hashCode()

Obviously, the contrapositive is not true about hashCodes, in otherwords,
    X.hashCode()==Y.hashCode() DOES NOT NECESSARILY MEAN X.equals(Y)

4.  The design choice of using the volume name on a partition as Identity is questionable.  Names can and do change. The Volume id (uint32) is a better choice for identity. We know that the Volserver keeps these unique, and even though names may change, IDs never change even across deletions.

5.  The designs correctness is ALSO affected by other choices in how we
identify partitions. :)   For instance, the volume ID is unique to the Cell, so, if the Partition does not tie back to Server and Cell and cell
is not considered implicitly in the identity test, then there is a trickle down effect where Volume identity is incorrect!

If the object model is done "right" - then these issues are eliminate.
So, half of doing it "right" includes realizing that cell, server, partition, volume are NESTED AGGREGRATIONS of classes.  When we assert identity in Aggregrations, we always know that the parent, containing class has to exist first - which eliminates the partition/server/cell being Null issue.  The other half of doing it right is to insure the object model
works not only for finding existing volumes, creating new volumes, *but*
insuring that Volume uniquess (and thus identity) is preserved across:
 
     -- vos move events
     -- volumes that exist as an impression in the VLDB but are not online
     -- volumes that exist on a live volserver but have no impression in    
        VLDB.


That said, this is an example of a partial fix to Volume:
=========================================================================
public class Volume ... {
  ....
  public boolean equals( Object ref )
  {
    if (ref == null || ref instanceof Volume == false) return false;
    Volume otherVolume = (Volume) ref;
    return name.equals(otherVolume.name) &&
            this.partition.equals(otherVolume.partition);
  }

  public int hashCode() {
       return name.hashCode() ^ partition.hashCode();
  }
} 


========================================================================
-- Jim