jsonapi-rb / jsonapi-rails

Rails gem for fast jsonapi-compliant APIs.
http://jsonapi-rb.org
MIT License
319 stars 63 forks source link

Prevent bogus serializer creation/cleanup after failure #71

Closed ethagnawl closed 5 years ago

ethagnawl commented 6 years ago

Currently, if the serializable resource generator fails (e.g. you generate a serializer for a model which doesn't exist), an empty file is created. IMO, the generator should either validate the existence of the model before creating the file or delete the file if the task fails.

beauby commented 6 years ago

Hi @ethagnawl – I agree. Would you mind issuing a PR?

ethagnawl commented 6 years ago

@beauby I'll have a look.

ethagnawl commented 6 years ago

@beauby I looked into this very briefly, and while the solution looks simple enough (see below), I don't really know how I'd go about simulating this behavior locally or writing an appropriate test.

My back-of-the-envelope approach would look something like:

def copy_serializable_file
  fail "#{model_klass} is not a valid model name." unless model_exists?

  template 'serializable.rb.erb',
           File.join('app/serializable', class_path,
                     "serializable_#{file_name}.rb")
end

private

  def model_exists?
    Rails.application.eager_load!
    models = ApplicationRecord.descendants.map(&:name)
    !!models.find { |model_name| model_name == klass_name }
  end

Though, I'm not clear on what the generator script's entrypoint is, so it might make sense to handle model validation even before copy_serializable_file. Also, I think it's safe to wantonly call eager_load!, but I'm not certain about all of the potential ramifications of doing that.

Looking forward to any thoughts you might have on any/all of the above.

pjmartorell commented 5 years ago

Though, I'm not clear on what the generator script's entrypoint is, so it might make sense to handle model validation even before copy_serializable_file.

The generator Jsonapi::SerializableGenerator is inferred from the generator name you pass to rails generate:

rails generate jsonapi:serializable User

You can read more about it in https://guides.rubyonrails.org/generators.html:

Our new generator is quite simple: it inherits from Rails::Generators::Base and has one method definition. When a generator is invoked, each public method in the generator is executed sequentially in the order that it is defined.

Your solution looks good. I'll try to add tests and do a PR