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

Binary state methods for custom outputs. #63

Open
elShiaLabeouf opened this issue Jul 1, 2023 · 12 comments
Open

Binary state methods for custom outputs. #63

elShiaLabeouf opened this issue Jul 1, 2023 · 12 comments

Comments

@elShiaLabeouf
Copy link

elShiaLabeouf commented Jul 1, 2023

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!

@apotonick
Copy link
Member

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

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
Copy link
Member

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. 🍻

@apotonick
Copy link
Member

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

@elShiaLabeouf
Copy link
Author

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
Copy link
Author

That was quite a bit of overengineering before 😂

@eliten00b
Copy link

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  

@apotonick
Copy link
Member

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

@apotonick
Copy link
Member

@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? 🍻

@eliten00b
Copy link

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

@apotonick
Copy link
Member

@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?

@eliten00b
Copy link

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

@apotonick
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants