Extractor configurability

[sandstrom]

Is there an existing issue for this?

• [X] I have searched the existing issues

Is your feature request related to a problem? Please describe

Association extractor is useful to modify to e.g. add a circuit-breaker, implement caching and for a few other things. Right now it's somewhat complicated to override/modify.

Describe the feature you'd like to see implemented

Right now, there are two types of extractors: AssociationExtractor and AutoExtractor (along with some sub-extractors).

Their roles are somewhat intertwined (AssociationExtractor calls the AutoExtractor, for example), and only one of them can be modified globally.

I'd propose a stricter separation, splitting it up into (probably) three extractors:

• FieldExtractor
• AssociationExtractor
• ValueExtractor (or DataExtractor)

The default ValueExtractor would encapsulate the logic currently existing in AutoExtractor, and I'd move the logic of deferring to block to field/association extractors. ValueExtractor could also be named something like DataExtractor instead.

Example

This is a rough sketch, in reality it would probably be slightly different.

class ValueExtractor < Extractor
  def extract(field_name, object, local_options, options = {})
    if object.is_a?(Hash)
      value = extract_hash(field_name, object, local_options, options)
    else
      value = extract_send(field_name, object, local_options, options)
    end
  end

  private

  def extract_hash(field_name, object, _local_options, _options = {})
    object[field_name]
  end

  def extract_send(field_name, object, _local_options, _options = {})
    object.public_send(field_name)
  end
end

class FieldExtractor < Extractor
  include EmptyTypes

  def initialize
    @extractor = Blueprinter.configuration.field_extractor_default.new # `ValueExtractor.new` by default
  end

  def extract(field_name, object, local_options, options = {})
    if options[:block]
      value = options[:block].call(object, local_options)
    else
      value = @value_extractor.extract(field_name, object, local_options, options)
    end

    value = @datetime_formatter.format(value, options)
    use_default_value?(value, options[:default_if]) ? default_value(options) : value
  end

  private

  def default_value(field_options)
    field_options.key?(:default) ? field_options.fetch(:default) : Blueprinter.configuration.field_default
  end
end

class AssociationExtractor < Extractor
  include EmptyTypes

  def initialize
    @extractor = Blueprinter.configuration.field_extractor_default.new # `ValueExtractor.new` by default
  end

  def extract(association_name, object, local_options, options = {})
    options_without_default = options.except(:default, :default_if)

    # Merge in assocation options hash
    local_options = local_options.merge(options[:options]) if options[:options].is_a?(Hash)

    if options[:block]
      value = options[:block].call(object, local_options)
    else
      value = @value_extractor.extract(association_name, object, local_options, options)
    end

    return default_value(options) if use_default_value?(value, options[:default_if])

    view = options[:view] || :default
    blueprint = association_blueprint(options[:blueprint], value)
    blueprint.prepare(value, view_name: view, local_options: local_options)
  end

  private

  def default_value(association_options)
    return association_options.fetch(:default) if association_options.key?(:default)

    Blueprinter.configuration.association_default
  end

  def association_blueprint(blueprint, value)
    blueprint.is_a?(Proc) ? blueprint.call(value) : blueprint
  end
end

Describe alternatives you've considered

No response

Additional context

No response

[sandstrom]

Happy to discuss this in more detail, and elaborate more.

[sandstrom]

Friendly ping @lessthanjacob @njbbaer @ritikesh

If you think this is a reasonable suggestion, let me know! If so, I'd propose these steps:

• I'll flesh out the proposed change in some more detail (written, in this issue)
• You get another chance to provide more feedback, and if you're happy we'll open a PR

For reference, there are also a few other issues that I've opened, where I'd also be happy to get your input.

Full list

• https://github.com/procore-oss/blueprinter/issues/403
• https://github.com/procore-oss/blueprinter/issues/404
• https://github.com/procore-oss/blueprinter/issues/405
• https://github.com/procore-oss/blueprinter/issues/406
• https://github.com/procore-oss/blueprinter/issues/407

[lessthanjacob]

I think this would be a reasonable refactor.

@njbbaer @ritikesh any thoughts/concerns here?

[ritikesh]

Agreed, we are open to contributions @sandstrom.

[sandstrom]

@lessthanjacob @ritikesh Thanks!

I'll have a chat with a colleague of mine, and we'll try to get back with a proposal.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[sandstrom]

Still relevant!

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[sandstrom]

Not stale