Bug in LookupChain::BY_NAMESPACE

[heaven]

I have 2 models: Profile and Public::Profile and 2 serializers:

• Api::Platform::Carmen::V1::ProfileSerializer
• Api::Platform::Carmen::V1::Public::ProfileSerializer

The first Profile could have many Public::Profiles so in the first serializer we have has_many :public_profiles.

As result AMS tries to use the first serializer for both, while the expected behavior would be using a separate serializer for Public::Profile.

Some debug information:

(byebug) resource_class
 => Public::Profile(id: integer, ...)

(byebug) resource_class_name(resource_class)
 => "Profile"

(byebug) namespace
 => Api::Platform::Carmen::V1

(byebug) "#{namespace}::#{resource_name}Serializer"
 => "Api::Platform::Carmen::V1::ProfileSerializer"

Absolutely unclear to me why resource_class_name demodulizes the name.

[heaven]

I think I've found an answer – when we open Public::ProfilesController we'll end up with Public::Public::ProfileSerializer name...

Not sure how to work around this yet. Perhaps instead of demodulizing, we should connect the namespaces of controller and model by ensuring there're no duplicates, like Api::Platform::Carmen::V1::Public + Public::Profile => Api::Platform::Carmen::V1::Public::Profile (like .split("::").uniq.join("::")). Which will break this for those who intentionally have duplicated nested namespaces like Public::Public (not sure why someone may need this, tho).

Also, there's a similar issue with the BY_PARENT_SERIALIZER, which also uses demodulize and doesn't find child serializers for namespaced models. For example, we have Market::Contractor, Market::Company and the following doesn't work:

class Api::Platform::Carmen::V1::ProfileSerializer < ApplicationSerializer
  class Market::ContractorSerializer < ApplicationSerializer
  end

  class Market::CompanySerializer < ApplicationSerializer
  end
end

Instead, it wants to find child ContractorSerializer and CompanySerializer classes.

[wasifhossain]

do you think that explicitly passing the serializer option in the association could help AMS look up the serializer in correct way?

[heaven]

Indeed it will, but here I am talking about the current lookup chain implementation and its unexpected behavior.

Currently, there's a problem with namespaces when using BY_PARENT_SERIALIZER proc, which shows up when you have Market::Account and Public::Account models and need a serializer for Profile that has many both :market_accounts and public_accounts:

class ProfileSerializer
  module Market
    class AccountSerializer; end
  end

  module Public
    class AccountSerializer; end
  end

  has_many :market_accounts
  has_many :public_accounts
end

None of these child serializers will be discovered by PARENT_SERIALIZER proc. Instead, AMS will expect to see just one AccountSerializer and will use it for both account types, which is wrong and looks like a bug.

I ended up with the following configuration:

ActiveModelSerializers::LookupChain::BY_PARENT_SERIALIZER_CUSTOM = lambda do |resource_class, serializer_class, _namespace|
  return if serializer_class == ActiveModel::Serializer

  "#{serializer_class}::#{resource_class.name}Serializer"
end

ActiveModelSerializers::LookupChain::BY_NAMESPACE_CUSTOM = lambda do |resource_class, _serializer_class, namespace|
  if namespace
    namespace_chain     = namespace.name.split("::")
    resource_name_chain = resource_class.name.split("::")

    if resource_name_chain.size > 1 and namespace_chain.last == resource_name_chain.first
      namespace_chain.pop
    end

    "#{(namespace_chain + resource_name_chain).join("::")}Serializer"
  end
end

ActiveModelSerializers.config.serializer_lookup_chain = [
  ActiveModelSerializers::LookupChain::BY_PARENT_SERIALIZER_CUSTOM,
  ActiveModelSerializers::LookupChain::BY_NAMESPACE_CUSTOM,
  ActiveModelSerializers::LookupChain::BY_RESOURCE_NAMESPACE,
  ActiveModelSerializers::LookupChain::BY_RESOURCE
]

Which works well with the namespaces for both models and controllers.

Not sure if any action is required in the gem, just letting you know about the discovered issues.