trailblazer-operation icon indicating copy to clipboard operation
trailblazer-operation copied to clipboard

Binary state methods for custom outputs.

Open elShiaLabeouf opened this issue 1 year ago • 12 comments

Hello @apotonick and the Trailblazer team!

I think Adding outputs is a really powerful feature since it enables handling multiple outcomes during the operation run.

I also enjoy using operation.success? and operation.failure? methods that come in handy in controller actions routing.

But when I'm using a custom output in an operation, to check which pipeline the operation went, I have to do the following:

operation = Payment::Operation::Create.(provider: "bla-unknown")
if operation.event.to_h[:semantic] == :provider_invalid
  render "error"
else
  render "success"
end  

I think it would be more intuitive and kinda elegant if instead of operation.event.to_h[:semantic] == :provider_invalid I could use a binary state method operation.provider_invalid?

I'd like to come up with a proposal to add such methods to a number of classes/objects like #<Trailblazer::Activity::Railway::End::Success semantic=:success>, #<Trailblazer::Activity::End semantic=:paypal> etc..

I have tested my changes locally and it works like this:

require 'trailblazer/operation'

class Execute < Trailblazer::Operation
  UsePaypal = Class.new(Trailblazer::Activity::Signal)

  step :find_provider, Output(UsePaypal, :paypal) => End(:paypal)
  step :charge_creditcard

  def find_provider(ctx, params:, **)
    return true unless params[:provider] == :paypal
    UsePaypal
  end

  def charge_creditcard(ctx, **)
    ctx[:charged] = true
  end
end

op = Execute.call({ params: { provider: :paypal } })
op.paypal?  # => true
op.success? # => false
op.failure? # => false

op = Execute.call({ params: { provider: :not_paypal } })
op.paypal?  # => false
op.success? # => true
op.failure? # => false


The changes

To achieve this I had to add bits of code to three gems:

  1. First, I added success? failure? fail_fast? pass_fast? and <custom_output>? to Trailblazer::Activity::End:
# trailblazer-activity-0.16.0/lib/trailblazer/activity/structures.rb
module Trailblazer
  class Activity
    class End
      def initialize(semantic:, **options)
        @options = options.merge(semantic: semantic)

        define_singleton_method("#{semantic}?") { true } # <-- this
        %i[success failure fail_fast pass_fast].each do |name| # <-- this
          define_singleton_method("#{name}?") { name == semantic } # <-- this
        end  # <-- this
      end

  1. Then I added the new custom output binary state method to End::Success, End::Failure objects in Trailblazer::Activity::DSL::Linear::Normalizer::OutputTuples#register_additional_outputs (which is called every time a new custom output is defined):
# trailblazer-activity-dsl-linear-1.2.3/lib/trailblazer/activity/dsl/linear/normalizer/output_tuples.rb
            def register_additional_outputs(ctx, output_tuples:, outputs:, **)
              # this below
              default_ouputs = %i[success failure fail_fast pass_fast]
              railway_ends = ctx[:sequence].find_all { |(semantic, railway_end, *)| default_ouputs.include?(semantic) }

              output_tuples.each do |(output, connector)|
                railway_ends.each do |(semantic, railway_end, *)|
                  railway_end.define_singleton_method("#{output.semantic}?") { output.semantic == semantic }
                end
              end
              # ---
              output_tuples =
                output_tuples.collect do |(output, connector)|
              ...
  1. and finally changed the constructor, success?, failure? and added method_missing for delegation in Trailblazer::Operation::Result:
# trailblazer-operation/lib/trailblazer/operation/result.rb

class Trailblazer::Operation
  class Result
    def initialize(event, data)
      @event, @data = event, data
    end

    def success?
      @event.to_h[:semantic] == :success
    end

    def failure?
      @event.to_h[:semantic] == :failure
    end

    private def method_missing(symbol, *args)
      if @event.respond_to?(symbol)
        @event.send(symbol, *args)
      else
        super
      end
    end

after that I tested the code with the example above and got those result.


I understand that the code I propose to add might seem complicated, but on the other hand in return we get:

  • a totally new feature of binary state methods in trailblazer-activity.
  • new custom outputs' binary state methods as an addition to the good old success? and failure? in trailblazer-operation.

