- 
                Notifications
    
You must be signed in to change notification settings  - Fork 83
 
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.