From b6c27f34fa43e941c4a8ad16a80392fd5e5123ad Mon Sep 17 00:00:00 2001
From: rhymes <rhymesete@gmail.com>
Date: Mon, 9 Sep 2019 16:15:12 +0200
Subject: [PATCH] Use a sortable unique ID for the event_id (#3972)

---
 Gemfile                                     |  1 +
 Gemfile.lock                                |  2 ++
 app/models/webhook/event.rb                 |  7 +++++--
 app/serializers/webhook/event_serializer.rb |  6 +++---
 app/services/secrets/generator.rb           | 10 ++++++++++
 spec/models/webhook/event_spec.rb           | 15 ++++++++++++++-
 spec/services/secrets/generator_spec.rb     | 18 ++++++++++++++++++
 7 files changed, 53 insertions(+), 6 deletions(-)
 create mode 100644 app/services/secrets/generator.rb
 create mode 100644 spec/services/secrets/generator_spec.rb

diff --git a/Gemfile b/Gemfile
index 8efb99ff7..7d48a5ba8 100644
--- a/Gemfile
+++ b/Gemfile
@@ -94,6 +94,7 @@ gem "timber-rails", "~> 1.0" #  Timber integration for Rails
 gem "twilio-ruby", "~> 5.25" # The official library for communicating with the Twilio REST API
 gem "twitter", "~> 6.2" # A Ruby interface to the Twitter API
 gem "uglifier", "~> 4.1" # Uglifier minifies JavaScript files
+gem "ulid", "~> 1.1" # Universally Unique Lexicographically Sortable Identifier implementation for Ruby
 gem "validate_url", "~> 1.0" # Library for validating urls in Rails
 gem "webpacker", "~> 3.5" # Use webpack to manage app-like JavaScript modules in Rails
 gem "webpush", "~> 1.0" # Encryption Utilities for Web Push payload
diff --git a/Gemfile.lock b/Gemfile.lock
index 3040fd87d..6bbf64e31 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -791,6 +791,7 @@ GEM
     uber (0.1.0)
     uglifier (4.1.20)
       execjs (>= 0.3.0, < 3)
+    ulid (1.1.0)
     unf (0.1.4)
       unf_ext
     unf_ext (0.0.7.6)
@@ -973,6 +974,7 @@ DEPENDENCIES
   twilio-ruby (~> 5.25)
   twitter (~> 6.2)
   uglifier (~> 4.1)
+  ulid (~> 1.1)
   validate_url (~> 1.0)
   vcr (~> 5.0)
   web-console (~> 3.7)
diff --git a/app/models/webhook/event.rb b/app/models/webhook/event.rb
index ac11e7d54..74baf9182 100644
--- a/app/models/webhook/event.rb
+++ b/app/models/webhook/event.rb
@@ -6,14 +6,17 @@ module Webhook
       article_destroyed
     ].freeze
 
-    attr_reader :event_type, :payload, :timestamp
+    attr_reader :event_type, :payload, :timestamp, :event_id
 
     def initialize(event_type:, payload: {})
       raise InvalidEvent unless EVENT_TYPES.include?(event_type)
 
       @event_type = event_type
       @payload = payload
-      @timestamp = Time.current.rfc3339
+
+      now = Time.current
+      @timestamp = now.rfc3339
+      @event_id = Secrets::Generator.sortable(now)
     end
 
     def as_json(*_args)
diff --git a/app/serializers/webhook/event_serializer.rb b/app/serializers/webhook/event_serializer.rb
index 5c6b48623..ea0995d13 100644
--- a/app/serializers/webhook/event_serializer.rb
+++ b/app/serializers/webhook/event_serializer.rb
@@ -1,10 +1,10 @@
 module Webhook
   class EventSerializer
     include FastJsonapi::ObjectSerializer
+
     set_type :webhook_event
-    set_id do |event|
-      "#{event.event_type}_#{event.timestamp}"
-    end
+    set_id :event_id
+
     attributes :event_type, :timestamp, :payload
   end
 end
diff --git a/app/services/secrets/generator.rb b/app/services/secrets/generator.rb
new file mode 100644
index 000000000..80c9bf59a
--- /dev/null
+++ b/app/services/secrets/generator.rb
@@ -0,0 +1,10 @@
+module Secrets
+  module Generator
+    module_function
+
+    # Generates a unique and lexicographically sortable ID
+    def sortable(time = Time.current)
+      ULID.generate(time)
+    end
+  end
+end
diff --git a/spec/models/webhook/event_spec.rb b/spec/models/webhook/event_spec.rb
index 831d0f47a..9e4d05244 100644
--- a/spec/models/webhook/event_spec.rb
+++ b/spec/models/webhook/event_spec.rb
@@ -4,7 +4,7 @@ RSpec.describe Webhook::Event, type: :model do
   let(:article) { create(:article) }
   let!(:payload) { Webhook::PayloadAdapter.new(article).hash }
 
-  it "rases an exception" do
+  it "raises an exception with a unknown event type" do
     expect do
       described_class.new(event_type: "cool_event")
     end.to raise_error(Webhook::InvalidEvent)
@@ -29,6 +29,19 @@ RSpec.describe Webhook::Event, type: :model do
       expect(attributes[:event_type]).to eq("article_updated")
       expect(attributes[:payload][:data][:attributes][:title]).to eq(article.title)
     end
+
+    it "provides an event_id dependent on time" do
+      event1 = described_class.new(event_type: "article_updated", payload: payload)
+      event1_id = event1.as_json.dig(:data, :id)
+
+      event2 = nil
+      Timecop.freeze(1.month.ago) do
+        event2 = described_class.new(event_type: "article_updated", payload: payload)
+      end
+      event2_id = event2.as_json.dig(:data, :id)
+
+      expect(event2_id < event1_id).to be(true)
+    end
   end
 
   describe "#to_json" do
diff --git a/spec/services/secrets/generator_spec.rb b/spec/services/secrets/generator_spec.rb
new file mode 100644
index 000000000..d441d1b52
--- /dev/null
+++ b/spec/services/secrets/generator_spec.rb
@@ -0,0 +1,18 @@
+require "rails_helper"
+
+RSpec.describe Secrets::Generator, type: :service do
+  describe ".sortable" do
+    it "generates unique identifiers" do
+      expect(described_class.sortable).not_to eq(described_class.sortable)
+    end
+
+    it "generates sortable identifiers" do
+      now_id = described_class.sortable
+      a_month_from_now_id = described_class.sortable(1.month.from_now)
+      a_month_ago_id = described_class.sortable(1.month.ago)
+
+      expected_ids = [a_month_ago_id, now_id, a_month_from_now_id]
+      expect([now_id, a_month_from_now_id, a_month_ago_id].sort).to eq(expected_ids)
+    end
+  end
+end