cerebris / jsonapi-resources

A resource-focused Rails library for developing JSON:API compliant servers.
http://jsonapi-resources.com
MIT License
2.32k stars 529 forks source link

`:unprocessable_entity` deprecated in newest Rack, and causing a `0` response #1456

Open pareeohnos opened 3 months ago

pareeohnos commented 3 months ago

This issue is a (choose one):

Checklist before submitting:

Description

A change introduced to Rack deprecates the status : unprocessable_entity in favour of :unprocessable_content. They have handled this deprecation internally, however jsonapi-resources retrieves the status code by directly accessing the Rack::Utils::SYMBOL_TO_STATUS_CODE hash. Due to the deprecation, no status is found which results in jsonapi-resources returning a 0 status when there are validation errors.

Short term this can be fixed by using the Rack methods for status retrieval rather than accessing the hash directly. Longer term, jsonapi-resources will need to stop using :unprocessable_entity in favour of : unprocessable_entity but it would have to be done in a way that checks the version of Rack.

Reproducible code:

begin
  require 'bundler/inline'
  require 'bundler'
rescue LoadError => e
  STDERR.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
  raise e
end

gemfile(true, ui: ENV['SILENT'] ? Bundler::UI::Silent.new : Bundler::UI::Shell.new) do
  source 'https://rubygems.org'

  gem 'rails', '7.1.3.4', require: false
  gem 'sqlite3', '~> 1.4'

  if ENV['JSONAPI_RESOURCES_PATH']
    gem 'jsonapi-resources', path: ENV['JSONAPI_RESOURCES_PATH'], require: false
  else
    gem 'jsonapi-resources', git: 'https://github.com/cerebris/jsonapi-resources', require: false
  end

end

# prepare active_record database
require 'active_record'

class NullLogger < Logger
  def initialize(*_args)
  end

  def add(*_args, &_block)
  end
end

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = ENV['SILENT'] ? NullLogger.new : Logger.new(STDOUT)
ActiveRecord::Migration.verbose = !ENV['SILENT']

ActiveRecord::Schema.define do
  # Add your schema here
  create_table :your_models, force: true do |t|
    t.string :name
  end
end

# create models
class YourModel < ActiveRecord::Base
  validates :name, presence: true
end

# prepare rails app
require 'action_controller/railtie'
# require 'action_view/railtie'
require 'jsonapi-resources'

class ApplicationController < ActionController::Base
end

# prepare jsonapi resources and controllers
class YourModelsController < ApplicationController
  include JSONAPI::ActsAsResourceController
end

class YourModelResource < JSONAPI::Resource
  attribute :name
  filter :name
end

class TestApp < Rails::Application
  config.root = File.dirname(__FILE__)
  config.logger = ENV['SILENT'] ? NullLogger.new : Logger.new(STDOUT)
  Rails.logger = config.logger

  secrets.secret_token = 'secret_token'
  secrets.secret_key_base = 'secret_key_base'

  config.eager_load = false

  config.hosts << 'example.org'
end

# initialize app
Rails.application.initialize!

JSONAPI.configure do |config|
  config.json_key_format = :underscored_key
  config.route_format = :underscored_key
end

# draw routes
Rails.application.routes.draw do
  jsonapi_resources :your_models, only: [:index, :create]
end

# prepare tests
require 'minitest/autorun'
require 'rack/test'

# Replace this with the code necessary to make your test fail.
class BugTest < Minitest::Test
  include Rack::Test::Methods

  def json_api_headers
    {'Accept' => JSONAPI::MEDIA_TYPE, 'CONTENT_TYPE' => JSONAPI::MEDIA_TYPE}
  end

  def test_index_your_models
    record = YourModel.create! name: 'John Doe'
    get '/your_models', nil, json_api_headers
    assert last_response.ok?
    json_response = JSON.parse(last_response.body)
    refute_nil json_response['data']
    refute_empty json_response['data']
    refute_empty json_response['data'].first 
    assert record.id.to_s, json_response['data'].first['id']
    assert 'your_models', json_response['data'].first['type']
    assert({'name' => 'John Doe'}, json_response['data'].first['attributes'])
  end

  def test_create_your_models
    json_request = {
        'data' => {
            type: 'your_models',
            attributes: {
                name: nil
            }
        }
    }
    post '/your_models', json_request.to_json, json_api_headers
    puts last_response.status
    assert last_response.status == 422
    refute_nil YourModel.find_by(name: 'Jane Doe')
  end

  private

  def app
    Rails.application
  end
end
arcreative commented 2 months ago

@pareeohnos Just one minor note--your description says ":unprocessable_entity in favour of :unprocessible_entity" the second time around.

arcreative commented 2 months ago

@pareeohnos I fixed this issue in my application by just adding the key back into the Rack utils for now:

Rack::Utils::SYMBOL_TO_STATUS_CODE[:unprocessable_entity] = 422

While I might expect the JR maintainers to fix this for current versions, I'm still running 0.9.12 and am not expecting a backport, so this is the simplest solution for me at the moment.

pareeohnos commented 2 months ago

@pareeohnos Just one minor note--your description says ":unprocessable_entity in favour of :unprocessible_entity" the second time around.

Whoops, good catch I've fixed.

@pareeohnos I fixed this issue in my application by just adding the key back into the Rack utils for now:

Rack::Utils::SYMBOL_TO_STATUS_CODE[:unprocessable_entity] = 422

While I might expect the JR maintainers to fix this for current versions, I'm still running 0.9.12 and am not expecting a backport, so this is the simplest solution for me at the moment.

Ah yeah that's a nice simple fix. The PR I've opened should be backwards compatible so if it gets merged and you are able to upgrade it shouldn't break anything but that's definitely the way to go until it's merged