Skip to content

Publish lifecycle events to routemaster #19

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 35 commits into from
Aug 8, 2017
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
5c917b0
Rough outline of event publishing, no tests yet
gregbeech Apr 12, 2017
f8176e9
Simplify publisher registration and rename mixing
gregbeech Apr 18, 2017
0601a2f
Merge branch 'master' into publish-lifecycle-events-to-routemaster
mezis Apr 19, 2017
e9df9b8
Tidy up some of the publisher code
gregbeech Apr 19, 2017
d1cb948
Add helper methods to check the event type
gregbeech Apr 19, 2017
82d1e97
Merge branch 'master' into publish-lifecycle-events-to-routemaster
gregbeech May 22, 2017
16c13ba
Add publishers spec
gregbeech Jun 1, 2017
431a46f
Merge branch 'master' into publish-lifecycle-events-to-routemaster
kplattret Jul 17, 2017
25eb951
Add routemaster-client to dependencies
kplattret Jul 17, 2017
7e347e9
Add tests for Routemaster::Publisher
kplattret Jul 17, 2017
914c7ee
Add test example for #publish!
kplattret Jul 18, 2017
346f6ff
Merge branch 'master' into publish-lifecycle-events-to-routemaster
kplattret Jul 18, 2017
e8028ac
Try with method stubs instead
kplattret Jul 18, 2017
b8d7b2a
Merge branch 'master' into publish-lifecycle-events-to-routemaster
kplattret Jul 25, 2017
3214b11
Update instructions in README
kplattret Jul 26, 2017
79c37aa
Merge branch 'master' into publish-lifecycle-events-to-routemaster
kplattret Jul 26, 2017
d51f58f
Not sure how these changes slipped through
kplattret Jul 26, 2017
c36a5ff
Update CHANGELOG.md
kplattret Jul 26, 2017
2f3032f
Merge branch 'master' into publish-lifecycle-events-to-routemaster
kplattret Jul 28, 2017
97b1d63
Move RM client config to RooOnRails and add further documentation
kplattret Jul 31, 2017
11cf4de
Require Routemaster and refactor config_spec
kplattret Aug 1, 2017
2b94c46
Fix empty array issue and add Routemaster integration test
kplattret Aug 2, 2017
1221947
Fix the integration tests
kplattret Aug 2, 2017
5609396
Add lifecycle method for each verb and add tests
kplattret Aug 2, 2017
ebe8518
Refactor LifecycleEvents and add tests
kplattret Aug 2, 2017
b3bcd14
Introduce ROUTEMASTER_PUBLISHING_ENBALED env var
kplattret Aug 2, 2017
fed4763
Update documentation to reflect latest changes
kplattret Aug 2, 2017
f9a97d9
Small tweaks to LifecycleEvents spec
kplattret Aug 2, 2017
fbb7baa
Fix tiny typo in the README
kplattret Aug 3, 2017
1c207a4
Check if create, update or noop event before publishing
kplattret Aug 3, 2017
5fded8e
Rename ROOBUS_ to ROUTEMASTER_
kplattret Aug 4, 2017
b42f01c
Unblock CodeClimate
kplattret Aug 7, 2017
8afb4e6
Fix CodeClimate issue
kplattret Aug 7, 2017
e6b6926
Fix Routemaster integration test
kplattret Aug 7, 2017
fddf7bf
Address PR comments
kplattret Aug 7, 2017
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# HEAD (2017-07-28)

Features:

