Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
swishstache opened this issue Mar 1, 2024 · 3 comments
Open

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

swishstache opened this issue Mar 1, 2024 · 3 comments
Labels

Comments

@swishstache
Copy link

swishstache commented Mar 1, 2024

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 #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
Copy link
Author

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
Copy link
Author

swishstache commented Mar 1, 2024

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).

@palkan
Copy link
Owner

palkan commented Mar 5, 2024

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 palkan added the question label Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants