Add support for iterators

[connormullett]

Adds the ability to iterate over nodes in the ring. This would help users attempting to copy the internal ring for serialization purposes and help with needs for replication where a user would want to access a node that neighbors the node responsible for the key

Additionally, this PR fixes and adds tests related to these changes

[jeromefroe]

Thanks for the contribution @connormullett! LGTM overall, just had one question on how we want to implement the iterator so as to potentially avoid needing to clone the internal vector.

[jeromefroe]

Do you think it would be worth using a std::vec::IntoIter here instead of a Vec so we don't have to clone the underlying Vec?

[connormullett]

That's a good point and a good solution to what I couldn't really figure out. Change is coming shortly

[connormullett]

I'm trying to find a clean implementation that doesn't clone the underlying Vec but also doesn't expose the private Node type. using Map gives a reference which unfortunately means cloning is required. Thoughts?

[jeromefroe]

LGTM overall, just had some minor comments about some of the internal details that I would be curious to get your thoughts on

[jeromefroe]

Can we get rid of index now that we're wrapping the underlying vector iterator?

[connormullett]

good call

[jeromefroe]

To avoid needing to collect the items from the vector into an iterator here, what do you think about using the internal iterator from the vector directly and mapping Node<T> to T in the hash ring iterator's next method? For example:

pub struct HashRingIterator<T> {
    iter: std::vec::IntoIter<Node<T>>,
}

impl<T> Iterator for HashRingIterator<T> {
    type Item = T;

    fn next(&mut self) -> Option<Self::Item> {
        self.iter.next().map(|n| n.node)
    }
}

[connormullett]

I didn't want to do it this way as it would expose the internal Node type and expose a layer of abstraction. This is much cleaner way of doing it. Changes coming soon

[jeromefroe]

Everything looks good to me, had just one minor comment on whether we could keep the Node type private.

[jeromefroe]

Do we need to make this a public type? If we map Node<T> to T in the next method would that allow us to keep the type private? For example:

pub struct HashRingIterator<T> {
    ring: std::vec::IntoIter<Node<T>>,
}

impl<T> Iterator for HashRingIterator<T> {
    type Item = T;

    fn next(&mut self) -> Option<Self::Item> {
        self.ring.next().map(|node| node.node)
    }
}

impl<T: Clone> IntoIterator for HashRing<T> {
    type Item = T;

    type IntoIter = HashRingIterator<T>;

    fn into_iter(self) -> Self::IntoIter {
        HashRingIterator {
            ring: self.ring.into_iter(),
        }
    }
}

[connormullett]

Ah this is much simpler and works. This is what I was trying to figure out but was unable to. Changes coming soon. Thanks for the assist

[connormullett]

Ah I missed the Node addition in HashRingIterator. This works great, commit coming soon

[jeromefroe]

Thanks for the contribution @connormullett!

[connormullett]

No problem, thanks for taking the time to review and help