The code above can be added to the Trailblazer family step by step, PR by PR and it won't break a thing. I'm willing to create this series of PRs.

Looking forward to hearing your thoughts on my proposal!

elShiaLabeouf avatar Jul 01 '23 18:07 elShiaLabeouf

Hi @elShiaLabeouf, wow, you have lots of ideas, I like that! :green_heart:

Regarding your specific requirement, my suggestion would be to enhance Operation::Result, which is the actual object exposing #success and #failure. There is no need to change deeper gems (in my understanding), or am I wrong here?

One important thing to keep in mind is that calling operations is a concept that will happen in an endpoint in future versions of TRB - meaning that methods like success? will not be used on the user side. You configure your endpoints with a higher level API. This is the reason that only Operation has the binary result methods, these concepts don't exist in activity and dsl.

apotonick avatar Jul 02 '23 09:07 apotonick

Oh BTW, one more thing! An Output is not a terminus (or "end" as we used to call it). An output defines which outgoing connection goes where, and that might lead to a specific terminus, however, sometimes this might just define what will be the next step, so be careful not to confuse those two concepts.

I will soon explain more about the Wiring API in a Trailblazer Tales episode. :beers:

apotonick avatar Jul 02 '23 09:07 apotonick

Hey @elShiaLabeouf feel free to hit me up directly on https://trailblazer.zulipchat.com to discuss :point_up:

apotonick avatar Jul 05 '23 09:07 apotonick

Oh, I see.

I've come up with this minimalistic PR. Please take a look.

I didn't realize the outputs were accessible from the operation class itself.

elShiaLabeouf avatar Jul 05 '23 22:07 elShiaLabeouf

That was quite a bit of overengineering before 😂

elShiaLabeouf avatar Jul 05 '23 22:07 elShiaLabeouf

Any update on this? I think it would be nice to check easier for different end semantics. Maybe just a one new method?

operation = Payment::Operation::Create.(provider: "bla-unknown")
if operation.success?
  render "success"
elsif operation.semantic == :provider_invalid # or operation.semantic?(:provider_invalid)
  render "provider_error"
else
  render "error"
end  

eliten00b avatar Jun 25 '24 14:06 eliten00b

We're putting this into the endpoint gem - I am working on it as I type, actually! Will update you in the next days! :green_heart:

apotonick avatar Jun 25 '24 14:06 apotonick

@eliten00b @elShiaLabeouf Here's a rough outlook of how I envision the interface: https://github.com/trailblazer/trailblazer-endpoint/blob/master/test/matcher_test.rb#L17 - it will look slightly less clumsy in a controller. Any comments here? :beers:

apotonick avatar Jun 25 '24 15:06 apotonick

Does this mean any custom terminus is not consider a failure anymore?

eliten00b avatar Jun 25 '24 16:06 eliten00b

@eliten00b Interesting! A custom output has never been a "failure" per definition, that's why we gave it an attribute "semantic". It's up to the user to decide what "not_found" means. What makes you think that this is now different with the above example?

apotonick avatar Jun 26 '24 07:06 apotonick

True, I never read that a custom terminus is consider a failure.

Most of our operation use the run Operation do; end in rails controllers. When we now started with custom terminus it skips the block. Which is ok, as it is not the ideal success or pass_fast semantic.

When I check Operation.().failure? it is true if the terminus is a custom one.

From your code it reads that at least the DSL#failure does not check the same as the Operation#failure? method. Which does sound ok to me, if the check become more fine.

From the initial code:

op = Execute.call({ params: { provider: :paypal } })
op.paypal?  # => true
op.success? # => false
op.failure? # => false

currently it would be:

op = Execute.call({ params: { provider: :paypal } })
# => <Trailblazer::Operation::Railway::Result:false>
op.event.to_h[:semantic] == :paypal # => true
op.success? # => false
op.failure? # => true

eliten00b avatar Jun 26 '24 09:06 eliten00b

@eliten00b Good point! Let me finalize my code over here and then we can discuss the API for endpoint. :coffee:

apotonick avatar Jun 26 '24 12:06 apotonick