Enable deep equality in OrderedSet

[PhilippGackstatter]

Description of change

The current implementation of CoreDocument, which uses an OrderedSet<DIDKey<T>> to represent verification methods, does not work as intended when rotating the key material of a given verification method. In this case, the diff between the old and the updated document is empty, and old_doc == updated_doc returns true. This is because DIDKey overwrites the equality implementation to compare the contents of the set by key only (i.e. the DID url).

To fix this, the DIDKey wrapper is removed, such that normal equality checks work as expected and intended. To disallow adding multiple methods with the same fragment but, for example, different key material -- which would be allowed with regular equality checks -- a KeyComparable trait is introduced which is used in the contains check. This trait requires elements to return a Key by which they are compared. A default implementation is provided such that types that implement AsRef<CoreDIDUrl> will use that url as the key. This approach should be flexible enough to allow other keys in the future.

Note to reviewers: I would appreciate a careful check of the changed trait bounds on the OrderedSet methods, to make sure they make sense.

Type of change

Add an x to the boxes that are relevant to your changes.

• Bug fix (a non-breaking change which fixes an issue)
• Enhancement (a non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation Fix

How the change has been tested

Existing tests all pass, and new tests were added to check the introductory example works as intended.

Change checklist

Add an x to the boxes that are relevant to your changes.

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[cycraig]

I really like the simplification and removal of DIDKey!

There is a problem with the OrderedSet::update and OrderedSet::replace methods since they still use the old equality comparsions.

Suggested changes:

  /// Replaces a `current` value with the given `update` value; returns `true`
  /// if the value was successfully replaced.
  #[inline]
  pub fn replace<U>(&mut self, current: &U, update: T) -> bool
  where
    T: KeyComparable,
    U: KeyComparable<Key = T::Key>,
  {
    self.change(update, |item, update| item.as_key() == current.as_key() || item.as_key() == update.as_key())
  }

  /// Updates an existing value in the `OrderedSet`; returns `true` if the value
  /// was successfully updated.
  #[inline]
  pub fn update(&mut self, update: T) -> bool
  where
    T: KeyComparable,
  {
    self.change(update, |item, update| item.as_key() == update.as_key())
  }

Here are the relevant unit tests which fail with the current PR:

  #[derive(Clone, Copy, PartialEq, Eq)]
  struct ComparableStruct {
    key: u8,
    value: i32,
  }

  impl KeyComparable for ComparableStruct {
    type Key = u8;

    fn as_key(&self) -> &Self::Key {
      &self.key
    }
  }

  #[test]
  fn test_ordered_set_replace() {
    let mut set = OrderedSet::new();

    // Create two structs with the same key.
    let cs1 = ComparableStruct {
      key: 0,
      value: 10,
    };
    let cs2 = ComparableStruct {
      key: 0,
      value: 20,
    };

    // Try replace it with the second.
    // This should succeed because the keys are equivalent.
    assert!(set.append(cs1));
    assert_eq!(set.len(), 1);

    assert!(set.replace(&cs1, cs2));
    assert_eq!(set.len(), 1);
    assert_eq!(set.head().unwrap().key, cs2.key);
    assert_eq!(set.head().unwrap().value, cs2.value);
  }

  #[test]
  fn test_ordered_set_replace_all() {
    let mut set = OrderedSet::new();
    let cs1 = ComparableStruct {
      key: 0,
      value: 10,
    };
    let cs2 = ComparableStruct {
      key: 1,
      value: 20,
    };
    assert!(set.append(cs1));
    assert!(set.append(cs2));
    assert_eq!(set.len(), 2);

    // Now replace cs1 with something that has the same key as cs2.
    // This should replace BOTH cs1 AND cs2.
    let cs3 = ComparableStruct {
      key: 1,
      value: 30,
    };
    assert!(set.replace(&cs1, cs3));
    assert_eq!(set.len(), 1);
    assert_eq!(set.head().unwrap().key, cs3.key);
    assert_eq!(set.head().unwrap().value, cs3.value);
  }

  #[test]
  fn test_ordered_set_update() {
    let mut set = OrderedSet::new();
    let cs1 = ComparableStruct {
      key: 0,
      value: 10,
    };
    assert!(set.append(cs1));
    assert_eq!(set.len(), 1);

    // This should update the value of cs1 since the keys are the same.
    let cs2 = ComparableStruct {
      key: 0,
      value: 20,
    };
    assert!(set.update(cs2));
    assert_eq!(set.len(), 1);
    assert_eq!(set.head().unwrap().key, cs2.key);
    assert_eq!(set.head().unwrap().value, cs2.value);

    // This should NOT update anything since the key does not match.
    let cs3 = ComparableStruct {
      key: 1,
      value: 20,
    };
    assert!(!set.update(cs3));
    assert_eq!(set.len(), 1);
    assert_eq!(set.head().unwrap().key, cs2.key);
    assert_eq!(set.head().unwrap().value, cs2.value);
  }

[cycraig]

Looks good to me.