From redis-rb 5.0.0, `sadd` and `srem` always return integers

[mikevoets]

In this gem it still returns true or false. This has changed from redis-rb version 5, see https://github.com/redis/redis-rb/blob/master/CHANGELOG.md.

As a workaround, I started using sadd? and srem? to align the behaviour of this gem and redis-rb.

[CoderMiguel]

I started a fork to work on this last night and realized the extent of this issue is pretty deep. There were a lot of changes in the upgrade to version 5 that will have a lot of impact on this gem. That I think ultimately needs to be a larger conversation than just fixing this bug.

One of the biggest questions would be does this gem want to maintain backwards compatibility with version 4 or not. Some others would include, is fixing this bug alone enough or would all of the methods effected need to be handled before the changes will be committed?

In the meantime, may I offer the following as a workaround for those that are using >= version 5:

class MockRedis
  module SetMethods
    def sadd(key, members)
      # members_class = members.class
      members = Array(members).map(&:to_s)
      assert_has_args(members, 'sadd')

      with_set_at(key) do |s|
        size_before = s.size
        if members.size > 1
          members.reverse_each { |m| s << m }
          s.size - size_before
        else
          added = !!s.add?(members.first)
          # if members_class == Array
            s.size - size_before
          # else
            # added
          # end
        end
      end
    end

    def srem(key, members)
      with_set_at(key) do |s|
        if members.is_a?(Array)
          orig_size = s.size
          members = members.map(&:to_s)
          s.delete_if { |m| members.include?(m) }
          orig_size - s.size
        else
          # !!s.delete?(members.to_s)
          s.delete?(members.to_s) ? 1 : 0
        end
      end
    end
  end
end

Comments about the above suggestion

If the specs are updated to expect the new output. This passes the bulk of the tests in the gem, all failures for the srem and sadd specs are from breaking changes in version 5. All but one of the failures coming from it_should_behave_like 'a set-only command':

    let(:redis_gem_v5?) { Redis::VERSION.to_i == 5 }
    let(:positive_response) { redis_gem_v5? ? 1 : true }
    let(:negative_response) { redis_gem_v5? ? 0 : false }

    it 'returns positive response if member is in the set' do
      expect(@redises.srem(@key, 'bert')).to eq(positive_response)
    end

    it 'returns negative response if member is not in the set' do
      expect(@redises.srem(@key, 'cookiemonster')).to eq(negative_response)
    end

[sds]

Thanks for the research and providing a workaround!

I'm supportive of dropping support for older Redis gem versions as part of fixing in the project itself. At some point the ecosystem needs to move forward.