- Publish lifecycle events to Routemaster (#19)

# v1.8.1 (2017-07-27)

Features:
Expand Down
30 changes: 19 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,18 @@
**Table of Contents**

- [Installation](#installation)
- [Usage](#usage)
- [Library features](#library-features)
- [New Relic configuration](#new-relic-configuration)
- [Rack middleware](#rack-middleware)
- [Database configuration](#database-configuration)
- [Sidekiq](#sidekiq)
- [HireFire Workers](#hirefire-workers)
- [Logging](#logging)
- [Google Oauth](#google-oauth)
- [Configuration and usage](#configuration-and-usage)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep the "library usage" v "command features" sections? Users have been confused by the dual nature of roo_on_rails (it's both a library/framework complement and a CLI, usable separately).

Copy link
Contributor

Choose a reason for hiding this comment

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

Absolutely! I was a bit confused by this table of content myself, partly because it changed while I was working on this PR. I'll rename "Configuration and usage" to "Library usage" 👍

- [New Relic configuration](#new-relic-configuration)
- [Rack middleware](#rack-middleware)
- [Database configuration](#database-configuration)
- [Sidekiq](#sidekiq)
- [HireFire (for Sidekiq workers)](#hirefire-for-sidekiq-workers)
- [Logging](#logging)
- [Google OAuth authentication](#google-oauth-authentication)
- [Routemaster Client](#routemaster-client)
- [Command features](#command-features)
- [Usage](#usage)
- [Description](#description)
- [Contributing](#contributing)
- [License](#license)

Expand Down Expand Up @@ -57,8 +59,6 @@ And then execute:

Then re-run your test suite to make sure everything is shipshape.

## Usage

## Configuration and usage

### New Relic configuration
Expand Down Expand Up @@ -199,6 +199,14 @@ application.

A simple but secure example is detailed in `README.google_oauth2.md`.

### Routemaster Client

When `ROUTEMASTER_ENABLED` is set to `true` we attempt to configure [`routemaster-client`](https://github.com/deliveroo/routemaster-client) on your application. In order for this to happen you need to set the following environment variables:

* `ROOBUS_URL` – the full URL of your Routemaster application (mandatory)
* `ROOBUS_UUID` – the UUID of your application, e.g. `logistics-dashboard` (mandatory)

If you then want to enable the publishing of events onto the event bus, you need to set `ROUTEMASTER_PUBLISHING_ENABLED` to `true` and implement publishers as needed. An example of how to do this is detailed in [`README.routemaster_client.md`](README.routemaster_client.md).

## Command features

Expand Down
107 changes: 107 additions & 0 deletions README.routemaster_client.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
## Using the Routemaster Client feature

[`routemaster-client`](https://github.com/deliveroo/routemaster-client) comes as a dependency of `roo_on_rails` with a basic implementation of lifecycle event publishers.

This code example assumes that you are using the latest version of the [`roo_on_rails`](roo_on_rails) gem and that you have set the correct environment variables for Routemaster Client to work on your app, as explained in the main [`README.md`](roo_on_rails#routemaster-client) file.

It also assumes that your app has an API for the resources you want to publish lifecycle events for, with matching routes and an `API_HOST` environment variable set.

### Setup lifecycle events for your models

We will most likely want to publish lifecycle events for several models, so to write slightly less code let's create a model concern first:

```ruby
# app/models/concerns/routemaster_lifecycle_events.rb
require 'roo_on_rails/routemaster/lifecycle_events'

module RoutemasterLifecycleEvents
extend ActiveSupport::Concern
include RooOnRails::Routemaster::LifecycleEvents

included do
publish_lifecycle_events
end
end
```

Then let's include this concern to the relevant model(s):

```ruby
# app/models/order.rb
class Order < ApplicationRecord
include RoutemasterLifecycleEvents

# ...
end
```

And another one for the example:

```ruby
# app/models/rider.rb
class Rider < ApplicationRecord
include RoutemasterLifecycleEvents

# ...
end
```

### Create publishers for lifecycle events

We have now configured our models to publish lifecycle events to Routemaster, but it won't send anything until you have enabled publishing and created matching publishers. Let's start with creating a `BasePublisher` that we can then inherit from:

```ruby
# app/publishers/base_publisher.rb
require 'roo_on_rails/routemaster/publisher'

class BasePublisher < RooOnRails::Routemaster::Publisher
include Rails.application.routes.url_helpers

def publish?
noop? || model.new_record? || model.previous_changes.any?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could always move this into the base implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds reasonable to me 👍

end
end
```

Then create a publisher for each model with lifecycle events enabled:

```ruby
# app/publishers/order_publisher.rb
class OrderPublisher < BasePublisher
def url
api_order_url(model, host: ENV.fetch('API_HOST'), protocol: 'https')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could move this into the base implementation too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, no, this one can't really move into the base implementation because sometimes there's an api_ prefix on the routes, sometimes not, and sometimes it's different. That's why I just raised I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think this one is best to handle in the individual service apps indeed.

end
end
```

and

```ruby
# app/publishers/rider_publisher.rb
class RiderPublisher < BasePublisher
def url
api_rider_url(model, host: ENV.fetch('API_HOST'), protocol: 'https')
end
end
```

### Register the publishers with Routemaster

The final step is to tell Routemaster that these publishers exist, so that it can listen to their events. We're going to do this in an initialiser:

```ruby
# config/initilizers/routemaster.rb
require 'roo_on_rails/routemaster/publishers'

PUBLISHERS = [
OrderPublisher,
RiderPublisher
].freeze

PUBLISHERS.each do |publisher|
model_class = publisher.to_s.gsub("Publisher", "").constantize
RooOnRails::Routemaster::Publishers.register(publisher, model_class: model_class)
end
```

We should now be all set for our app to publish lifecycle events for `orders` and `riders` onto the event bus, so that other apps can listen to them.
1 change: 1 addition & 0 deletions lib/roo_on_rails.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ module RooOnRails
require 'roo_on_rails/railties/sidekiq'
require 'roo_on_rails/railties/rake_tasks'
require 'roo_on_rails/railties/google_oauth'
require 'roo_on_rails/railties/routemaster'
end
8 changes: 8 additions & 0 deletions lib/roo_on_rails/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ def google_auth_controller
ENV.fetch('GOOGLE_AUTH_CONTROLLER')
end

def routemaster_enabled?
enabled? 'ROUTEMASTER_ENABLED', default: false
end

def routemaster_publishing_enabled?
enabled? 'ROUTEMASTER_PUBLISHING_ENABLED', default: false
end

private

ENABLED_PATTERN = /\A(YES|TRUE|ON|1)\Z/i
Expand Down
3 changes: 3 additions & 0 deletions lib/roo_on_rails/default.env
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ NEW_RELIC_DEVELOPER_MODE=false
NEW_RELIC_LICENSE_KEY=override-me
RACK_SERVICE_TIMEOUT=10
RACK_WAIT_TIMEOUT=30
ROOBUS_URL=''
ROOBUS_UUID=''
ROUTEMASTER_ENABLED=false
SIDEKIQ_ENABLED=true
SIDEKIQ_THREADS=25
SIDEKIQ_DATABASE_REAPING_FREQUENCY=10
36 changes: 36 additions & 0 deletions lib/roo_on_rails/railties/routemaster.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
require 'roo_on_rails/config'

module RooOnRails
module Railties
class Routemaster < Rails::Railtie
initializer 'roo_on_rails.routemaster' do
next unless Config.routemaster_enabled?

$stderr.puts 'initializer roo_on_rails.routemaster'

abort 'Aborting: ROOBUS_URL and ROOBUS_UUID are required' if roobus_credentials_blank?

require 'routemaster/client'

::Routemaster::Client.configure do |config|
config.url = roobus_url
config.uuid = roobus_uuid
end
end

private

def roobus_credentials_blank?
roobus_url.blank? && roobus_uuid.blank?
end

def roobus_url
ENV.fetch('ROOBUS_URL')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It strikes me that using ROUTEMASTER_URL and ROUTEMASTER_UUID would probably be cleaner names, but if people are dead set on ROOBUS_ then I guess that's fine.

Copy link
Contributor

@kplattret kplattret Aug 3, 2017

Choose a reason for hiding this comment

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

I just assumed that these were prefixed with ROOBUS_ for a good reason and didn't even question it, but it would definitely make sense to be more consistent and use ROUTEMASTER_. Happy to make this change if people agree.

Note: It looks like it would just leave ROOBUS_CALLBACK_URL using ROOBUS_.

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 prefer ROUTEMASTER_ over ROOBUS_

end

def roobus_uuid
ENV.fetch('ROOBUS_UUID')
end
end
end
end
56 changes: 56 additions & 0 deletions lib/roo_on_rails/routemaster/lifecycle_events.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
require 'active_support/concern'
require 'new_relic/agent'
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing a require for roo_on_rails/routemaster/publishers which is referenced.

require 'roo_on_rails/routemaster/publishers'

module RooOnRails
module Routemaster
module LifecycleEvents
extend ActiveSupport::Concern

ACTIVE_RECORD_TO_ROUTEMASTER_EVENT_MAP = {
create: :created,
update: :updated,
destroy: :deleted,
noop: :noop
}.freeze
private_constant :ACTIVE_RECORD_TO_ROUTEMASTER_EVENT_MAP

def publish_lifecycle_event(event)
publishers = Routemaster::Publishers.for(self, routemaster_event_type(event))
publishers.each do |publisher|
begin
publisher.publish!
rescue => e
NewRelic::Agent.notice_error(e)
end
end
end

private

def routemaster_event_type(event)
ACTIVE_RECORD_TO_ROUTEMASTER_EVENT_MAP[event].tap do |type|
raise "invalid lifecycle event '#{event}'" unless type
end
end

%i(create update destroy noop).each do |event|
define_method("publish_lifecycle_event_on_#{event}") do
publish_lifecycle_event(event)
end
end

module ClassMethods
def publish_lifecycle_events(*events)
events = events.any? ? events : %i(create update destroy)
events.each do |event|
after_commit(
:"publish_lifecycle_event_on_#{event}",
on: event
)
end
end
end
end
end
end
57 changes: 57 additions & 0 deletions lib/roo_on_rails/routemaster/publisher.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
require 'roo_on_rails/config'
require 'routemaster/client'

module RooOnRails
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing a require for routemaster/client.

module Routemaster
class Publisher
attr_reader :model, :event

def initialize(model, event, client: ::Routemaster::Client)
@model = model
@event = event
@client = client
end

def publish?
Copy link

Choose a reason for hiding this comment

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

Why does this method always return true?

Copy link
Contributor

Choose a reason for hiding this comment

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

The intention is that it's ready to be overloaded in the clients own publishers

Copy link
Contributor

Choose a reason for hiding this comment

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

The idea is that it's true by default and you can then overwrite this in your implementation. You can see this in the example I provide.

true
end

def will_publish?
Config.routemaster_publishing_enabled? && publish?
end

def publish!
return unless will_publish?
@client.send(event, topic, url, data: stringify_keys(data))
end

def topic
@model.class.name.tableize
end

def url
raise NotImplementedError
end

def data
nil
end

%i(created updated deleted noop).each do |event_type|
define_method :"#{event_type}?" do
event.to_sym == event_type
end
end

private

def stringify_keys(hash)
return hash if hash.nil? || hash.empty?

hash.each_with_object({}) do |(k, v), h|
h[k.to_s] = v.is_a?(Hash) ? stringify_keys(v) : v
end
end
end
end
end
17 changes: 17 additions & 0 deletions lib/roo_on_rails/routemaster/publishers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
module RooOnRails
module Routemaster
module Publishers
@publishers = {}

def self.register(publisher_class, model_class:)
@publishers[model_class] ||= Set.new
@publishers[model_class] << publisher_class
end

def self.for(model, event)
publisher_classes = @publishers[model.class]
publisher_classes.map { |c| c.new(model, event) }
end
end
end
end
1 change: 1 addition & 0 deletions roo_on_rails.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Gem::Specification.new do |spec|
spec.add_runtime_dependency 'omniauth-google-oauth2'
spec.add_runtime_dependency 'faraday'
spec.add_runtime_dependency 'faraday_middleware'
spec.add_runtime_dependency 'routemaster-client'

spec.add_development_dependency 'bundler', '~> 1.13'
spec.add_development_dependency 'rake', '~> 10.0'
Expand Down
Loading