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

add auto-generated timestamp and sent_at (ruby-sdk) #97

Merged
merged 7 commits into from
Jan 12, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
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
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

## master

## 3.3.0 (2017-12-15)

**BREAKING CHANGES:**

- [#97](github.com/castle/castle-ruby/pull/97) when data is sent in batches you may want to wrap data options with to_options method before you send it to the worker (see README) to include proper timestamp in the query

**Features:**

- [#97](github.com/castle/castle-ruby/pull/97) `Castle::Client` has additional option `timestamp`, `timestamp` and `sent_at` time values are automatically added to the requests, added `Castle::Client.to_options` method which adds properly formatted timestamp param to the options

## 3.2.0 (2017-12-15)

**BREAKING CHANGES:**
Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ group :test do
gem 'coveralls_reborn'
gem 'rspec'
gem 'simplecov'
gem 'timecop'
gem 'webmock'
end
2 changes: 2 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ GEM
term-ansicolor (1.6.0)
tins (~> 1.0)
thor (0.20.0)
timecop (0.9.1)
tins (1.16.3)
webmock (3.2.1)
addressable (>= 2.3.6)
Expand All @@ -63,6 +64,7 @@ DEPENDENCIES
rake
rspec
simplecov
timecop
webmock

BUNDLED WITH
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ end

```ruby
request_context = ::Castle::Client.to_context(request)
track_options = {
track_options = ::Castle::Client.to_options({
event: '$login.succeeded',
user_id: user.id,
properties: {
Expand All @@ -131,6 +131,6 @@ track_options = {
traits: {
key: 'value'
}
}
})
CastleTrackingWorker.perform_async(request_context, track_options)
```
2 changes: 1 addition & 1 deletion lib/castle.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
require 'castle/utils/merger'
require 'castle/utils/cloner'
require 'castle/context_merger'
require 'castle/context_sanitizer'
require 'castle/default_context'
require 'castle/commands/with_context'
require 'castle/commands/identify'
require 'castle/commands/authenticate'
require 'castle/commands/track'
Expand Down
26 changes: 19 additions & 7 deletions lib/castle/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,28 @@ module Castle
class Client
class << self
def from_request(request, options = {})
new(to_context(request, options), options[:do_not_track])
new(
to_context(request, options),
to_options(options)
)
end

def to_context(request, options = {})
default_context = Castle::DefaultContext.new(request, options[:cookies]).call
Castle::ContextMerger.new(default_context).call(options[:context] || {})
def to_context(request, context_options = {})
Copy link
Contributor

Choose a reason for hiding this comment

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

why renaming to context_options, I think if any I would call it additional_context see other methods

default_context = Castle::DefaultContext.new(request, context_options[:cookies]).call
Castle::ContextMerger.call(default_context, context_options[:context])
end

def to_options(options = {})
options[:timestamp] ||= Time.now.iso8601
Copy link
Contributor

Choose a reason for hiding this comment

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

use Time.now.utc.iso8601(3)

options
end
end

attr_accessor :api

def initialize(context, do_not_track = false)
@do_not_track = do_not_track
def initialize(context, options = {})
@do_not_track = options[:do_not_track]
@timestamp = options[:timestamp]
@context = context
@api = API.new
end
Expand All @@ -25,6 +34,7 @@ def authenticate(options = {})
options = Castle::Utils.deep_symbolize_keys(options || {})

if tracked?
options[:timestamp] = @timestamp if @timestamp
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if we should allow it for authenticate thoughts @wallin?

command = Castle::Commands::Authenticate.new(@context).build(options)
begin
@api.request(command).merge(failover: false, failover_reason: nil)
Expand All @@ -40,6 +50,7 @@ def identify(options = {})
options = Castle::Utils.deep_symbolize_keys(options || {})

return unless tracked?
options[:timestamp] = @timestamp if @timestamp
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd change it for all methods to options[:timestamp] ||= that way you can also pass in a timestamp to identify/track


command = Castle::Commands::Identify.new(@context).build(options)
@api.request(command)
Expand All @@ -49,6 +60,7 @@ def track(options = {})
options = Castle::Utils.deep_symbolize_keys(options || {})

return unless tracked?
options[:timestamp] = @timestamp if @timestamp

command = Castle::Commands::Track.new(@context).build(options)
@api.request(command)
Expand All @@ -70,7 +82,7 @@ def tracked?

def setup_context(request, cookies, additional_context)
default_context = Castle::DefaultContext.new(request, cookies).call
Castle::ContextMerger.new(default_context).call(additional_context || {})
Castle::ContextMerger.call(default_context, additional_context)
end

def failover_response_or_raise(failover_response, error)
Expand Down
11 changes: 8 additions & 3 deletions lib/castle/commands/authenticate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@
module Castle
module Commands
class Authenticate
include WithContext
def initialize(context)
@context = context
end

def build(options = {})
validate!(options)
build_context!(options)
context = ContextMerger.call(@context, options[:context])
context = ContextSanitizer.call(context)

Castle::Command.new('authenticate', options, :post)
Castle::Command.new('authenticate',
options.merge(context: context, sent_at: Time.now.iso8601),
Copy link
Contributor

Choose a reason for hiding this comment

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

Time.now.utc.is8601(3) also shouldn't this be the responsibility of Castle::Command will DRY it up and add responsibility to the Object making the request

:post)
end

private
Expand Down
11 changes: 8 additions & 3 deletions lib/castle/commands/identify.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@
module Castle
module Commands
class Identify
include WithContext
def initialize(context)
@context = context
end

def build(options = {})
validate!(options)
build_context!(options)
context = ContextMerger.call(@context, options[:context])
context = ContextSanitizer.call(context)

Castle::Command.new('identify', options, :post)
Castle::Command.new('identify',
options.merge(context: context, sent_at: Time.now.iso8601),
Copy link
Contributor

Choose a reason for hiding this comment

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

see comment on authenticate

:post)
end

private
Expand Down
11 changes: 8 additions & 3 deletions lib/castle/commands/track.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@
module Castle
module Commands
class Track
include WithContext
def initialize(context)
@context = context
end

def build(options = {})
validate!(options)
build_context!(options)
context = ContextMerger.call(@context, options[:context])
context = ContextSanitizer.call(context)

Castle::Command.new('track', options, :post)
Castle::Command.new('track',
options.merge(context: context, sent_at: Time.now.iso8601),
Copy link
Contributor

Choose a reason for hiding this comment

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

see comment on authenticate

:post)
end

private
Expand Down
28 changes: 0 additions & 28 deletions lib/castle/commands/with_context.rb

This file was deleted.

11 changes: 5 additions & 6 deletions lib/castle/context_merger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@

module Castle
class ContextMerger
def initialize(context)
@main_context = Castle::Utils::Cloner.call(context)
end

def call(request_context)
Castle::Utils::Merger.call(@main_context, request_context)
class << self
def call(initial_context, request_context)
main_context = Castle::Utils::Cloner.call(initial_context)
Castle::Utils::Merger.call(main_context, request_context || {})
end
end
end
end
19 changes: 19 additions & 0 deletions lib/castle/context_sanitizer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# frozen_string_literal: true

module Castle
class ContextSanitizer
class << self
def call(context)
sanitized_active_mode(context) || {}
end

private

def sanitized_active_mode(context)
return context unless context && context.key?(:active)
return context if [true, false].include?(context[:active])
context.reject { |key| key == :active }
end
end
end
end
16 changes: 8 additions & 8 deletions lib/castle/utils/merger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@ module Castle
module Utils
class Merger
def self.call(first, second)
Castle::Utils.deep_symbolize_keys!(first)
Castle::Utils.deep_symbolize_keys!(second)
first_s = Castle::Utils.deep_symbolize_keys(first)
second_s = Castle::Utils.deep_symbolize_keys(second)

second.each do |name, value|
second_s.each do |name, value|
if value.nil?
first.delete(name)
elsif value.is_a?(Hash) && first[name].is_a?(Hash)
call(first[name], value)
first_s.delete(name)
elsif value.is_a?(Hash) && first_s[name].is_a?(Hash)
first_s[name] = call(first_s[name], value)
else
first[name] = value
first_s[name] = value
end
end
first
first_s
end
end
end
Expand Down
Loading