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

Fix typing for the getter of serialized columns #1423

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

wpolicarpo
Copy link
Contributor

@wpolicarpo wpolicarpo commented Feb 28, 2023

Motivation

According to this table, serialized columns always return an empty Array or Hash if their coders are either an Array or Hash respectively and the value in the database is null.

Currently tapioca assumes the getter will always return a nilable type, which is not true:

Toggle me for an example!
require "bundler/inline"

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

  gem "activerecord", "7.0.4.2"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

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

ActiveRecord::Schema.define do
  create_table :books, force: true do |t|
    t.string :authors
  end
end

class Book < ActiveRecord::Base
  serialize :authors, Array
end

class BookTest < Minitest::Test
  def test_populated_array
    book = Book.new(authors: ["foo", "bar"])
    assert_equal(["foo", "bar"], book.authors)
  end

  def test_empty_array
    book = Book.new(authors: [])
    assert_equal([], book.authors)
  end

  def test_nil
    book = Book.new(authors: nil)
    assert_equal([], book.authors)
  end
end

Implementation

This PR changes the getters return type for serialized columns where the coder is one of those exceptional classes.

Tests

Added tests for all getters, making sure the Array and Hash ones check for a not nilable type being returned.

@wpolicarpo wpolicarpo requested a review from a team as a code owner February 28, 2023 18:24
@wpolicarpo wpolicarpo force-pushed the proper-typing-for-serialized-ar-columns branch 2 times, most recently from 7d041ee to 1e3c362 Compare March 2, 2023 18:11
@wpolicarpo wpolicarpo force-pushed the proper-typing-for-serialized-ar-columns branch from 1e3c362 to 1e09109 Compare March 3, 2023 12:10
@wpolicarpo wpolicarpo merged commit 6d7f9a7 into main Mar 3, 2023
@wpolicarpo wpolicarpo deleted the proper-typing-for-serialized-ar-columns branch March 3, 2023 12:40
@egiurleo egiurleo added the bugfix label Mar 9, 2023
@shopify-shipit shopify-shipit bot temporarily deployed to production March 10, 2023 16:06 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants