Support expiration time for lists

[nzwsch]

Closes #129

Since Leon did not respond, I created a PR on his behalf.
I tried to find another way to do it, but there is little change from the code he first wrote.
One thing to note is that the array seemed to be empty if I specified the values for expires_in and default at the same time.

  test "append with default expiring list" do
    @list = Kredis.list "mylist", default: ->() { %w[ 1 ] }, expires_in: 1.second
    @list.append(%w[ 2 3 ])

    sleep 0.5.seconds
    assert_equal %w[ 1 2 3 ], @list.elements

    sleep 0.6.seconds
    assert_equal [], @list.elements # <== It should be ["1"] but I don't figure out to set default here..
  end

If this PR is accepted, I will look into other Hash, OrderedSet, etc. to see if they can support expires_in as well.

[leoncruz]

Hi, @nzwsch sorry for disappear. I have working on code that showed on the #129 and found a problem in my application. The expiration time is always set when a request occours. For example: the first request there is no list, so creates the list, then set the expiration time. On the next request, with existing list, the expiration time is set again. This could be a possible behavior, but is not what I want. I don't know if you found this problem too

[leoncruz]

Hey, @nzwsch after some tests I think i found a solution. There is a nx parameter on expire method that indicates to redis to not reset the expiration after already set. This is my code:

# lib/kredis/types/list.rb

def prepend(*elements)
  return if elements.flatten.empty?

  lpush types_to_strings(elements, typed)

  expire expires_in.to_i, nx: !reset_expire_on_update if expires_in
end

The reset_expire_on_update indicates that expiration time will be reset when a new element is prepended. By default is set to false:

# lib/kredis/types.rb
def list(key, default: nil, typed: :string, config: :shared, after_change: nil, expires_in: nil, reset_expire_on_update: false)
  type_from(List, config, key, after_change: after_change, default: default, typed: typed, expires_in: expires_in, reset_expire_on_update: reset_expire_on_update)
end

Would be great if you could test this and add to your PR.

[nzwsch]

@leoncruz

I looked at the code you presented and did a little implementation, is there any reason why we should support nx in expire?
Looking at the existing code I see that it uses nx and ex in the set method of counter.rb.
I don't remember it well as it was a few months ago, but when I first worked on this PR I tried to use the multi block without expire but it didn't pass the test.
So I can't follow your request, but how about simply using only expire in this PR or simply not calling expire instead of using nx?

def prepend(*elements)
  return if elements.flatten.empty?

  lpush types_to_strings(elements, typed)
  expire_in expires_in if expires_in && !reset_expire_on_update # This keyword is a little long, I think
  elements
end

Frankly I am glad you commented on this PR, but I don't think we should expect this PR to be responsive because the maintainers are not.
I would prefer to take the monkey patch approach.

[leoncruz]

@nzwsch The ideia of nx on expire is to not reset the expiration time if the value on list update. For example, I get your test and add a new line where a new item is inserted on list:

test "prepend with expiring list" do
  @list = Kredis.list "mylist", expires_in: 1.second
  @list.prepend(%w[1 2 3])

  sleep 0.5.seconds
  assert_equal %w[ 3 2 1 ], @list.elements

  puts "TTL before insertion: #{Kredis.redis.pttl "mylist"}"
  @list.prepend 4 # new item insertion
  puts "TTL after insertion: #{Kredis.redis.pttl "mylist"}"

  sleep 0.6.seconds
  assert_equal [], @list.elements
end

Then, the test fails because the list is not empty yet. I print the time to live on test to see that increase after insertion:

If use the nx this behavior not happend:

def expire_in(seconds)
  expire seconds.to_i, nx: true
end

The test pass and the ttl not increase:

This is the idea of reset_expire_on_update param. If it's true will be reset the expiration time on each list update, like the first scenarion (without nx). But, if it's false, the expiration time will not be reseted.

[dhh]

Don't think we need this. You can just call expire expires_in if expires_in inline.

[nzwsch]

I think I borrowed this code from scalar.rb, but it certainly would not have been necessary to define it in a method.

[nzwsch]

@leoncruz
Thank you for presenting the code example again.
I finally understand what you say.
My environment was using Redis 7, so I was able to pass the test successfully, but from the results on GitHub, it seems that the test fails if the Redis version is less than 7.

[dhh]

These test failures all seem real.

[leoncruz]

@nzwsch I this errors are because the nx on expire is only available in redis 7.0 :(. Follow the documentation: https://redis.io/commands/expire/. I don't known how to proceed with this. Maybe check the redis version?

[nzwsch]

@leoncruz
First of all, thank you for your comments.
I could consider adopting the suggested code if it were my repository, but since this is a Rails repository, it would be difficult to adopt an implementation that only behaves differently for a specific Redis version.
Unfortunately, at this point it is difficult to pass this test, so I have decided to close this PR.

[heka1024]

Hi, @nzwsch. Could you review #142, which based on your works? I made a small change to support lower versions of redis.