Support abstract interfaces instead of specific ActiveRecord objects in Formatters

[supremebeing7]

This is in reference to v0.6.0.

I use active_interaction for services in lots of my projects, and recently started using this gem to enforce JSONAPI spec more easily.

When I have an ActiveInteraction instance - interaction - and pass it into jsonapi_render_errors json: interaction in my controller, I get TypeError: no implicit conversion of Symbol into Integer because it tries to sanitize the errors. However, ActiveInteraction's errors follows the same interface as ActiveRecord's errors. The first line of JSONAPI::Utils::Response::Formatters#jsonapi_format_errors would work for ActiveInteraction, except that it's explicitly checking if data is an active_record_obj?, which of course it isn't.

Would it be possible to make this not rely on whether it's an ActiveRecord object, but instead just check that it responds to the necessary methods?

[supremebeing7]

I'd be glad to try to make this change, but wanted to make sure this was something you'd support first. I also may need some guidance as I go. If you're good with the idea, I can put some time into a plan of action before starting any real code.

[tiagopog]

Well, for this feature in specific I think we'd rather to avoid another duck typing since all error objects are already expected to quack like the #errors method. Thus, to diff the AR or equivalent objects from other ones – in order to use the helper sugar and pass only the object itself (jsonapi_render_errors @user) – could be quite tricky and maybe lead to unexpected behavior.

I see the utility of duck typing when we really need to call the method(s) in question, otherwise I feel more secure to check for the object's class. But anyway, I'm totally open for ideias :-)

For now, another way for achieving a proper rendering should be by creating your own error class:

class MyCustomError < ::JSONAPI::Exceptions::Error
  attr_accessor :object

  def initialize(object)
    @object = object
  end

  def errors
    [JSONAPI::Error.new(
      code: '125',
      status: :unprocessable_entity,
      id: 'my_custom_validation_error',
      title: 'My custom error message'
    )]
  end
end

Then you should pass the object like so:

jsonapi_render_errors MyCustomError.new(my_failing_object)

You can also inherit from JSONAPI::Utils::Exceptions::ActiveRecord to apply any particular implementation or instantiate an error object for that class:

Exceptions = JSONAPI::Utils::Exceptions

def create
  # ...
  jsonapi_render_errors Exceptions::ActiveRecord.new(my_failing_object)
end

Thoughts?

[supremebeing7]

I understand the concern about possible unintended behavior. I'll tinker with the alternatives you mentioned and see if I can make those work. Thanks.

…

On Jun 22, 2017 9:53 PM, "Tiago Guedes" ***@***.***> wrote: Well, for this feature in specific I think we'd rather to avoid another duck typing since all error objects are already expected to quack the #errors method. Thus, to diff the AR or equivalent objects from other ones – in order to use the helper sugar and pass only the object itself (jsonapi_render_errors @user) – could be quite tricky and maybe lead to unexpected behavior. I see the utility of duck typing when we really need to call the method(s) in question, otherwise I feel more secure to check for the object's class. Anyway, I'm totally open for ideias :-) For now, another way for achieving a proper rendering should be by creating your own error class: class MyCustomError < ::JSONAPI::Exceptions::Error attr_accessor :object def initialize(object) @object = object end def errors [JSONAPI::Error.new( code: '125', status: :unprocessable_entity, id: 'my_custom_validation_error', title: 'My custom error message' )] endend Then you could pass the object like so: jsonapi_render_errors MyCustomError.new(my_failing_object) You can even inherit from JSONAPI::Utils::Exceptions::ActiveRecord to apply any particular implementation or instantiate an error object for that class: Exceptions = JSONAPI::Utils::Exceptions jsonapi_render_errors Exceptions::ActiveRecord.new(my_failing_object) Thoughts? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#66 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGbYDIPZvpBagcKvaO_vxST-x5WAXGeTks5sG0TdgaJpZM4OBqSP> .

[tiagopog]

Thanks for the feedback, @supremebeing7. It seems to me that there's a lack of documentation for this kind of use case at README.md, then I'll keep this issue open until it gets better examples for rendering error objects.

[supremebeing7]

OK, so this does work:

jsonapi_render_errors json: JSONAPI::Utils::Exceptions::ActiveRecord.new(interaction, @request.resource_klass, context)

The unfortunate thing is that we use these interaction objects all over our code, so we'd have to do this everywhere. I can write a helper method to do it, but it's still extra mental gymnastics for me or others on this project to remember to call it that way. The same would be the case if using the other approach (custom exception class) you mentioned.

Anyway, I'll keep poking at this and post here if I can figure out a better way. Thanks for the guidance.

[supremebeing7]

So far, the only way I've been able to get around this is just by reopening the module and overwriting the jsonapi_format_errors method:

module JSONAPI
  module Utils
    module Response
      module Formatters
        alias_method :old_jsonapi_format_errors, :jsonapi_format_errors

        def jsonapi_format_errors(data)
          if active_interaction_object?(data)
            data = JSONAPI::Utils::Exceptions::ActiveRecord.new(
              data,
              @request.resource_klass,
              context
            )
          end
          old_jsonapi_format_errors(data)
        end

        def active_interaction_object?(data)
          defined?(ActiveInteraction::Base) && data.is_a?(ActiveInteraction::Base)
        end
      end
    end
  end
end

not ideal, because it muddies our upgrade path when this gem is updated, but it works and it doesn't require any additional code in the controllers.

Beyond adding code to the gem to work with another Exception like this, I don't really see a better way. So, I suppose we can close this one out, unless you think there's some more abstract changes we could make that would be helpful to others.

[tiagopog]

Actually, with your last comment in mind, there's something we could work around: what about to configure (white list) the error classes the helper should work by default?

For example, having such config:

JSONAPI.configure do |config|
  config.default_error_formatters = [ActiveRecord::Base, ActiveInteraction::Base]
end

We could apply the following check:

def use_default_error_formatter?(object)
  JSONAPI.configuration.default_error_formatters.map { |klass| defined?(klass) && object.is_a?(klass) }.reduce(:|)
end

Then within the formatter:

def jsonapi_format_errors(data)
  if use_default_error_formatter?(data)
    #...
  end
end

What do you think? Would you be interested to bring this feature to the gem?

[supremebeing7]

Yeah I think that would work really well. Thanks for the discussion and idea.

I'll try to get something pushed and make a pull request over the weekend.

[supremebeing7]

Finally got around to picking at this. I made some progress, but am running into an issue with

JSONAPI.configure do |config|
  config.default_error_formatters = [ActiveRecord::Base, ActiveInteraction::Base]
end

because that config option is not available on JSONAPI.

I can create a configuration class in this gem but I don't like the idea of having two different configurations to keep track of - one for JSONAPI and one for JSONAPI::Utils. I could PR to that gem but it seems odd to make changes there to solve a problem I'm facing here.

@tiagopog Do you have any thoughts?