From 9b314271d6cfdad2494f7eb06aaa2ba168f20541 Mon Sep 17 00:00:00 2001 From: Rui Baltazar Date: Mon, 20 Nov 2017 14:20:45 +0800 Subject: [PATCH 01/11] WIP - updated gem file and gemspec config --- Gemfile | 9 ++---- Gemfile.lock | 2 -- test/rails-app/Gemfile | 19 +++++------ test/rails-app/Gemfile.lock | 63 ++++++++++++++++-------------------- trailblazer-endpoint.gemspec | 11 +++---- 5 files changed, 43 insertions(+), 61 deletions(-) diff --git a/Gemfile b/Gemfile index a6860ed..1c3a259 100644 --- a/Gemfile +++ b/Gemfile @@ -1,13 +1,10 @@ -source 'https://rubygems.org' +source "https://rubygems.org" # Specify your gem's dependencies in trailblazer.gemspec gemspec -gem "multi_json" - +gem "dry-validation" gem "minitest-line" - +gem "multi_json" gem "trailblazer", path: "../trailblazer" # gem "trailblazer-operation", path: "../operation" - -gem "dry-validation" diff --git a/Gemfile.lock b/Gemfile.lock index 874053f..2d1cb07 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -10,7 +10,6 @@ PATH remote: . specs: trailblazer-endpoint (0.0.1) - dry-matcher GEM remote: https://rubygems.org/ @@ -38,7 +37,6 @@ GEM dry-container (~> 0.2, >= 0.2.6) dry-core (~> 0.1) dry-equalizer (~> 0.2) - dry-matcher (0.5.0) dry-types (0.9.0) concurrent-ruby (~> 1.0) dry-configurable (~> 0.1) diff --git a/test/rails-app/Gemfile b/test/rails-app/Gemfile index f21143a..a8cad0f 100644 --- a/test/rails-app/Gemfile +++ b/test/rails-app/Gemfile @@ -1,17 +1,14 @@ -source 'https://rubygems.org' +source "https://rubygems.org" - -# Bundle edge Rails instead: gem 'rails', github: 'rails/rails' -gem 'rails', '~> 5.0.0', '>= 5.0.0.1' +gem "multi_json" +# Bundle edge Rails instead: gem "rails", github: "rails/rails" +gem "rails", "~> 5.0.0", ">= 5.0.0.1" # Use sqlite3 as the database for Active Record -gem 'sqlite3' +gem "sqlite3" # Windows does not include zoneinfo files, so bundle the tzinfo-data gem -gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] +gem "tzinfo-data", platforms: %i[mingw mswin x64_mingw jruby] +gem "trailblazer" gem "trailblazer-endpoint", path: "../../." -gem "trailblazer", path: "../../../trailblazer" -gem "trailblazer-operation", path: "../../../operation" -# gem "trailblazer-rails" - -gem "multi_json" +gem "trailblazer-operation" diff --git a/test/rails-app/Gemfile.lock b/test/rails-app/Gemfile.lock index a72cf06..afe0a8c 100644 --- a/test/rails-app/Gemfile.lock +++ b/test/rails-app/Gemfile.lock @@ -1,25 +1,7 @@ PATH - remote: ../../../operation - specs: - trailblazer-operation (0.0.6) - declarative - pipetree (>= 0.0.4, < 0.1.0) - uber (>= 0.1.0, < 0.2.0) - -PATH - remote: ../../../trailblazer - specs: - trailblazer (2.0.0.beta1) - declarative - reform (>= 2.2.0, < 3.0.0) - trailblazer-operation - uber (>= 0.1.0, < 0.2.0) - -PATH - remote: ../../. + remote: ../.. specs: trailblazer-endpoint (0.0.1) - dry-matcher GEM remote: https://rubygems.org/ @@ -64,13 +46,16 @@ GEM arel (7.1.4) builder (3.2.2) concurrent-ruby (1.0.2) - declarative (0.0.8) - uber (>= 0.0.15) - disposable (0.3.2) - declarative (>= 0.0.8, < 1.0.0) + declarative (0.0.9) + declarative-builder (0.1.0) + declarative-option (< 0.2.0) + declarative-option (0.1.0) + disposable (0.4.3) + declarative (>= 0.0.9, < 1.0.0) + declarative-builder (< 0.2.0) + declarative-option (< 0.2.0) representable (>= 2.4.0, <= 3.1.0) - uber - dry-matcher (0.5.0) + uber (< 0.2.0) erubis (2.7.0) globalid (0.3.7) activesupport (>= 4.1.0) @@ -89,7 +74,7 @@ GEM nio4r (1.2.1) nokogiri (1.6.8.1) mini_portile2 (~> 2.1.0) - pipetree (0.0.4) + pipetree (0.1.1) rack (2.0.1) rack-test (0.6.3) rack (>= 1.0) @@ -117,13 +102,13 @@ GEM rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) rake (11.3.0) - reform (2.2.3) - disposable (>= 0.3.0, < 0.4.0) + reform (2.2.4) + disposable (>= 0.4.1) representable (>= 2.4.0, < 3.1.0) - uber (>= 0.0.15, < 0.2.0) - representable (3.0.2) - declarative (~> 0.0.5) - uber (>= 0.0.15, < 0.2.0) + representable (3.0.4) + declarative (< 0.1.0) + declarative-option (< 0.2.0) + uber (< 0.2.0) sprockets (3.7.0) concurrent-ruby (~> 1.0) rack (> 1, < 3) @@ -134,6 +119,14 @@ GEM sqlite3 (1.3.12) thor (0.19.1) thread_safe (0.3.5) + trailblazer (2.0.6) + declarative + reform (>= 2.2.0, < 3.0.0) + trailblazer-operation (>= 0.0.12, < 0.1.0) + trailblazer-operation (0.0.13) + declarative + pipetree (>= 0.1.1, < 0.2.0) + uber tzinfo (1.2.2) thread_safe (~> 0.1) uber (0.1.0) @@ -148,10 +141,10 @@ DEPENDENCIES multi_json rails (~> 5.0.0, >= 5.0.0.1) sqlite3 - trailblazer! + trailblazer trailblazer-endpoint! - trailblazer-operation! + trailblazer-operation tzinfo-data BUNDLED WITH - 1.12.5 + 1.15.4 diff --git a/trailblazer-endpoint.gemspec b/trailblazer-endpoint.gemspec index b884e25..61775e1 100644 --- a/trailblazer-endpoint.gemspec +++ b/trailblazer-endpoint.gemspec @@ -1,14 +1,14 @@ -lib = File.expand_path('../lib', __FILE__) +lib = File.expand_path("../lib", __FILE__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) -require 'trailblazer/endpoint/version' +require "trailblazer/endpoint/version" Gem::Specification.new do |spec| spec.name = "trailblazer-endpoint" spec.version = Trailblazer::Endpoint::VERSION spec.authors = ["Nick Sutterer"] spec.email = ["apotonick@gmail.com"] - spec.description = %q{Generic HTTP handlers for operation results.} - spec.summary = %q{Generic HTTP handlers for operation results.} + spec.description = "Generic HTTP handlers for operation results." + spec.summary = "Generic HTTP handlers for operation results." spec.homepage = "http://trailblazer.to/gems/operation/endpoint.html" spec.license = "LGPL-3.0" @@ -17,9 +17,6 @@ Gem::Specification.new do |spec| spec.test_files = spec.files.grep(%r{^(test|spec|features)/}) spec.require_paths = ["lib"] - - spec.add_dependency "dry-matcher" - spec.add_development_dependency "bundler" spec.add_development_dependency "rake" spec.add_development_dependency "minitest" From 436ebf0adf7ca638c02bb862086e78b5fc7c007c Mon Sep 17 00:00:00 2001 From: Rui Baltazar Date: Mon, 20 Nov 2017 14:21:43 +0800 Subject: [PATCH 02/11] WIP - added rubocop yml --- .rubocop.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .rubocop.yml diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..6bca04c --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,29 @@ +AllCops: + TargetRubyVersion: 2.3.1 + +Style/StringLiterals: + EnforcedStyle: double_quotes + SupportedStyles: + - single_quotes + - double_quotes + +Metrics/LineLength: + Max: 100 + +Metrics/BlockLength: + CountComments: false # count full line comments? + Max: 25 + ExcludedMethods: [feature, describe, shared_examples_for] + +Style/FrozenStringLiteralComment: + Enabled: false + +Documentation: + Enabled: false + +NumericLiterals: + Exclude: + - 'config/initializers/app_version.rb' + +LambdaCall: + Enabled: false From 1121979e1153faf7f7ff88861ce7e7330298daa8 Mon Sep 17 00:00:00 2001 From: Rui Baltazar Date: Mon, 20 Nov 2017 14:22:03 +0800 Subject: [PATCH 03/11] WIP - re-worked endpoint concept --- lib/trailblazer/endpoint.rb | 114 ++++++++++++++++++++++-------------- 1 file changed, 71 insertions(+), 43 deletions(-) diff --git a/lib/trailblazer/endpoint.rb b/lib/trailblazer/endpoint.rb index 8879e38..124be1e 100644 --- a/lib/trailblazer/endpoint.rb +++ b/lib/trailblazer/endpoint.rb @@ -1,54 +1,82 @@ -require "dry/matcher" - module Trailblazer class Endpoint - # this is totally WIP as we need to find best practices. - # also, i want this to be easily extendable. - Matcher = Dry::Matcher.new( - present: Dry::Matcher::Case.new( # DISCUSS: the "present" flag needs some discussion. - match: ->(result) { result.success? && result["present"] }, - resolve: ->(result) { result }), - success: Dry::Matcher::Case.new( - match: ->(result) { result.success? }, - resolve: ->(result) { result }), - created: Dry::Matcher::Case.new( - match: ->(result) { result.success? && result["model.action"] == :new }, # the "model.action" doesn't mean you need Model. - resolve: ->(result) { result }), - not_found: Dry::Matcher::Case.new( - match: ->(result) { result.failure? && result["result.model"] && result["result.model"].failure? }, - resolve: ->(result) { result }), - # TODO: we could add unauthorized here. - unauthenticated: Dry::Matcher::Case.new( - match: ->(result) { result.failure? && result["result.policy.default"].failure? }, # FIXME: we might need a &. here ;) - resolve: ->(result) { result }), - invalid: Dry::Matcher::Case.new( - match: ->(result) { result.failure? && result["result.contract.default"] && result["result.contract.default"].failure? }, - resolve: ->(result) { result }) - ) + DEFAULT_MATCHERS = { + created: { + rule: ->(result) { result.success? && result["model.action"] == :new }, + resolve: ->(result, representer) do + { + "data": representer.new(result["model"]), + "status": :created + } + end + }, + success: { + rule: ->(result) { result.success? }, + resolve: ->(result, representer) do + { + "data": representer.new(result["results"]), + "status": :ok + } + end + }, + unauthenticated: { + rule: ->(result) { result.policy_error? }, + resolve: ->(_result, _representer) do + { + "data": {}, + "status": :unauthorized + } + end + }, + invalid: { + # rule: ->(result) { result.failure? && result["result.contract.default"]&.failure? }, + rule: ->(result) { result.failure? }, + resolve: ->(_result, _representer) do + { + "data": {}, + "status": :unprocessable_entity + } + end + }, + not_found: { + rule: ->(result) { result.failure? && result["result.model"]&.failure? }, + resolve: ->(result, _representer) do + { + "data": { errors: result["result.model.errors"] }, + "status": :unprocessable_entity + } + end + } + } - # `call`s the operation. - def self.call(operation_class, handlers, *args, &block) - result = operation_class.(*args) - new.(result, handlers, &block) + # options expects a TRB Operation result + # it might have a representer, else will assume the default name + def self.call(operation_result, representer_class = nil, overrides = {}) + # TODO: What to do when nothing matches? + endpoint_opts = { result: operation_result, representer: representer_class } + new.(endpoint_opts, overrides) end - def call(result, handlers=nil, &block) - matcher.(result, &block) and return if block_given? # evaluate user blocks first. - matcher.(result, &handlers) # then, generic Rails handlers in controller context. - end + def call(options, overrides) + overrides.each do |rule_key, rule_description| + rule = rule_description[:rule] || DEFAULT_MATCHERS[rule_key][:rule] + resolve = rule_description[:resolve] || DEFAULT_MATCHERS[rule_key][:resolve] + if rule.nil? || resolve.nil? + puts "Matcher is not properly set. #{rule_key} will be ignored" + next + end - def matcher - Matcher + return resolve.(options[:result], options[:representer]) if rule.(options[:result]) + end + matching_rules(overrides).each do |rule_key, rule_description| + if rule_description[:rule].(options[:result]) + return rule_description[:resolve].(options[:result], options[:representer]) + end + end end - module Controller - # endpoint(Create) do |m| - # m.not_found { |result| .. } - # end - def endpoint(operation_class, options={}, &block) - handlers = Handlers::Rails.new(self, options).() - Endpoint.(operation_class, handlers, *options[:args], &block) - end + def matching_rules(overrides) + DEFAULT_MATCHERS.except(*overrides.keys) end end end From 9e70574d75be99735555b537219b22460f92dc13 Mon Sep 17 00:00:00 2001 From: Rui Baltazar Date: Mon, 20 Nov 2017 15:06:10 +0800 Subject: [PATCH 04/11] WIP - writing specs for current endpoint --- Gemfile | 1 - lib/trailblazer/endpoint.rb | 11 +- test/endpoint_test.rb | 239 ++++++++++++++++++++---------------- test/test_helper.rb | 2 +- 4 files changed, 137 insertions(+), 116 deletions(-) diff --git a/Gemfile b/Gemfile index 1c3a259..fcbf177 100644 --- a/Gemfile +++ b/Gemfile @@ -7,4 +7,3 @@ gem "dry-validation" gem "minitest-line" gem "multi_json" gem "trailblazer", path: "../trailblazer" -# gem "trailblazer-operation", path: "../operation" diff --git a/lib/trailblazer/endpoint.rb b/lib/trailblazer/endpoint.rb index 124be1e..d3cc5d0 100644 --- a/lib/trailblazer/endpoint.rb +++ b/lib/trailblazer/endpoint.rb @@ -14,13 +14,13 @@ class Endpoint rule: ->(result) { result.success? }, resolve: ->(result, representer) do { - "data": representer.new(result["results"]), + "data": representer.new(result["model"]), "status": :ok } end }, unauthenticated: { - rule: ->(result) { result.policy_error? }, + rule: ->(result) { result.failure? && result["result.policy.default"]&.failure? }, resolve: ->(_result, _representer) do { "data": {}, @@ -52,8 +52,9 @@ class Endpoint # options expects a TRB Operation result # it might have a representer, else will assume the default name def self.call(operation_result, representer_class = nil, overrides = {}) + representer = operation_result["representer.serializer.class"] || representer_class # TODO: What to do when nothing matches? - endpoint_opts = { result: operation_result, representer: representer_class } + endpoint_opts = { result: operation_result, representer: representer } new.(endpoint_opts, overrides) end @@ -68,7 +69,7 @@ def call(options, overrides) return resolve.(options[:result], options[:representer]) if rule.(options[:result]) end - matching_rules(overrides).each do |rule_key, rule_description| + matching_rules(overrides).each do |_rule_key, rule_description| if rule_description[:rule].(options[:result]) return rule_description[:resolve].(options[:result], options[:representer]) end @@ -76,7 +77,7 @@ def call(options, overrides) end def matching_rules(overrides) - DEFAULT_MATCHERS.except(*overrides.keys) + DEFAULT_MATCHERS.reject { |k, _v| overrides.keys.include? k } end end end diff --git a/test/endpoint_test.rb b/test/endpoint_test.rb index 3abc0bc..6fbca51 100644 --- a/test/endpoint_test.rb +++ b/test/endpoint_test.rb @@ -1,4 +1,4 @@ -require "test_helper" +require "./test_helper" require "reform" require "trailblazer" @@ -6,9 +6,15 @@ require "trailblazer/endpoint" require "trailblazer/endpoint/rails" +require "byebug" + class EndpointTest < Minitest::Spec + # NOTE: Consider moving all this code to a separate class as + # it is relevant for the test but it is boilerplate for testing Song = Struct.new(:id, :title, :length) do - def self.find_by(id:nil); id.nil? ? nil : new(id) end + def self.find_by(id: nil) + id.nil? ? nil : new(id) + end end require "representable/json" @@ -29,37 +35,12 @@ class Deserializer < Representable::Decorator property :title end - let (:my_handlers) { - ->(m) do - m.present { |result| _data << result["representer.serializer.class"].new(result["model"]).to_json } - end - } - - #--- - # present class Show < Trailblazer::Operation extend Representer::DSL - step Model( Song, :find_by ) + step Model(Song, :find_by) representer :serializer, Serializer end - # if you pass in "present"=>true as a dependency, the Endpoint will understand it's a present cycle. - it do - Trailblazer::Endpoint.new.(Show.({ id: 1 }, { "present" => true }), my_handlers) - _data.must_equal ['{"id":1}'] - end - - # passing handlers directly to Endpoint#call. - it do - result = Show.({ id: 1 }, { "present" => true }) - Trailblazer::Endpoint.new.(result) do |m| - m.present { |result| _data << result["representer.serializer.class"].new(result["model"]).to_json } - end - - _data.must_equal ['{"id":1}'] - end - - class Create < Trailblazer::Operation step Policy::Guard ->(options) { options["user.current"] == ::Module } @@ -67,9 +48,6 @@ class Create < Trailblazer::Operation representer :serializer, Serializer representer :deserializer, Deserializer representer :errors, Serializer::Errors - # self["representer.serializer.class"] = Representer - # self["representer.deserializer.class"] = Deserializer - extend Contract::DSL contract do @@ -82,92 +60,135 @@ class Create < Trailblazer::Operation end end - step Model( Song, :new ) + step Model(Song, :new) step Contract::Build() - step Contract::Validate( representer: self["representer.deserializer.class"] ) - step Persist( method: :sync ) + step Contract::Validate(representer: self["representer.deserializer.class"]) step ->(options) { options["model"].id = 9 } end - let (:controller) { self } - let (:_data) { [] } - def head(*args); _data << [:head, *args] end - - let(:handlers) { Trailblazer::Endpoint::Handlers::Rails.new(self, path: "/songs").() } - def render(options) - _data << options - end - # not authenticated, 401 - it do - result = Create.( { id: 1 }, "user.current" => false ) - # puts "@@@@@ #{result.inspect}" - - Trailblazer::Endpoint.new.(result, handlers) - _data.inspect.must_equal %{[[:head, 401]]} - end - - # created - # length is ignored as it's not defined in the deserializer. - it do - result = Create.( {}, "user.current" => ::Module, "document" => '{"id": 9, "title": "Encores", "length": 999 }' ) - # puts "@@@@@ #{result.inspect}" - - Trailblazer::Endpoint.new.(result, handlers) - _data.inspect.must_equal '[[:head, 201, {:location=>"/songs/9"}]]' - end - - class Update < Create - self.~ Model( :find_by ) - end - - # 404 - it do - result = Update.({ id: nil }, "user.current" => ::Module, "document" => '{"id": 9, "title": "Encores", "length": 999 }' ) - - Trailblazer::Endpoint.new.(result, handlers) - _data.inspect.must_equal '[[:head, 404]]' - end - - #--- - # validation failure 422 - # success - it do - result = Create.({}, "user.current" => ::Module, "document" => '{ "title": "" }') - Trailblazer::Endpoint.new.(result, handlers) - _data.inspect.must_equal '[{:json=>"{\\"messages\\":{\\"title\\":[\\"must be filled\\"]}}", :status=>422}]' - end - - - include Trailblazer::Endpoint::Controller - #--- - # Controller#endpoint - # custom handler. - it do - invoked = nil - - endpoint(Update, { id: nil }) do |res| - res.not_found { invoked = "my not_found!" } + describe "default matchers" do + it "handles create" do + result = Create.( + {}, + "user.current" => ::Module, + "document" => '{"id": 9, "title": "Encores", "length": 999 }' + ) + response = Trailblazer::Endpoint.(result) + response[:data].to_json.must_equal({ id: 9 }.to_json) + response[:status].must_equal :created end - invoked.must_equal "my not_found!" - _data.must_equal [] # no rails code involved. - end - - # generic handler because user handler doesn't match. - it do - invoked = nil - - endpoint( Update, { id: nil }, args: {"user.current" => ::Module} ) do |res| - res.invalid { invoked = "my invalid!" } + it "handles success" do + result = Show.(id: 1) + response = Trailblazer::Endpoint.(result) + response[:data].to_json.must_equal({ id: 1 }.to_json) + response[:status].must_equal :ok end - _data.must_equal [[:head, 404]] - invoked.must_equal nil + it "handles unauthenticated" do + result = Create.( + {}, + "document" => '{"id": 9, "title": "Encores", "length": 999 }' + ) + response = Trailblazer::Endpoint.(result) + response[:data].to_json.must_equal({}.to_s) + response[:status].must_equal :unauthorized + end end - # only generic handler - it do - endpoint(Update, { id: nil }) - _data.must_equal [[:head, 404]] - end +# # if you pass in "present"=>true as a dependency, the Endpoint will understand it's a present cycle. +# it do +# Trailblazer::Endpoint.new.(Show.({ id: 1 }, { "present" => true }), my_handlers) +# _data.must_equal ['{"id":1}'] +# end +# +# # passing handlers directly to Endpoint#call. +# it do +# result = Show.({ id: 1 }, { "present" => true }) +# Trailblazer::Endpoint.new.(result) do |m| +# m.present { |result| _data << result["representer.serializer.class"].new(result["model"]).to_json } +# end +# +# _data.must_equal ['{"id":1}'] +# end +# +# let(:controller) { self } +# let(:_data) { [] } +# def head(*args) +# _data << [:head, *args] +# end +# +# let(:handlers) { Trailblazer::Endpoint::Handlers::Rails.new(self, path: "/songs").() } +# def render(options) +# _data << options +# end +# # not authenticated, 401 +# it do +# result = Create.({ id: 1 }, "user.current" => false) +# +# Trailblazer::Endpoint.new.(result, handlers) +# _data.inspect.must_equal %([[:head, 401]]) +# end +# +# # created +# # length is ignored as it's not defined in the deserializer. +# it do +# result = Create.({}, "user.current" => ::Module, "document" => '{"id": 9, "title": "Encores", "length": 999 }') +# +# Trailblazer::Endpoint.new.(result, handlers) +# _data.inspect.must_equal '[[:head, 201, {:location=>"/songs/9"}]]' +# end +# +# class Update < Create +# step Model(Song, :find_by) +# end +# +# # 404 +# it do +# result = Update.({ id: nil }, "user.current" => ::Module, "document" => '{"id": 9, "title": "Encores", "length": 999 }' ) +# +# Trailblazer::Endpoint.new.(result, handlers) +# _data.inspect.must_equal "[[:head, 404]]" +# end +# +# #--- +# # validation failure 422 +# # success +# it do +# result = Create.({}, "user.current" => ::Module, "document" => '{ "title": "" }') +# Trailblazer::Endpoint.new.(result, handlers) +# _data.inspect.must_equal '[{:json=>"{\\"messages\\":{\\"title\\":[\\"must be filled\\"]}}", :status=>422}]' +# end +# +# #--- +# # Controller#endpoint +# # custom handler. +# it do +# invoked = nil +# +# endpoint(Update, id: nil) do |res| +# res.not_found { invoked = "my not_found!" } +# end +# +# invoked.must_equal "my not_found!" +# _data.must_equal [] # no rails code involved. +# end +# +# # generic handler because user handler doesn't match. +# it do +# invoked = nil +# +# endpoint(Update, { id: nil }, args: {"user.current" => ::Module}) do |res| +# res.invalid { invoked = "my invalid!" } +# end +# +# _data.must_equal [[:head, 404]] +# invoked.must_equal nil +# end +# +# # only generic handler +# it do +# endpoint(Update, id: nil) +# _data.must_equal [[:head, 404]] +# end end diff --git a/test/test_helper.rb b/test/test_helper.rb index ada5c51..479ab0e 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,3 +1,3 @@ +$LOAD_PATH.unshift File.expand_path('../../lib', __FILE__) require "minitest/autorun" require "trailblazer" -require "trailblazer/endpoint" From b4506be9ef3f41c6b69ffa9ae45fad15eb4f8144 Mon Sep 17 00:00:00 2001 From: Rui Baltazar Date: Mon, 20 Nov 2017 15:06:18 +0800 Subject: [PATCH 05/11] WIP - added gitignore --- test/.gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 test/.gitignore diff --git a/test/.gitignore b/test/.gitignore new file mode 100644 index 0000000..36298d2 --- /dev/null +++ b/test/.gitignore @@ -0,0 +1 @@ +.byebug_history From 38df30646c9b25e6b3162116422b876f7922aabe Mon Sep 17 00:00:00 2001 From: Rui Baltazar Date: Mon, 20 Nov 2017 16:01:02 +0800 Subject: [PATCH 06/11] WIP - passing all endpoint sepcs --- .rubocop.yml | 2 +- lib/trailblazer/endpoint.rb | 13 ++- test/endpoint_test.rb | 196 ++++++++++++++++++------------------ 3 files changed, 107 insertions(+), 104 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 6bca04c..80c3361 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -8,7 +8,7 @@ Style/StringLiterals: - double_quotes Metrics/LineLength: - Max: 100 + Max: 80 Metrics/BlockLength: CountComments: false # count full line comments? diff --git a/lib/trailblazer/endpoint.rb b/lib/trailblazer/endpoint.rb index d3cc5d0..e31cdf3 100644 --- a/lib/trailblazer/endpoint.rb +++ b/lib/trailblazer/endpoint.rb @@ -28,21 +28,20 @@ class Endpoint } end }, - invalid: { - # rule: ->(result) { result.failure? && result["result.contract.default"]&.failure? }, - rule: ->(result) { result.failure? }, + not_found: { + rule: ->(result) { result.failure? && result["result.model"]&.failure? }, resolve: ->(_result, _representer) do { "data": {}, - "status": :unprocessable_entity + "status": :not_found } end }, - not_found: { - rule: ->(result) { result.failure? && result["result.model"]&.failure? }, + contract_failure: { + rule: ->(result) { result.failure? && result["result.contract.default"]&.failure? }, resolve: ->(result, _representer) do { - "data": { errors: result["result.model.errors"] }, + "data": { messages: result["result.contract.default"]&.errors&.messages }, "status": :unprocessable_entity } end diff --git a/test/endpoint_test.rb b/test/endpoint_test.rb index 6fbca51..da62e02 100644 --- a/test/endpoint_test.rb +++ b/test/endpoint_test.rb @@ -66,6 +66,10 @@ class Create < Trailblazer::Operation step ->(options) { options["model"].id = 9 } end + class Update < Trailblazer::Operation + step Model(Song, :find_by) + end + describe "default matchers" do it "handles create" do result = Create.( @@ -91,104 +95,104 @@ class Create < Trailblazer::Operation "document" => '{"id": 9, "title": "Encores", "length": 999 }' ) response = Trailblazer::Endpoint.(result) - response[:data].to_json.must_equal({}.to_s) + response[:data].to_s.must_equal({}.to_s) response[:status].must_equal :unauthorized end + + it "handles not found" do + result = Update.( + { id: nil }, + "user.current" => ::Module, + "document" => '{"id": 9, "title": "Encores", "length": 999 }' + ) + response = Trailblazer::Endpoint.(result) + response[:data].to_json.must_equal({}.to_s) + response[:status].must_equal :not_found + end + + it "handles broken contracts" do + result = Create.( + {}, + "user.current" => ::Module, + "document" => '{ "title": "" }' + ) + response = Trailblazer::Endpoint.(result) + response[:data].must_equal({ messages: { title: ["must be filled"]}}) + response[:status].must_equal :unprocessable_entity + end end -# # if you pass in "present"=>true as a dependency, the Endpoint will understand it's a present cycle. -# it do -# Trailblazer::Endpoint.new.(Show.({ id: 1 }, { "present" => true }), my_handlers) -# _data.must_equal ['{"id":1}'] -# end -# -# # passing handlers directly to Endpoint#call. -# it do -# result = Show.({ id: 1 }, { "present" => true }) -# Trailblazer::Endpoint.new.(result) do |m| -# m.present { |result| _data << result["representer.serializer.class"].new(result["model"]).to_json } -# end -# -# _data.must_equal ['{"id":1}'] -# end -# -# let(:controller) { self } -# let(:_data) { [] } -# def head(*args) -# _data << [:head, *args] -# end -# -# let(:handlers) { Trailblazer::Endpoint::Handlers::Rails.new(self, path: "/songs").() } -# def render(options) -# _data << options -# end -# # not authenticated, 401 -# it do -# result = Create.({ id: 1 }, "user.current" => false) -# -# Trailblazer::Endpoint.new.(result, handlers) -# _data.inspect.must_equal %([[:head, 401]]) -# end -# -# # created -# # length is ignored as it's not defined in the deserializer. -# it do -# result = Create.({}, "user.current" => ::Module, "document" => '{"id": 9, "title": "Encores", "length": 999 }') -# -# Trailblazer::Endpoint.new.(result, handlers) -# _data.inspect.must_equal '[[:head, 201, {:location=>"/songs/9"}]]' -# end -# -# class Update < Create -# step Model(Song, :find_by) -# end -# -# # 404 -# it do -# result = Update.({ id: nil }, "user.current" => ::Module, "document" => '{"id": 9, "title": "Encores", "length": 999 }' ) -# -# Trailblazer::Endpoint.new.(result, handlers) -# _data.inspect.must_equal "[[:head, 404]]" -# end -# -# #--- -# # validation failure 422 -# # success -# it do -# result = Create.({}, "user.current" => ::Module, "document" => '{ "title": "" }') -# Trailblazer::Endpoint.new.(result, handlers) -# _data.inspect.must_equal '[{:json=>"{\\"messages\\":{\\"title\\":[\\"must be filled\\"]}}", :status=>422}]' -# end -# -# #--- -# # Controller#endpoint -# # custom handler. -# it do -# invoked = nil -# -# endpoint(Update, id: nil) do |res| -# res.not_found { invoked = "my not_found!" } -# end -# -# invoked.must_equal "my not_found!" -# _data.must_equal [] # no rails code involved. -# end -# -# # generic handler because user handler doesn't match. -# it do -# invoked = nil -# -# endpoint(Update, { id: nil }, args: {"user.current" => ::Module}) do |res| -# res.invalid { invoked = "my invalid!" } -# end -# -# _data.must_equal [[:head, 404]] -# invoked.must_equal nil -# end -# -# # only generic handler -# it do -# endpoint(Update, id: nil) -# _data.must_equal [[:head, 404]] -# end + describe "overriding locally" do + # NOTE: Added cases will be evaluated before the defaults + # This allows creating special cases that would else be covered + # in a generic handler + it "allows adding new cases" do + result = Create.( + {}, + "user.current" => ::Module, + "document" => '{ "title": "" }' + ) + super_special = { + rule: ->(result) do + result.failure? && result["result.contract.default"]&.failure? && result["result.contract.default"]&.errors&.messages.include?(:title) + end, + resolve: ->(_result, _representer) do + { "data": { messages: ["status 200, ok but!"] }, "status": :ok } + end + } + response = Trailblazer::Endpoint.(result, nil, super_special: super_special) + response[:data].must_equal(messages: ["status 200, ok but!"]) + response[:status].must_equal :ok + end + + it "allows re-writing the rule for existing matcher" do + result = Create.( + {}, + "user.current" => ::Module, + "document" => '{ "title": "" }' + ) + not_found_rule = ->(result) do + result.failure? && result["result.contract.default"]&.failure? && result["result.contract.default"]&.errors&.messages.include?(:title) + end + response = Trailblazer::Endpoint.(result, nil, not_found: { rule: not_found_rule } ) + response[:data].must_equal({}) + response[:status].must_equal :not_found + end + + it "allows re-writing the resolve for existing matcher" do + result = Create.( + {}, + "user.current" => ::Module, + "document" => '{ "title": "" }' + ) + contract_resolve = ->(result, _representer) do + { + "data": { messages: result["result.contract.default"]&.errors&.messages }, + "status": :bad_request + } + end + response = Trailblazer::Endpoint.(result, nil, contract_failure: { resolve: contract_resolve } ) + response[:data].must_equal({ messages: { title: ["must be filled"]}}) + response[:status].must_equal :bad_request + end + + it "allows re-writing the whole matcher" do + result = Create.( + {}, + "user.current" => ::Module, + "document" => '{ "title": "" }' + ) + new_contract_failure = { + rule: ->(result) do + result.failure? + end, + resolve: ->(_result, _representer) do + { "data": { messages: ["status 200, ok but!"] }, "status": :ok } + end + } + response = Trailblazer::Endpoint.(result, nil, contract_failure: new_contract_failure) + response[:data].must_equal(messages: ["status 200, ok but!"]) + response[:status].must_equal :ok + end + end end From add981ae7f262828434efb11d08bf97851ee4a37 Mon Sep 17 00:00:00 2001 From: Rui Baltazar Date: Mon, 20 Nov 2017 16:01:25 +0800 Subject: [PATCH 07/11] WIP - removed byebug require --- test/endpoint_test.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/endpoint_test.rb b/test/endpoint_test.rb index da62e02..b6a25a1 100644 --- a/test/endpoint_test.rb +++ b/test/endpoint_test.rb @@ -6,8 +6,6 @@ require "trailblazer/endpoint" require "trailblazer/endpoint/rails" -require "byebug" - class EndpointTest < Minitest::Spec # NOTE: Consider moving all this code to a separate class as # it is relevant for the test but it is boilerplate for testing From 4e05da179c86fe511cd230c6b2828fcfdc5a1001 Mon Sep 17 00:00:00 2001 From: Rui Baltazar Date: Mon, 20 Nov 2017 17:39:51 +0800 Subject: [PATCH 08/11] WIP - updated readme to have some description of how it works --- README.md | 144 ++++++++++++++++++++++++++++++++++++++++++ test/endpoint_test.rb | 2 +- 2 files changed, 145 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 5d568f5..53f185c 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,150 @@ Decouple finding out *what happened* from *what to do*. +## Motivation + +Trailblazer brings a lot of clarity to your controller logic, +pushing you to create operations that have a clear workflow. Each operation +returns a result object with enough information in it to evaluate what to do. +The problem now lies on the code duplication that one is forced to write to +evaluate a set of possible cases, generally solved the same way for each +controller. + +E.g. an unauthenticated request should always be resolved in the same way +(exclude special cases) + +From this idea, Endpoint gem came to life. Wrapping some of the most common +cases with a common solution for them. This allows you to don't have to worry +with the returning values of each of your operations. + +Naturally, not everyone has common cases and, in the light of Trailblazer +flexibility, you can override all the behavior at any level of the app. + +## Usage + +### If your operation does not have a representer specified + +Consider the following controller + +```ruby +class Api::V1::MyMagicController < Api::V1::BaseController + def index + result = V1::MyMagic::Index.(params, current_user: current_user) + response = Trailblazer::Endpoint.(result, V1::MyMagic::Representer::Index) + render json: response[:data], status: response[:status] + end +end +``` + +As you can see, the controller calls your operation that will return a result +object. This object is then passed to `Trailblazer::Endpoint` that will inspect +the result object and decide what to put in the response. Typically, response +object will have a param `data` and a `status`. + +E.g. if the operation is `successful`, then `data` will have the representation +of the `model` while `status` will have `:ok`. + +### If your operation has a representer specified + +Consider the following controller + +```ruby +class Api::V1::MyMagicController < Api::V1::BaseController + def index + result = V1::MyMagic::Index.(params, current_user: current_user) + response = Trailblazer::Endpoint.(result) + render json: response[:data], status: response[:status] + end +end +``` + +The main difference is that Endpoint during the inspection will fetch +the representer class automatically. This way you don't need to pass a +representer to the Endpoint. All the remaining logic is still valid. + +## How to override the default matchers? + +As promised in the motivation, if you feel the need to override a specific +matcher, you can do so both globally (good for when your whole application +needs this logic) or locally (good if a specific endpoint is expected to behave +differently). + + +Consider the following controller + +```ruby +class Api::V1::MyMagicController < Api::V1::BaseController + def index + op_res = V1::MyMagic::Index.(params, current_user: current_user) + success_proc = ->(result, _representer) do + { "data": result["response"], "status": :ok } + end + response = Trailblazer::Endpoint.(op_res, nil, success: { resolve: success_proc }) + render json: response[:data], status: response[:status] + end +end +``` + +In this particular case, the default behavior for a successful operation is not +what we want to use. So we can write our own resolution and pass it as part +of the overrides. Because we pass it to the endpoint as the `resolve` proc for +the matcher `success`, the existing `success` rule will still be evaluated and +in case it returns true, our `success_proc` will be invoked instead. + +Likewise we can override the matching logic. + +Consider the following controller + +```ruby +class Api::V1::MyMagicController < Api::V1::BaseController + def index + op_res = V1::MyMagic::Index.(params, current_user: current_user) + success_proc = ->(result) { result.success? && result['models'].count > 0 } + response = Trailblazer::Endpoint.(op_res, nil, success: { rule: success_proc }) + render json: response[:data], status: response[:status] + end +end +``` + +You can easily understand that we are now overriding the rule proc with one of +our custom made rules. The resolution would still be the default one. + +## Completely custom solution + +As you can imagine we can't think of all possible solutions for every single +use case. So this gem still gives you the flexibility of creating your own +matchers and resolutions. Any custom matcher will be evaluated before the +default ones. If you provide a custom matcher with an existing name, you'll be +overriding the whole matcher with your own solution. + +Consider the following controller + +```ruby +class Api::V1::MyMagicController < Api::V1::BaseController + def index + op_res = V1::MyMagic::Index.(params, current_user: current_user) + success = { success: { + rule: ->(result) { result.success? && result['models'].count > 0 }, + resolve: ->(result, _representer) { { data: "more than 0", status: :ok } } + } + } + super_special = { + super_special: { + rule: ->(result) { result.success? && result['models'].count > 100 }, + resolve: ->(result, _representer) { { data: "more than 100", status: :ok } } + } + } + response = Trailblazer::Endpoint.(op_res, nil, { super_special: super_special, success: success } ) + render json: response[:data], status: response[:status] + end +end +``` + +In this more complex example, we are creating a custom `super_special` matcher +and overriding the `success`. Please note that the order is important and as +mentioned before, custom matchers will be evaluated before the default ones. +This applies for the overridden ones as well. + t test/controllers/songs_controller_test.rb --backtrace ## TODO diff --git a/test/endpoint_test.rb b/test/endpoint_test.rb index b6a25a1..4427c15 100644 --- a/test/endpoint_test.rb +++ b/test/endpoint_test.rb @@ -93,7 +93,7 @@ class Update < Trailblazer::Operation "document" => '{"id": 9, "title": "Encores", "length": 999 }' ) response = Trailblazer::Endpoint.(result) - response[:data].to_s.must_equal({}.to_s) + response[:data].must_equal({}) response[:status].must_equal :unauthorized end From 35a2bebc302447ccf8a4dc046d195676355e32b7 Mon Sep 17 00:00:00 2001 From: Rui Baltazar Date: Mon, 27 Nov 2017 19:58:02 +0800 Subject: [PATCH 09/11] WIP - added universal matcher --- lib/trailblazer/endpoint.rb | 42 +++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/lib/trailblazer/endpoint.rb b/lib/trailblazer/endpoint.rb index e31cdf3..834b2c7 100644 --- a/lib/trailblazer/endpoint.rb +++ b/lib/trailblazer/endpoint.rb @@ -3,7 +3,7 @@ class Endpoint DEFAULT_MATCHERS = { created: { rule: ->(result) { result.success? && result["model.action"] == :new }, - resolve: ->(result, representer) do + resolve: lambda do |result, representer| { "data": representer.new(result["model"]), "status": :created @@ -12,7 +12,7 @@ class Endpoint }, success: { rule: ->(result) { result.success? }, - resolve: ->(result, representer) do + resolve: lambda do |result, representer| { "data": representer.new(result["model"]), "status": :ok @@ -20,8 +20,10 @@ class Endpoint end }, unauthenticated: { - rule: ->(result) { result.failure? && result["result.policy.default"]&.failure? }, - resolve: ->(_result, _representer) do + rule: lambda do |result| + result.failure? && result["result.policy.default"]&.failure? + end, + resolve: lambda do |_result, _representer| { "data": {}, "status": :unauthorized @@ -29,8 +31,10 @@ class Endpoint end }, not_found: { - rule: ->(result) { result.failure? && result["result.model"]&.failure? }, - resolve: ->(_result, _representer) do + rule: lambda do |result| + result.failure? && result["result.model"]&.failure? + end, + resolve: lambda do |_result, _representer| { "data": {}, "status": :not_found @@ -38,10 +42,25 @@ class Endpoint end }, contract_failure: { - rule: ->(result) { result.failure? && result["result.contract.default"]&.failure? }, - resolve: ->(result, _representer) do + rule: lambda do |result| + result.failure? && result["result.contract.default"]&.failure? + end, + resolve: lambda do |result, _representer| { - "data": { messages: result["result.contract.default"]&.errors&.messages }, + "data": { + messages: result["result.contract.default"]&.errors&.messages + }, + "status": :unprocessable_entity + } + end + }, + fallback: { + rule: ->(_result) { true }, + resolve: lambda do |_result, _representer| + { + "data": { + messages: ["Unexpected operation result"] + }, "status": :unprocessable_entity } end @@ -52,7 +71,6 @@ class Endpoint # it might have a representer, else will assume the default name def self.call(operation_result, representer_class = nil, overrides = {}) representer = operation_result["representer.serializer.class"] || representer_class - # TODO: What to do when nothing matches? endpoint_opts = { result: operation_result, representer: representer } new.(endpoint_opts, overrides) end @@ -66,7 +84,9 @@ def call(options, overrides) next end - return resolve.(options[:result], options[:representer]) if rule.(options[:result]) + if rule.(options[:result]) + return resolve.(options[:result], options[:representer]) + end end matching_rules(overrides).each do |_rule_key, rule_description| if rule_description[:rule].(options[:result]) From db464f0d804a0a35565831f4abd5c80c1be54ed4 Mon Sep 17 00:00:00 2001 From: Rui Baltazar Date: Tue, 6 Aug 2019 12:19:26 +0800 Subject: [PATCH 10/11] WIP - overwrite the old endpoint with latest production version --- lib/trailblazer/endpoint.rb | 80 +++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 44 deletions(-) diff --git a/lib/trailblazer/endpoint.rb b/lib/trailblazer/endpoint.rb index 834b2c7..4ea6915 100644 --- a/lib/trailblazer/endpoint.rb +++ b/lib/trailblazer/endpoint.rb @@ -4,52 +4,50 @@ class Endpoint created: { rule: ->(result) { result.success? && result["model.action"] == :new }, resolve: lambda do |result, representer| - { - "data": representer.new(result["model"]), - "status": :created - } + { "data": representer.new(result[:model]), "status": :created } + end + }, + deleted: { + rule: ->(result) { result.success? && result["model.action"] == :destroy }, + resolve: lambda do |result, _representer| + { "data": { id: result[:model].id }, "status": :ok } + end + }, + found: { + rule: ->(result) { result.success? && result["model.action"] == :find_by }, + resolve: lambda do |result, representer| + { "data": representer.new(result[:model]), "status": :ok } end }, success: { rule: ->(result) { result.success? }, resolve: lambda do |result, representer| - { - "data": representer.new(result["model"]), - "status": :ok - } + data = if representer + representer.new(result[:results]) + else + result[:results] + end + { "data": data, "status": :ok } end }, unauthenticated: { - rule: lambda do |result| - result.failure? && result["result.policy.default"]&.failure? - end, - resolve: lambda do |_result, _representer| - { - "data": {}, - "status": :unauthorized - } - end + rule: ->(result) { result.policy_error? }, + resolve: ->(_result, _representer) { { "data": {}, "status": :unauthorized } } }, not_found: { - rule: lambda do |result| - result.failure? && result["result.model"]&.failure? - end, - resolve: lambda do |_result, _representer| + rule: ->(result) { result.failure? && result["result.model"]&.failure? }, + resolve: lambda do |result, _representer| { - "data": {}, - "status": :not_found + "data": { errors: result["result.model.errors"] }, + "status": :unprocessable_entity } end }, - contract_failure: { - rule: lambda do |result| - result.failure? && result["result.contract.default"]&.failure? - end, + invalid: { + rule: ->(result) { result.failure? }, resolve: lambda do |result, _representer| { - "data": { - messages: result["result.contract.default"]&.errors&.messages - }, + "data": { errors: result.errors || result[:errors] }, "status": :unprocessable_entity } end @@ -57,21 +55,16 @@ class Endpoint fallback: { rule: ->(_result) { true }, resolve: lambda do |_result, _representer| - { - "data": { - messages: ["Unexpected operation result"] - }, - "status": :unprocessable_entity - } + { "data": { errors: "Can't process the result" }, + "status": :unprocessable_entity } end } - } + }.freeze - # options expects a TRB Operation result + # NOTE: options expects a TRB Operation result # it might have a representer, else will assume the default name def self.call(operation_result, representer_class = nil, overrides = {}) - representer = operation_result["representer.serializer.class"] || representer_class - endpoint_opts = { result: operation_result, representer: representer } + endpoint_opts = { result: operation_result, representer: representer_class } new.(endpoint_opts, overrides) end @@ -79,14 +72,13 @@ def call(options, overrides) overrides.each do |rule_key, rule_description| rule = rule_description[:rule] || DEFAULT_MATCHERS[rule_key][:rule] resolve = rule_description[:resolve] || DEFAULT_MATCHERS[rule_key][:resolve] + if rule.nil? || resolve.nil? puts "Matcher is not properly set. #{rule_key} will be ignored" next end - if rule.(options[:result]) - return resolve.(options[:result], options[:representer]) - end + return resolve.(options[:result], options[:representer]) if rule.(options[:result]) end matching_rules(overrides).each do |_rule_key, rule_description| if rule_description[:rule].(options[:result]) @@ -96,7 +88,7 @@ def call(options, overrides) end def matching_rules(overrides) - DEFAULT_MATCHERS.reject { |k, _v| overrides.keys.include? k } + DEFAULT_MATCHERS.except(*overrides.keys) end end end From dcdd71c98ebf46db3103b609841ccd82437657b0 Mon Sep 17 00:00:00 2001 From: Rui Baltazar Date: Tue, 6 Aug 2019 12:24:16 +0800 Subject: [PATCH 11/11] WIP - added result extender --- lib/trailblazer/endpoint.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/lib/trailblazer/endpoint.rb b/lib/trailblazer/endpoint.rb index 4ea6915..3320cc2 100644 --- a/lib/trailblazer/endpoint.rb +++ b/lib/trailblazer/endpoint.rb @@ -91,4 +91,28 @@ def matching_rules(overrides) DEFAULT_MATCHERS.except(*overrides.keys) end end + + class Operation + class Result + def errors + return self["result.policy.failure"] if policy_error? + return self["contract.default"].errors.full_messages if contract_error? + return self["result.model.errors"] if model_error? + + self[:errors] + end + + def policy_error? + self["result.policy.failure"].present? + end + + def model_error? + self["result.model.errors"].present? + end + + def contract_error? + self["contract.default"].present? + end + end + end end