Fix adapter inflection bug for api -> API

[bf4]

Bug: #993

Summary

1. Added a (failing) test for when inflecting API (Rob McFadzean in Added a (failing) test for when inflecting API #997)
2. sub s/API/Api

Some work done in here was moved in #1017

[bf4]

cc @kurko @joaomdmoura your thoughts?

[joaomdmoura]

@bf4 this seems almost the same logic of the adapter method
We should probably remove it from there in order to use this new get method, right?

[bf4]

You mean on the serializer? I can dedupe that in a new commit or pr

[joaomdmoura]

Yeap, it's exactly the same logic. IMHO it's related to this PR, check this out:

We can replace the actual code:

def self.adapter
  adapter_class = case config.adapter
  when Symbol
    ActiveModel::Serializer::Adapter.adapter_class(config.adapter)
  when Class
    config.adapter
  end
  unless adapter_class
    valid_adapters = Adapter.constants.map { |klass| ":#{klass.to_s.downcase}" }
    raise ArgumentError, "Unknown adapter: #{config.adapter}. Valid adapters are: #{valid_adapters}"
  end

  adapter_class
end

By something like:

def self.adapter
  ActiveModel::Serializer::Adapter.adapter_class(config.adapter)
end

[bf4]

I'll take a look when I'm next at a computer. Do you like the new interface and behavior? Or have an alternative in mind?

[joaomdmoura]

cool! Yup, really liked it! If we can make it the only place to deal with this logic then I'm okay with it 😄

[bf4]

Cool, I was a little uncertain about the naming

So, the two methods havr slightly different behavior

The one in the adapter would return nil if none found, but the one in the serializer raises an argument error

So, i was thinking of having get and get! on the adapter

Needs some thought where nil is desired

[bf4]

Added another commit to consider. I've been playing with the interface for success/failurre handling of finding the adapter by name and whether to find it by constants, or safe constantize, etc. So, this is for consideration as a way to do.

[joaomdmoura]

@bf4 it's seems to me nil is never desired. What are your thoughts about keep the argument error approach and remote get!?

[bf4]

Sounds good. I'd still like to allow for user defined failure handling

[bf4]

You'll notice that get can take a block to handle adapter not found, and is used by get!

        def get!(adapter)
          get(adapter) do |failure_message| fail ArgumentError, failure_message end
        end

[bf4]

It was easier to handle the simple case of api being inflected to API rather than Api via sub (not sure that's the most performant way. I didn't investigate), rather than have a whole bunch of changes around that either mutating ActiveSupport::Inflector.inflections(:en).acronyms or using I18n.with_locale(:ams) { }, or changing how we look up constants, etc.

[bf4]

The stuff with the registered adapter I'll do in another PR, because it also brings up issues with autoloading

[joaomdmoura]

@bf4 Will #1017 eliminate this? Because I'm not sure about it.

[bf4]

No, this PR is still required. I just think the complexity I was introducing wasn't required for the range of inflection failures

[joaomdmoura]

Okay @bf4. Good, it seems the most straightforward solution for me as well. I'm merging it.