Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PLATI-1363] Use Identity as the default compressor & serializer #5

Open
wants to merge 2 commits into
base: braze-main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/dalli.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# frozen_string_literal: true
require 'dalli/compressor'
require 'dalli/identity'
require 'dalli/client'
require 'dalli/ring'
require 'dalli/server'
Expand Down
4 changes: 2 additions & 2 deletions lib/dalli/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ class Client
# - :threadsafe - ensure that only one thread is actively using a socket at a time. Default: true.
# - :expires_in - default TTL in seconds if you do not pass TTL as a parameter to an individual operation, defaults to 0 or forever
# - :compress - defaults to false, if true Dalli will compress values larger than 1024 bytes before sending them to memcached.
# - :serializer - defaults to Marshal
# - :compressor - defaults to zlib
# - :serializer - defaults to Identity
# - :compressor - defaults to Identity
# - :cache_nils - defaults to false, if true Dalli will not treat cached nil values as 'not found' for #fetch operations.
# - :digest_class - defaults to Digest::MD5, allows you to pass in an object that responds to the hexdigest method, useful for injecting a FIPS compliant hash object.
#
Expand Down
25 changes: 25 additions & 0 deletions lib/dalli/identity.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# frozen_string_literal: true

module Dalli
class Identity
# A no-op serializer/compressor that always returns its input

class << self
def dump(value)
return value
end

def load(value)
return value
end

def compress(value)
return value
end

def decompress(value)
return value
end
end
end
end
4 changes: 2 additions & 2 deletions lib/dalli/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ class Server
:value_max_bytes => 1024 * 1024,
# surpassing value_max_bytes either warns (false) or throws (true)
:error_when_over_max_size => false,
:compressor => Compressor,
:compressor => (defined?($TESTING) && $TESTING) ? Compressor : Dalli::Identity,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dalli's pattern seems to be omitting the shared module prefix.

Suggested change
:compressor => (defined?($TESTING) && $TESTING) ? Compressor : Dalli::Identity,
:compressor => (defined?($TESTING) && $TESTING) ? Compressor : Identity,

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should arguably update the tests to pass Compressor instead of use the default, if they fail otherwise. I don't feel particularly strongly about that though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that initially but it required a good amount of changes. I think we should probably keep the number of changes to a minimum to avoid diverging from upstream too much. Also I think doing it this way will allow us to bring in changes from upstream more easily in the future.

# min byte size to attempt compression
:compression_min_size => 1024,
# max byte size for compression
:compression_max_size => false,
:serializer => Marshal,
:serializer => (defined?($TESTING) && $TESTING) ? Marshal : Dalli::Identity,
felixmo marked this conversation as resolved.
Show resolved Hide resolved
:username => nil,
:password => nil,
:keepalive => true,
Expand Down
26 changes: 26 additions & 0 deletions test/test_identity.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# frozen_string_literal: true
require_relative 'helper'

describe 'Identity' do
subject { Dalli::Identity }

let(:some_value) { SecureRandom.alphanumeric(10) }

it "implements the serializer interface (dump/load) with the identity function" do
assert_equal(some_value, subject.dump(some_value))
assert_equal(some_value, subject.load(some_value))
end

it "implements the compressor interface (compress/decompress) with the identity function" do
assert_equal(some_value, subject.compress(some_value))
assert_equal(some_value, subject.decompress(some_value))
end

it "works as a compressor & serializer" do
memcached(29199) do |_, port|
dc = Dalli::Client.new("localhost:#{port}", :compressor => subject, :serializer => subject)
dc.set('some_key', 'some_value')
assert_equal('some_value', dc.get('some_key'))
end
end
end