store_attribute icon indicating copy to clipboard operation
store_attribute copied to clipboard

BUG: using :json type results in double-encoded value

Open swishstache opened this issue 1 year ago • 3 comments

Ruby Version: 3.2.2

Rails Version: 6.1.7.6

PostgreSQL Version: 14

Store Attribute Version: 0.8.1 confirmed on 1.2.0 as well

What did you do?

Specified the type as :json when using store_attribute & store_accessor on a JSONB type PostgreSQL column.

What did you expect to happen?

Attributes should serialize as JSON.

What actually happened?

Attributes serialize as a string.

Details

This is a circle back to https://github.com/palkan/store_attribute/issues/28. At the time, I thought the issue constrained to only store_accessor and that we simply should not have been specifying a type. But, it seems reasonable that the gem should do the right thing and not re-encode the attribute when using :json as the type. This affects store_attribute too.

Per the ActiveRecord docs

NOTE: If you are using structured database data types (e.g. PostgreSQL hstore/json, or MySQL 5.7+ json) there is no need for the serialization provided by .store. Simply use .store_accessor instead to generate the accessor methods. Be aware that these columns use a string keyed hash and do not allow access using a symbol.

As a workaround, I've created a new type that simply passes through the values on serialize/deserialize. I'm hoping the gem can be updated so we can avoid this.

Reproduction

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"
  ruby '3.2.2'

  gem "rails", "6.1.7.6"
  gem "sqlite3"
  gem 'store_attribute', '0.8.1'
  gem 'byebug'
end

require "active_record"
require "minitest/autorun"
# require "logger"
Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test)

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
# ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :users do |t|
    t.json "preferences", default: {}
    t.timestamps
  end
end

class ApplicationRecord < ActiveRecord::Base
  self.abstract_class = true
end

class JsonProxy < ActiveRecord::Type::Json
  def type
    :json_proxy
  end

  def serialize(value)
    value
  end
end

ActiveRecord::Type.register(:json_proxy, JsonProxy)


class User < ApplicationRecord
  store_attribute :preferences, :skip_provisioning, :json, default: []
  store_attribute :preferences, :skip_provisioning_fixed, :json_proxy, default: []

  store_accessor :preferences, form_data: :json
  store_accessor :preferences, form_data_fixed: :json_proxy
  store_accessor :preferences, :typeless_form_data
end

