balvig / spyke

Interact with REST services in an ActiveRecord-like manner
MIT License
901 stars 66 forks source link

Ability to add custom transformation/unpacking of embedded data #153

Open thedarkside opened 8 months ago

thedarkside commented 8 months ago

Spyke expects data in the form of

{ 
  data: {
    id: 1,
    attributes: { 
      name: "xy",
      hero: { #<-- embedded resource
        id: 8,
        attributes: {}
      }
    }
  }
}

But apis must not stick to this schema. For example strapi is encapsulating nested objects in a subsequent "data" object like this:

{ 
  data: {
    id: 1,
    attributes: { 
      name: "xy",
      hero: {
        data: { #<-- That one
          id: 8,
          attributes: {}
        }
      }
    }
  }
}

Currently the only way of making that Spyke-compatible is to hook into Faraday and transform the payload before it gets processed by Spyke.

But at this stage we have to iterate over the whole payload and guess. We can't easily rely on the associations configuration in our Spyke models.

A better approach would be to provide a way to change the processing of nested objects if necessary.

Ive created a patch as a poc:

  class Spyke::Base
    # Allows customization via overriding. Per default returns embedded payload untouched.
    def self.unpack_embedded(payload) = payload
  end

  class Spyke::Associations::Association
    private
    def embedded_data
      parent.class.unpack_embedded parent.attributes[name]
    end
  end

With this in place i'am now able to override my StrapiBaseRecord to hook into the embedded processing and transform the payload to make it Spyke-compatible.

class StrapiBaseRecord < Spyke::Base
  def self.unpack_embedded(payload)
    # Strapi encodes embedded data nested in a "data" object we need to unpack first.
    payload&.dig(:data)
  end
end

What do you think? Currently i am using Spyke just to consume payload. No writing involved at all. Therefor i'am not sure this approach is safe for all operations. For my usecase it's working great!

balvig commented 8 months ago

Hey @thedarkside, thanks for your thoughts!

I've actually been having similar ideas. 😅

I started fiddling around with something in https://github.com/balvig/spyke/pull/137/files a while ago that I never quite got to wrap up. Could you take a look and see if it's something like that you had in mind?

thedarkside commented 8 months ago

@balvig yes that would indeed make it simpler on my side because currenlty i still have a Faraday middleware in place just to rename Strapi's meta attribute in metadata:

  class FormatHandler < Faraday::Middleware
    def on_complete(env)
      env.body[:metadata] = env.body.delete(:meta)
    end
  end

  def self.connection
    @conn ||= Strapi::Client.conn do |builder|
      builder.use FormatHandler
    end
  end

I dont like this boilerplate either. Should be definitely easier to accomplish!

But your idea isnt flexible enough i fear. For example it isnt sufficient if the data of interest is nested deeper (more then 1 levels). It also doesnt allow custom transformations of the data before it gets parsed by Spyke.

It also would not solve my problem outlined above either because the data of interest is nested deeper in the payload your solution wouldnt reach.

I prefer to leave that flexibility to the users here by opening up a surface they can hook into if they want to. With inheritance this is really easy to do. I also had the idea of providing callback functionality (also part of ActiveSupport). This would be even more sophisticated but for my case way to much.

So imho i would also implement a unpack_base method in the same fashion which could be overridden by a custom implementation like this:

class StrapiBaseRecord < Spyke::Base
  def self.unpack_base(payload)
    payload[:metadata] = payload.delete(:meta)
    payload
  end
end

In there you can do whatever you want. Adding, deleting, transforming whatever needs to be done to feed the data into Spyke.

thedarkside commented 8 months ago

A working monkeypatch including now also the base unpacking mechanic. Ive borrowed the Spyke::Result patch from your changes.

  class Spyke::Result
    def self.new_from_response_body(body)
      new(body)
    end
  end

  class Spyke::Base
    # Allows customization via overriding. Per default returns payload untouched.
    def self.unpack_base(payload) = payload
    def self.unpack_embedded(payload) = payload
  end

  module Spyke::Http::ClassMethods
    def request(method, path, params = {})
      ActiveSupport::Notifications.instrument('request.spyke', method: method) do |payload|
        response = send_request(method, path, params)
        payload[:url], payload[:status] = response.env.url, response.status
        ::Spyke::Result.new_from_response_body(unpack_base response.body)
      end
    end
  end

  class Spyke::Associations::Association
    private
    def embedded_data
      parent.class.unpack_embedded parent.attributes[name]
    end
  end