roar-jsonapi icon indicating copy to clipboard operation
roar-jsonapi copied to clipboard

Update Operation with Roar JSONAPI representer forces contract to open up id

Open KonstantinKo opened this issue 7 years ago • 1 comments

Cross-referencing trailblazer/trailblazer-operation#17, where I originially submitted this issue

Complete Description of Issue

When using trailblazer-operation, roar-jsonapi, and reform in conjunction with an Update call, the id field of a resource is forced to be editable.

That seems like a bad idea to me. I personally would prefer it to ignore the id part of the JSONAPI request.

Admittedly this is also a bit of an inconsistency with the JSONAPI spec. An update as a PATCH /resources/1 MUST include the id field. An inconsistency between the URL id and the given id in the document is not handled. Maybe there is an edge case where you would want to edit the ID. However whether you want to or not, it has to be provided as a parameter.

Steps to reproduce (Code sample)

class MyResource < Struct.new(:id, :name)
  def self.find_by(id:)
     DB[id.to_i]
  end
  def save
    DB[id.to_i] = self
  end
end
DB = {1 => MyResource.new(1, 'one')} # Simulating database

class MyContract < Reform::Form
  property :name
end

class MyRepresenter < Roar::Decorator
  include Roar::JSON::JSONAPI.resource :resources
  attributes do
    property :name
  end
end

class MyOperation < Trailblazer::Operation
  step Model(MyResource, :find_by)
  step Contract::Build(constant: MyContract)
  step Contract::Validate(representer: MyRepresenter)
  step Contract::Persist()
end

MyOperation.({id: 1}, 'document' => '{"data":{"type":"resources","attributes":{"name":"changed"},"id":"9999"}}')
# => NoMethodError: undefined method `id=' for #<MyContract>

# Adding `property :id` to MyContract fixes the NoMethodError,
# but allows editing the ID field which is usually a security vulnerability:
MyOperation.({id: 1}, 'document' => '{"data":{"type":"resources","attributes":{"name":"changed"},"id":"9999"}}')
# => <Result:true ...>
DB
# => {1 => #<struct name="one">, 9999 => #<struct name="changed">}

Expected behavior

As stated, in edge cases this might be the actually desired behavior. In most cases one would not want an ID to be editable. So an opt-in would be preferable to me.

System configuration

Roar version: roar-edcd8efa0181 Roar JSONAPI version: roar-jsonapi-3ca7fa8e0f5a

KonstantinKo avatar May 17 '17 12:05 KonstantinKo

@myabc was it solved or we should work on it?

panSarin avatar Apr 28 '20 10:04 panSarin