class StoreAttributeGemTest < ActiveSupport::TestCase
  def test_skip_provisioning_should_default_to_empty_array
    user = User.create!

    preferences_before_type_cast = JSON.parse(user.preferences_before_type_cast)
    # user.preferences_before_type_cast #=> "{\"skip_provisioning\":\"[]\",\"skip_provisioning_fixed\":[]}"
    
    field_before_type_cast = preferences_before_type_cast.fetch('skip_provisioning', nil)

    assert field_before_type_cast.is_a?(Array)
    assert_empty field_before_type_cast
  end

  def test_skip_provisioning_should_persist_as_an_array_when_given_a_value
    user = User.create!
    user.skip_provisioning |= ['teams']
    user.save!
    user.reload

    preferences_before_type_cast = JSON.parse(user.preferences_before_type_cast)
    # user.preferences_before_type_cast #=> "{\"skip_provisioning\":\"[\\\"teams\\\"]\",\"skip_provisioning_fixed\":[]}"

    field_before_type_cast = preferences_before_type_cast.fetch('skip_provisioning', nil)

    assert field_before_type_cast.is_a?(Array)
    assert_equal ['teams'], field_before_type_cast
  end
  
  
  def test_form_data_should_persist_as_json
    expected_form_data = { "communication_id" => "" }
    user = User.create!(form_data: expected_form_data)

    preferences_before_type_cast = JSON.parse(user.preferences_before_type_cast)
    # user.preferences_before_type_cast #=> "{\"skip_provisioning\":\"[]\",\"skip_provisioning_fixed\":[],\"form_data\":\"{\\\"communication_id\\\":\\\"\\\"}\"}"
    
    field_before_type_cast = preferences_before_type_cast.fetch('form_data', nil)

    assert field_before_type_cast.is_a?(Hash)
    assert_equal expected_form_data, field_before_type_cast
  end
  
  def test_skip_provisioning_fixed_should_default_to_empty_array
    user = User.create!

    preferences_before_type_cast = JSON.parse(user.preferences_before_type_cast)
    # user.preferences_before_type_cast #=> "{\"skip_provisioning\":\"[]\",\"skip_provisioning_fixed\":[]}"

    field_before_type_cast = preferences_before_type_cast.fetch('skip_provisioning_fixed', nil)

    assert field_before_type_cast.is_a?(Array)
    assert_empty field_before_type_cast
  end

  def test_skip_provisioning_fixed_should_persist_as_an_array_when_given_a_value
    user = User.create!
    user.skip_provisioning_fixed |= ['teams']
    user.save!
    user.reload

    preferences_before_type_cast = JSON.parse(user.preferences_before_type_cast)
    # user.preferences_before_type_cast #=> "{\"skip_provisioning\":\"[]\",\"skip_provisioning_fixed\":[\"teams\"]}"

    field_before_type_cast = preferences_before_type_cast.fetch('skip_provisioning_fixed', nil)

    assert field_before_type_cast.is_a?(Array)
    assert_equal ['teams'], field_before_type_cast
  end

  def test_form_data_fixed_should_persist_as_json
    expected_form_data = { "communication_id" => "" }
    user = User.create!(form_data_fixed: expected_form_data)

    preferences_before_type_cast = JSON.parse(user.preferences_before_type_cast)
    # user.preferences_before_type_cast #=> "{\"skip_provisioning\":\"[]\",\"skip_provisioning_fixed\":[],\"form_data_fixed\":{\"communication_id\":\"\"}}"

    field_before_type_cast = preferences_before_type_cast.fetch('form_data_fixed', nil)

    assert field_before_type_cast.is_a?(Hash)
    assert_equal expected_form_data, field_before_type_cast
  end

  def test_typeless_form_data_should_persist_as_json
    expected_form_data = { "communication_id" => "" }
    user = User.create!(typeless_form_data: expected_form_data)

    preferences_before_type_cast = JSON.parse(user.preferences_before_type_cast)
    # user.preferences_before_type_cast #=> "{\"skip_provisioning\":\"[]\",\"skip_provisioning_fixed\":[],\"typeless_form_data\":{\"communication_id\":\"\"}}"

    field_before_type_cast = preferences_before_type_cast.fetch('typeless_form_data', nil)

    assert field_before_type_cast.is_a?(Hash)
    assert_equal expected_form_data, field_before_type_cast
  end
end

swishstache avatar Mar 01 '24 21:03 swishstache

This has come up recently because we were upgrading from Rails 6.1.7.6 to Rails 7.0. The former will return the values as JSON but the latter does not - it returns a string which was causing errors in usage.

swishstache avatar Mar 01 '24 21:03 swishstache

This can be avoided when using store_accessor:

class User < ApplicationRecord
  # works fine
  store_accessor :params, :form_data
  
  # double encodes
  store_accessor :params, form_data: :json
end

However, we use store_attribute as well for the default value feature and I don't think there's a workaround (outside the custom type).

swishstache avatar Mar 01 '24 21:03 swishstache

Attributes serialize as a string.

And that's correct; we have no assumptions regarding the store format (it could be not only JSON but, for example, YAML), we serialize the attribute value into a JSON and it's a string.

If you want to store an array within a store field, you shouldn't do any type casting and just set a default:

store_attribute :params, :form_data, default: []

I've added the following change to your gist and all tests pass:

 class User < ApplicationRecord
+  store_attribute :preferences, :skip_provisioning, ActiveModel::Type::Value.new, default: []
-  store_attribute :preferences, :skip_provisioning, :json, default: []
   store_attribute :preferences, :skip_provisioning_fixed, :json_proxy, default: []

+  store_accessor :preferences, form_data: ActiveModel::Type::Value.new
-  store_accessor :preferences, form_data: :json
   store_accessor :preferences, form_data_fixed: :json_proxy
   store_accessor :preferences, :typeless_form_data
 end

Note that I had to use ActiveModel::Type::Value.new. because passing no type (as I suggested above) is currently not supported.

Let me know if that's what you was looking for.

palkan avatar Mar 05 '24 01:03 palkan