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

Respect namespacing in SerializerResolver #93

Merged
merged 1 commit into from
Feb 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/panko/serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def method_added(method)

def has_one(name, options = {})
serializer_const = options[:serializer]
serializer_const = Panko::SerializerResolver.resolve(name.to_s) if serializer_const.nil?
serializer_const = Panko::SerializerResolver.resolve(name.to_s, self) if serializer_const.nil?

raise "Can't find serializer for #{self.name}.#{name} has_one relationship." if serializer_const.nil?

Expand All @@ -88,7 +88,7 @@ def has_one(name, options = {})

def has_many(name, options = {})
serializer_const = options[:serializer] || options[:each_serializer]
serializer_const = Panko::SerializerResolver.resolve(name.to_s) if serializer_const.nil?
serializer_const = Panko::SerializerResolver.resolve(name.to_s, self) if serializer_const.nil?

raise "Can't find serializer for #{self.name}.#{name} has_many relationship." if serializer_const.nil?

Expand Down
43 changes: 28 additions & 15 deletions lib/panko/serializer_resolver.rb
Original file line number Diff line number Diff line change
@@ -1,25 +1,38 @@
# frozen_string_literal: true

require 'active_support/core_ext/string/inflections'
require 'active_support/core_ext/module/introspection'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the two AS features used by this file.


class Panko::SerializerResolver
def self.resolve(name)
serializer_name = "#{name.singularize.camelize}Serializer"
serializer_const = safe_const_get(serializer_name)
class << self
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liverty to convert to class << self, as it makes it easier to have private class method. Previously is_serializer and safe_const_get were actually public.

def resolve(name, from)
serializer_const = nil

return nil if serializer_const.nil?
return nil unless is_serializer(serializer_const)
if namespace = namespace_for(from)
serializer_const = safe_serializer_get("#{namespace}::#{name.singularize.camelize}Serializer")
end

serializer_const
end
serializer_const ||= safe_serializer_get("#{name.singularize.camelize}Serializer")
serializer_const
end

private
private

def self.is_serializer(const)
const < Panko::Serializer
end
if Module.method_defined?(:module_parent_name)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rails 6.0 renamed Module#parent_name into Module#module_parent_name, so to support Rails 5.2 we need to do some feature checking.

def namespace_for(from)
from.module_parent_name
end
else
def namespace_for(from)
from.parent_name
end
end

def self.safe_const_get(name)
Object.const_get(name)
rescue NameError
nil
def safe_serializer_get(name)
const = Object.const_get(name)
const < Panko::Serializer ? const : nil
rescue NameError
nil
end
end
end
1 change: 1 addition & 0 deletions panko_serializer.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@ Gem::Specification.new do |spec|
spec.extensions << "ext/panko_serializer/extconf.rb"

spec.add_dependency "oj", "~> 3.10.0"
spec.add_dependency "activesupport"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the resolver uses a bunch of ActiveSupport core_ext, it's better to add it to the dependencies.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was always a question for me, whether I want a direct dependency on rails for Panko since it can serialize plain ruby objects. but to be honest with me, it's 90% of the project, so ok. 👍

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you, we can also try to eliminate the dependency.

camelize for instance is very easy to get rid of, singularize however it's quite trickier.

But maybe there's a way to have this code only called in Rails context? I don't know.

end
30 changes: 25 additions & 5 deletions spec/panko/serializer_resolver_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
class CoolSerializer < Panko::Serializer
end

result = Panko::SerializerResolver.resolve("cool")
result = Panko::SerializerResolver.resolve("cool", Object)

expect(result._descriptor).to be_a(Panko::SerializationDescriptor)
expect(result._descriptor.type).to eq(CoolSerializer)
Expand All @@ -17,7 +17,7 @@ class CoolSerializer < Panko::Serializer
class PersonSerializer < Panko::Serializer
end

result = Panko::SerializerResolver.resolve("persons")
result = Panko::SerializerResolver.resolve("persons", Object)

expect(result._descriptor).to be_a(Panko::SerializationDescriptor)
expect(result._descriptor.type).to eq(PersonSerializer)
Expand All @@ -27,22 +27,42 @@ class PersonSerializer < Panko::Serializer
class MyCoolSerializer < Panko::Serializer
end

result = Panko::SerializerResolver.resolve("my_cool")
result = Panko::SerializerResolver.resolve("my_cool", Object)

expect(result._descriptor).to be_a(Panko::SerializationDescriptor)
expect(result._descriptor.type).to eq(MyCoolSerializer)
end

it "resolves serializer in namespace first" do
class CoolSerializer < Panko::Serializer
end
module MyApp
class CoolSerializer < Panko::Serializer
end

class PersonSerializer < Panko::Serializer
end
end

result = Panko::SerializerResolver.resolve("cool", MyApp::PersonSerializer)
expect(result._descriptor).to be_a(Panko::SerializationDescriptor)
expect(result._descriptor.type).to eq(MyApp::CoolSerializer)

result = Panko::SerializerResolver.resolve("cool", Panko)
expect(result._descriptor).to be_a(Panko::SerializationDescriptor)
expect(result._descriptor.type).to eq(CoolSerializer)
end

describe "errors cases" do
it "returns nil when the serializer name can't be found" do
expect(Panko::SerializerResolver.resolve("post")).to be_nil
expect(Panko::SerializerResolver.resolve("post", Object)).to be_nil
end

it "returns nil when the serializer is not Panko::Serializer" do
class SomeObjectSerializer
end

expect(Panko::SerializerResolver.resolve("some_object")).to be_nil
expect(Panko::SerializerResolver.resolve("some_object", Object)).to be_nil
end
end
end