Provide VoidOutput for Fire&Forget command usage

[jaredpetersen]

Bug Report

Current Behavior

The documentation lists several different response types that can be leveraged for the dynamic command feature and VoidOutput is one of them. However, VoidOutput cannot be used because it's package private to io.lettuce.core.dynamic.output. All of the other outputs are public and live in io.lettuce.core.output.

Input Code

Input Code

final CommandOutput commandOutput = new VoidOutput(StringCodec.UTF8);

Expected behavior/code

New instances of VoidOutput may be created.

Environment

• Lettuce version(s): 6.0.1.RELEASE
• Redis version: N/A

Possible Solution

Move VoidOutput to io.lettuce.core.output and declare it as public.

Additional context

None

[jaredpetersen]

It's possible that StatusOutput should be used instead of VoidOutput per Fire & Forget. If that's the case, then VoidOutput should not be included in the documentation since it's only available internally.

[mp911de]

VoidOutput was never intended to be used directly since the implementation is an implementation detail so that command methods can simply work with void.

Care to elaborate where you'd want to use VoidOutput in your code?

[jaredpetersen]

I don't have any specific examples. Basically I'm just writing some code that needs to throw various arbitrary module commands at Redis to load data and then get a confirmation that the command is done. I don't care about the output at all -- I just need to know that the command finished. I saw VoidOutput in the documentation first and locked in on that.

It sounds like StatusOutput is the way to go for this instead:

Fire&Forget is the simple-most way to dispatch commands. You just trigger it and then you do not care what happens with it, whether it completes or not, and you don’t have access to the command output

[mp911de]

That works for RESP2 and only when using byte outputs. Numeric values will fail with IllegalStateException. From the perspective of using Fire and Forget programmatically, it would make sense to have a void codec publicly available instead of referring to StatusOutput that works only for certain commands.

[jaredpetersen]

@mp911de Thanks for the help on this and for adding it to the next milestone!

Would the new class for this purpose be pretty much identical to the other VoidOutput class?

package io.lettuce.core.output;
// . . .

public class VoidOutput<K, V> extends CommandOutput<K, V, Void> {
    public VoidOutput(RedisCodec<K, V> codec) {
        super(codec, null);
    }

    @Override
    public void set(ByteBuffer bytes) {
        // no-op
    }

    @Override
    public void set(long integer) {
        // no-op
    }

    @Override
    public void set(double number) {
        // no-op
    }

    @Override
    public void set(boolean value) {
        // no-op
    }
}

I'm not familiar with these output classes (as I'm sure you're well aware) and I had no idea what Redis Serialization Protocol was before you mentioned it so please forgive my ignorance 😄

[mp911de]

All good, no worries. So basically, we would move VoidCodec into the output package and add overrides for all set methods to ignore their output. Given that VoidOutput is essentially a black hole, we can provide a static instance (and a factory like <K, V> VoidOutput<K, V> instance() as there's no need for a codec because it's not returning anything.

[jaredpetersen]

A couple of clarification points:

So basically, we would move VoidCodec into the output package

VoidCodec does not exist -- did you mean to move VoidOutput to io.lettuce.core.output? I'm assuming this was a simple flub 🙂

add overrides for all set methods to ignore their output

Yup, makes total sense and lines up with my previous sample code suggestion

Given that VoidOutput is essentially a black hole, we can provide a static instance (and a factory like <K, V> VoidOutput<K, V> instance() as there's no need for a codec because it's not returning anything

The problem is that VoidOutput extends abstract class RedisCommandOutput and the super constructor demands a non-null codec. Additionally, other classes reference that particular constructor and pass in an arbitrary codec. We can fix the LettuceAssertion easily but refactoring the dependent classes is going to be tricky.

An alternative solution might entail creating aVoidCodec class that doesn't do anything and have users pass that in for the codec. Or, create another constructor that does not require a codec and instantiate VoidCodec inside of it.

. . .

Let me know your thoughts on this. For what it's worth, just using the sample code I posted with the String codec appears to be working fine for my use case. But it would be better to avoid passing in a useless codec.

[mp911de]

You're right, VoidCodec was a typo. It should be VoidOutput. However, we should add a VoidCodec as inner class so we don't use a particular other codec to satisfy the non-null requirement of passing a valid codec like you already mentioned.

I would also advise to not expose a constructor (that is making the constructor of VoidOutput private) and provide a static factory method that makes use of an unsafe cast into VoidOutput<K, V>. We can ignore generics here since we don't emit any value at all.

[jaredpetersen]

The problem is that other classes still rely on that constructor so you can't make the constructor private without further consequences. That's mainly the OutputRegistry class which wants all *Output classes to fit neatly into CommandOutputFactory. I'm not sure how you'd get VoidOutput to work with that map variable without that particular constructor.

I think I'm out of my depth here because of my unfamiliarity with the system. You definitely have a good idea in your head about how to go about this so I'll defer to you for implementation. Wish I understood more though so that I could alleviate that burden 😞

[mp911de]

Thanks for the discussion, it was helpful to understand the use-case for VoidOutput. Happy to take this change from here.

[mp911de]

Done.