Add Redis Streams commands

[Terkwood]

Description

This change set adds high-level wrappers for the Redis Streams commands (XREAD, XADD, etc). This is effectively a merge of https://github.com/grippy/redis-streams-rs into redis-rs.

Adds a feature streams which is turned on by default (following the example of geo). Adds tests for the new commands. Adds an example of usage.

Resolves #162.

These changes deliver a significant feature set which was requested in August 2018.

Task List

• address some outstanding lints
• add examples of how to manipulate the reply types
• fix doc links Add Redis Streams commands #319 (comment) etc

Attribution

This body of work was produced by @grippy and the contributors in https://github.com/grippy/redis-streams-rs. Thanks for making this available to the community!

[grippy]

@Terkwood you rule! Thanks for porting this over. I'm taking a look now and will add comments if I see anything.

[grippy]

This looks good.

One small issue I have is there's a slight disconnect as to what each of these commands now returns if you're looking directly at the code for each new command. It might be worthwhile to add a few code examples on how to work with some of the commands that have custom reply types?

[Terkwood]

Adding an executable streams.rs example file sounds like a great idea, thanks!

[Terkwood]

Is there any significant problem left with this change set which should prevent the merge?

[Marwes]

LGTM otherwise

[Terkwood]

Awesome -- thanks for the feedback! I'll work through these and hopefully have them finished by EOD. I've got a bunch of health-related junk coming up so this is my last chance to address issues before being forced to take a break!

[Terkwood]

OK, it looks like everything has been addressed, CI is passing, etc.

@grippy: There's some outstanding discussion on the usage of XCLAIM in the comment threads. Hopefully we can keep that as part of the PR, but if you have time, you might want to weigh in there, as it's the only place where we don't have 100% written consensus.

[Terkwood]

But I don't think we necessarily need additional input to move forward, if the response above satisfies you (Marwes). Presumably grippy favors the use of just_ids since it's his own creation.

It'll probably come as no surprise that I'm thus voting for an immediate merge 😉 This PR will unlock quite a bit of potential for other users to leverage the streams API from the comfort of their rust environment 🦀 🏞️

[grippy]

@Terkwood Sorry for the delay... It's been a while since I was in this code. As long as it matches the XCLAIM Redis command, I would vote for keeping JUSTID's as is.

[Terkwood]

Thanks! 🌟

[Terkwood]

OK, the additional just_ids method is removed.

From the redis article:

JUSTID: Return just an array of IDs of messages successfully claimed, without returning the actual message. Using this option means the retry counter is not incremented.

So if you want the IDs only, and you want that specific behavior, you can get that. You can just pass the just_id option as true when you call our API.

pub struct StreamClaimOptions {
    /// Set IDLE <milliseconds> cmd arg.
    idle: Option<usize>,
    /// Set TIME <mstime> cmd arg.
    time: Option<usize>,
    /// Set RETRYCOUNT <count> cmd arg.
    retry: Option<usize>,
    /// Set FORCE cmd arg.
    force: bool,
    /// Set JUSTID cmd arg. Be advised: the response
    /// type changes with this option.
    justid: bool,
}

@Marwes was right to point out that the small helper method isn't strictly necessary, since the server can give us what we want. Also, I don't think the current state of the PR contradicts @grippy's previous comment -- as of the current commit, we expose all XCLAIM options.

Thanks again for being patient here, a lot of the comments I posted in the previous section were confused! 😁 But it's worth it, because we're within striking distance of having a big, new chunk of functionality exposed. Very much appreciate the input! 🌈

[Terkwood]

I want to push at least one more test case up before we merge, so please sit tight...

[Terkwood]

OK. It looks like just_ids is covered under test after all:

// claim and only return JUSTID
    let claimed: Vec<String> = con
        .xclaim_options(
            "k1",
            "g1",
            "c5",
            4,
            &claim_justids,
            StreamClaimOptions::default().with_force().with_justid(),
        )
        .unwrap();
    // we just claimed the original 10 ids
    // and only returned the ids
    assert_eq!(claimed.len(), 10);

So I will leave this PR in the hands of the maintainers, thanks! The review was very helpful.

Let us know if there are additional concerns that can be addressed! 🌤️

[Marwes]

I actually just pointed this out because users can do self.ids.iter().map(|msg| &msg.id) themselves, which is often more effecient since the Vec allocation can be avoided.

But hey, whatever works :)

[Terkwood]

Woo!!!! 🎉 🕺🏿 🥳