-
Notifications
You must be signed in to change notification settings - Fork 84
Default values on initialization #119
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
Merged
Merged
Changes from all commits
Commits
Show all changes
39 commits
Select commit
Hold shift + click to select a range
133a0e0
Add default proxy value
tleish 4eff947
Add default list values
tleish 903f2a9
Add default to unique_list
tleish a668f78
Add default to flag
tleish 35dc9bf
Add default to string
tleish e6bcbbd
Add default to integer
tleish ba0092c
Add default to decimal
tleish 52b308b
Add default to datetime
tleish de0e5b1
Add default to float
tleish 77499ed
Add default proc to enum
tleish 46fa83a
rename default_value method to default
tleish abdf95a
Add default proc to set
tleish 4933f6c
Add additional unit tests
tleish 321d33c
Add default proc to json and counter
tleish 411d04a
Add default proc to hash and boolean
tleish ed3cbd8
Add before_method_hook
tleish 1030836
Fix for ruby 3
tleish 1bb9288
refactor Kredis::Types::BeforeMethodsHook#before_methods
tleish 10ceac9
code review updates
tleish 1bff6bf
merge main
tleish 7e85720
fix code review feedback
tleish 180f57c
Match indentation
dhh bf2ba58
create custom callnx method and refactor
tleish d0146ee
add additional default options to set
tleish 90d9208
move primary #set_default method to Kredis::Types::Proxying
tleish 5c4664b
updated enum multi block to use standar [-1] pattern instead of .last
tleish ddf555f
refactor init_default_in_multi to not use multi if default not defined
tleish ef2eacb
refactor multi in Kredis::Types::Proxying
tleish 4b1ab12
Merge remote-tracking branch 'origin/main' into default-values-on-ini…
lewispb 6faf4d0
Set default values on type initialization
lewispb a140434
Merge branch 'main' into default-values-on-initialize
lewispb 52000bb
Run Rubocop autocorrect over default value tests
lewispb 2237ecc
Document default values and organize readme
lewispb c32c663
Support concurrent initalization with defaults
lewispb c808076
Avoid breaking current enum functionality, allowing setting an invali…
lewispb b29a895
Now we have proper concurrency, no need for scalar default nx: true
lewispb 5c5ee24
Restore test for enum not existing when value is nil
lewispb cc08f25
Setting an invalid enum default is a bug
lewispb b1a818a
No need to double proxy watch
lewispb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
# frozen_string_literal: true | ||
|
||
module Kredis::DefaultValues | ||
extend ActiveSupport::Concern | ||
|
||
prepended do | ||
attr_writer :default | ||
|
||
proxying :watch, :unwatch, :exists? | ||
|
||
def default | ||
case @default | ||
when Proc then @default.call | ||
when Symbol then send(@default) | ||
else @default | ||
end | ||
end | ||
|
||
private | ||
def set_default | ||
raise NotImplementedError, "Kredis type #{self.class} needs to define #set_default" | ||
end | ||
end | ||
|
||
def initialize(...) | ||
super | ||
|
||
if default | ||
watch do | ||
set_default unless exists? | ||
|
||
unwatch | ||
end | ||
end | ||
end | ||
end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this occur every request, or only when a value does not exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the context of a Rails web request, as the Kredis object is initialized, the key is checked with WATCH, EXISTS, UNWATCH once, but then subsequent operations within the request do not incur any Redis command overhead. If no default value is specified, no additional Redis command overhead is incurred at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify:
Scenario 1: Default NOT defined
(no additional overhead)
Scenario 2: Default defined, value NOT exists
WATCH, EXISTS, UNWATCH
Scenario 3: Default defined, value exists
(no additional overhead?)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it's:
Scenario 1: Default NOT defined
(no additional overhead)
Scenario 2: Default defined, value NOT exists
WATCH, EXISTS, write default (depending on type), UNWATCH
Scenario 3: Default defined, value exists
WATCH, EXISTS, UNWATCH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems a more efficient approach would be to only add extra overhead of default is defined AND value doesn't exist. In other words:
This avoids extra overhead if the value has been set already.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree @tleish. That's exactly what we do here, but with the added WATCH / UNWATCH.
To illustrate the problem that the WATCH / UNWATCH solves check this test case:
kredis/test/types/counter_test.rb
Lines 113 to 121 in b1a818a
Without the WATCH / UNWATCH, the value existing check is prone to a race condition.