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

Migration workflow #32

Closed
romansavrulin opened this issue Aug 31, 2019 · 7 comments

Comments

@romansavrulin
Copy link

commented Aug 31, 2019

How can we handle data migration in underlying jsonb field?

Here's an example. I've defined the following structure for AssetIssueList v1

class AssetIssue
  include StoreModel::Model

  attribute :key, :string, default: ''
  attribute :name, :string, default: ''
  attribute :exists, :boolean, default: false

  validates :key, name, :exists, presence: true
end

class AssetIssueList
  include StoreModel::Model

  attribute :version, :integer, default: 1
  attribute :list, AssetIssue.to_array_type, default: [ ]

  validates :version, :list, presence: true
end

class Asset < ApplicationRecord
  attribute :issues, AssetIssueList.to_type

  after_find :validateIssues
  def validateIssues
    if issues.nil? || issues.version != ISSUES_VERSION
      updateDefaultIssues
    end
  end
end

After some time, I've decided to remove attribute :name from AssetIssue. Application is unable to start due to error

ActiveModel::UnknownAttributeError (unknown attribute 'name' for AssetIssue.)

/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/attribute_assignment.rb:53:in `_assign_attribute'
/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/attribute_assignment.rb:44:in `block in _assign_attributes'
/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/attribute_assignment.rb:43:in `each'
/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/attribute_assignment.rb:43:in `_assign_attributes'
/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/attribute_assignment.rb:35:in `assign_attributes'
/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/model.rb:81:in `initialize'
/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/attributes.rb:80:in `initialize'
/usr/local/bundle/gems/store_model-0.5.2/lib/store_model/types/array_type.rb:78:in `new'
/usr/local/bundle/gems/store_model-0.5.2/lib/store_model/types/array_type.rb:78:in `block in ensure_model_class'
/usr/local/bundle/gems/store_model-0.5.2/lib/store_model/types/array_type.rb:77:in `map'
/usr/local/bundle/gems/store_model-0.5.2/lib/store_model/types/array_type.rb:77:in `ensure_model_class'
/usr/local/bundle/gems/store_model-0.5.2/lib/store_model/types/array_type.rb:34:in `cast_value'
/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/type/value.rb:38:in `cast'
/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/attribute.rb:174:in `type_cast'
/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/attribute.rb:42:in `value'
/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/attribute_set.rb:41:in `fetch_value'
/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/attributes.rb:130:in `attribute'
/usr/local/bundle/gems/activemodel-6.0.0/lib/active_model/attribute_methods.rb:383:in `list'

/app/models/navigation_asset.rb:14:in `validateIssues'

How to deal with that? Couldn't be not existent static keys (name in my example) just dynamic in AssetIssueList so we can migrate underlying data storage with ease?

@romansavrulin

This comment has been minimized.

Copy link
Author

commented Aug 31, 2019

I've digged into the code and found that json_type has rescue

def decode_and_initialize(value)
decoded = ActiveSupport::JSON.decode(value) rescue nil
@model_klass.new(decoded) unless decoded.nil?
rescue ActiveModel::UnknownAttributeError => e
handle_unknown_attribute(decoded, e)
end
# rubocop:enable Style/RescueModifier

But array_type seems not

def ensure_model_class(array)
array.map do |object|
object.is_a?(@model_klass) ? object : @model_klass.new(object)
end
end

@DmitryTsepelev

This comment has been minimized.

Copy link
Owner

commented Aug 31, 2019

Hi @romansavrulin!

That's a fair point, array elements should be handled in the exact same way. I'd really appreciate a PR or I'll implement it by myself a little bit later 🙂

As an alternative solution, the same result could be reached by adding a regular SQL migration that will update the structure of the JSON (I guess it would be helpful to document this process)

@romansavrulin

This comment has been minimized.

Copy link
Author

commented Aug 31, 2019

@DmitryTsepelev I have never developed or fixed a ruby gem before, but I can try. Can you provide a link to any good starting point guide about how to install this gem as a git submodule to an exiting project, instead of using a bundler definition?

@DmitryTsepelev

This comment has been minimized.

Copy link
Owner

commented Aug 31, 2019

The easiest way is to change the gem itself:

  1. Fork the gem and clone it to your machine
  2. Navigate to the gem folder and run bundle install
  3. Run specs (it's just a regular rspec, specs should pass)
  4. Add new specs for ArrayType (similar to these specs)
  5. Fix the code to pass the specs
  6. Commit and make and PR, CI will make sure specs are passing

If you want to play with your own version of the gem as a part of a real project, you can open up project's Gemfile and replace gem "store_model" with gem "store_model", path: "/path/to/the/gem/on/your/local/machine" and run bundle install – it will use your local version.

Please let me know if you have any questions or issues 🙂

@alexeevit

This comment has been minimized.

Copy link

commented Sep 4, 2019

What behavior is expected?

  • ArrayType#unkown_attributes returns hash contains all unknown attributes from all the elements
    or
  • Each of the array elements has it's own #unknown_attributes

The first one is little strange because it will merge all #unkown_attributes hashes and replace elements which has equal keys. But the description of the task on cultofmartians is contains exactly this behavior.

@DmitryTsepelev

This comment has been minimized.

Copy link
Owner

commented Sep 4, 2019

@alexeevit the second one, we need to have an array of objects with #unknown_attributes filled for them (I'll update the description at cult since it sounds confusing 🙂)

@DmitryTsepelev

This comment has been minimized.

Copy link
Owner

commented Sep 5, 2019

Released a fix as part of 0.5.3. Closing the issue for now, please let me know if something goes wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.