Skip to content

Commit

Permalink
Improve remote resource behavior
Browse files Browse the repository at this point in the history
* Previously, we'd disallow all writes. Now we allow associating to an
existing remote entity (only applies to `belongs_to).
* Previously, we'd avoid links for remote resources. Now, we avoid link
validation (because we can't validate a remote API), but still generate
the links (you can of course always turn this off).
* As part of the last point, improved foreign key inferrence. We usually
infer FK via the model name, but for remote resources the model is
OpenStruct so we should use the resource name instead.
  • Loading branch information
Lee Richmond committed Mar 30, 2019
1 parent 0ce4f3d commit f0e22dd
Show file tree
Hide file tree
Showing 11 changed files with 174 additions and 17 deletions.
6 changes: 6 additions & 0 deletions lib/graphiti/resource/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ def adapter=(val)
def remote=(val)
super
include ::Graphiti::Resource::Remote
self.endpoint = {
path: val,
full_path: val,
url: val,
actions: [:index, :show]
}
end

def model
Expand Down
8 changes: 6 additions & 2 deletions lib/graphiti/resource/remote.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@ def remote_url
end
end

def save(*args)
raise Errors::RemoteWrite.new(self.class)
def save(model, meta)
if meta[:attributes] == {} && meta[:method] == :update
model
else
raise Errors::RemoteWrite.new(self.class)
end
end

def destroy(*args)
Expand Down
2 changes: 2 additions & 0 deletions lib/graphiti/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ def generate_types
def generate_endpoints
{}.tap do |endpoints|
@resources.each do |r|
next if r.remote?

r.endpoints.each do |e|
actions = {}
e[:actions].each do |a|
Expand Down
1 change: 0 additions & 1 deletion lib/graphiti/sideload.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ def initialize(name, opts)
end

if remote?
@link = false
@resource_class = create_remote_resource
end
end
Expand Down
19 changes: 15 additions & 4 deletions lib/graphiti/sideload/belongs_to.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,21 @@ def infer_foreign_key
if polymorphic_child?
parent.foreign_key
else
model = resource.model
namespace = namespace_for(model)
model_name = model.name.gsub("#{namespace}::", "")
:"#{model_name.underscore}_id"
if resource.remote?
namespace = namespace_for(resource.class)
resource_name = resource.class.name
.gsub("#{namespace}::", "")
.gsub('Resource', '')
if resource_name.include?('.remote')
resource_name = resource_name.split('.remote')[0].split('.')[1]
end
:"#{resource_name.singularize.underscore}_id"
else
model = resource.model
namespace = namespace_for(model)
model_name = model.name.gsub("#{namespace}::", "")
:"#{model_name.underscore}_id"
end
end
end

Expand Down
2 changes: 2 additions & 0 deletions lib/graphiti/util/serializer_relationships.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ def validate_link!
end

def validate_link_for_sideload!(sideload)
return if sideload.resource.remote?

action = sideload.type == :belongs_to ? :show : :index
cache_key = :"#{@sideload.object_id}-#{action}"
return if self.class.validated_link_cache.include?(cache_key)
Expand Down
30 changes: 26 additions & 4 deletions spec/remote_resource_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -777,10 +777,32 @@ def self.name
end

context "when updating" do
it "raises error" do
expect {
klass.find(id: 1, data: {type: "employees"}).update_attributes
}.to raise_error(Graphiti::Errors::RemoteWrite, /not supported/)
let(:payload) do
{
data: {
type: 'employees',
id: '123',
attributes: { last_name: 'Jane' }
}
}
end

context 'and only associating to a remote parent' do
before do
payload[:data].delete(:attributes)
end

it 'works' do
klass.find(payload).update_attributes
end
end

context 'and passing more attributes than simple association' do
it "raises error" do
expect {
klass.find(payload).update_attributes
}.to raise_error(Graphiti::Errors::RemoteWrite, /not supported/)
end
end
end

Expand Down
5 changes: 3 additions & 2 deletions spec/schema_diff_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,9 @@ def self.name
end
end

it "does not diff" do
expect(diff).to eq([])
it "notes only the newly-missing missing endpoint" do
expect(diff)
.to eq(["Endpoint \"/schema_diff/employees\" was removed."])
end
end

Expand Down
10 changes: 10 additions & 0 deletions spec/schema_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,16 @@ def self.name
end
end

context 'when a resource is remote' do
before do
employee_resource.remote = 'http://foo.com/employees'
end

it 'does not add endpoints to the schema' do
expect(schema[:endpoints]).to eq({})
end
end

context "when 2 resources, same path" do
let(:employee_search_resource) do
Class.new(application_resource) do
Expand Down
57 changes: 53 additions & 4 deletions spec/serialization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,18 @@ def positions
.to eq("/poro/positions?filter[employee_id]=1")
end

context 'that is remote' do
before do
resource.has_many :positions, remote: 'http://foo.com/positions'
end

it 'links correctly' do
render
expect(positions["links"]["related"])
.to eq("http://foo.com/positions?filter[employee_id]=1")
end
end

context "opting-out of linking" do
before do
resource.has_many :positions, link: false
Expand Down Expand Up @@ -1159,6 +1171,24 @@ def define_relationship
.to eq("/poro/mastercards/789")
end

context 'that is remote' do
before do
resource.polymorphic_belongs_to :credit_card do
group_by(:credit_card_type) do
on(:Mastercard).belongs_to :mastercard,
remote: 'http://foo.com/mastercards'
end
end
end

it 'links correctly' do
render
credit_card = json["data"][0]["relationships"]["credit_card"]
expect(credit_card["links"]["related"])
.to eq("http://foo.com/mastercards/789")
end
end

context "but one child does not have an endpoint" do
before do
mastercard_resource.endpoint = nil
Expand Down Expand Up @@ -1212,15 +1242,24 @@ def define_relationship
end

context "and a has_one relationship" do
before do
resource.has_one :bio
end

it "links to index endpoint" do
resource.has_one :bio
render
expect(json["data"][0]["relationships"]["bio"]["links"]["related"])
.to eq("/poro/bios?filter[employee_id]=#{employee.id}")
end

context 'that is remote' do
before do
resource.has_one :bio, remote: 'http://foo.com/bios'
end

it 'links correctly' do
render
expect(json["data"][0]["relationships"]["bio"]["links"]["related"])
.to eq("http://foo.com/bios?filter[employee_id]=#{employee.id}")
end
end
end

context "and a belongs_to relationship" do
Expand Down Expand Up @@ -1305,6 +1344,16 @@ def classification
xit "links correctly" do
end
end

context 'when remote' do
it 'links correctly' do
resource.belongs_to :classification,
remote: 'http://foo.com/classifications'
render
expect(classification['links']['related'])
.to eq('http://foo.com/classifications/789')
end
end
end
end
end
Expand Down
51 changes: 51 additions & 0 deletions spec/sideload_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,57 @@ class SideloadSpecEmployee
expect(instance.infer_foreign_key).to eq(:sideload_spec_employee_id)
end
end

context 'when the resource is remote' do
let(:name) { "positions" }

context 'via the sideload :remote option' do
it 'is inferred correctly from the parent resource' do
opts.delete(:resource)
opts[:remote] = "http://foo.com/positions"
expect(instance.infer_foreign_key).to eq(:employee_id)
end

context 'and belongs_to' do
let(:instance) { Class.new(Graphiti::Sideload::BelongsTo).new(name, opts) }

before do
opts[:type] = :belongs_to
end

it 'works' do
opts.delete(:resource)
opts[:remote] = "http://foo.com/positions"
expect(instance.infer_foreign_key).to eq(:position_id)
end
end
end

context 'via resource class' do
before do
opts[:resource] = Class.new(Graphiti::Resource) do
self.remote = 'http://foo.com'
def self.name;'PORO::PositionResource';end
end
end

it 'is inferred correctly from the parent resource' do
expect(instance.infer_foreign_key).to eq(:employee_id)
end

context 'and belongs_to' do
let(:instance) { Class.new(Graphiti::Sideload::BelongsTo).new(name, opts) }

before do
opts[:type] = :belongs_to
end

it 'works' do
expect(instance.infer_foreign_key).to eq(:position_id)
end
end
end
end
end

describe "#ids_for_parents" do
Expand Down

0 comments on commit f0e22dd

Please # to comment.