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

Add configuration with camelizing keys #160

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
8 changes: 8 additions & 0 deletions lib/panko/attribute.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,17 @@ module Panko
class Attribute
def self.create(name, alias_name: nil)
alias_name = alias_name.to_s unless alias_name.nil?
alias_name = transform_key(name.to_s) if alias_name.nil? && Panko.configuration.key_type
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not entirely sure what the expected behavior should be when an alias is provided in snake case, but the configured key type is in camel case. What do you think? My suggestion is to camelize the alias if it's provided, but I'd like to hear your thoughts.

Copy link
Author

@kortirso kortirso Sep 24, 2024

Choose a reason for hiding this comment

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

I see your point, I think, while transforming keys is general setting for all keys and aliases is specific setting, then aliases should be in priority, like developer has general settings with transforming, but for specific key he can specify alias

and if you agree with this - then no changes is required in PR

Copy link
Owner

Choose a reason for hiding this comment

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

Agree with you, so please:

Copy link
Author

Choose a reason for hiding this comment

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

I added test for using configuration with alias

Attribute.new(name.to_s, alias_name)
end

def self.transform_key(name)
Copy link
Owner

Choose a reason for hiding this comment

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

can we handle the case where key_type is nil and just return the name as is?

Copy link
Author

Choose a reason for hiding this comment

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

and looks like I have added check before method calling if Panko.configuration.key_type, but I'll add in any case

Copy link
Author

Choose a reason for hiding this comment

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

and I think about memoization at this point, what do you think?

if developer serializes 1000 elements with the same key, no need to transform key 1000 times, 1 is enough

Copy link
Owner

Choose a reason for hiding this comment

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

  1. Why we need the if Panko.configuration.key_type in self.create , we already handle in transform_key now.
  2. and I think about memoization at this point, what do you think? - I don't think it's necessary, since attributes are creating when serializer is defined and not during serialization time, so you pay the "transformation cost" only at boot time.

case Panko.configuration.key_type
when Panko::Configuration::CAMEL_CASE_LOWER then name.camelize(:lower)
when Panko::Configuration::CAMEL_CASE then name.camelize
end
end

def ==(other)
return name.to_sym == other if other.is_a? Symbol
return name == other.name && alias_name == other.alias_name if other.is_a? Panko::Attribute
Expand Down
30 changes: 30 additions & 0 deletions lib/panko/configuration.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# frozen_string_literal: true
Copy link
Owner

@yosiat yosiat Sep 23, 2024

Choose a reason for hiding this comment

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

We should have documentation for this, can you please add?

If not, I'll add after this PR will be merged.

Copy link
Author

Choose a reason for hiding this comment

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

there is small documentation with example in lib/panko_serializer.rb how to use it

but probably you can add any documentation later

Copy link
Owner

Choose a reason for hiding this comment

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

I was talking about the documentation here: https://github.com/yosiat/panko_serializer/tree/master/docs


module Panko
class Configuration
InitializeError = Class.new(StandardError)

CAMEL_CASE_LOWER = "camelCase"
CAMEL_CASE = "CamelCase"

AVAILABLE_KEY_TYPES = [CAMEL_CASE_LOWER, CAMEL_CASE].freeze

attr_accessor :key_type

def initialize
@key_type = nil
end

def validate
validate_key_type
end

private

def validate_key_type
return unless key_type

raise InitializeError, "Invalid key type" unless AVAILABLE_KEY_TYPES.include?(key_type)
end
end
end
22 changes: 22 additions & 0 deletions lib/panko_serializer.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require "panko/version"
require "panko/configuration"
require "panko/attribute"
require "panko/association"
require "panko/serializer"
Expand All @@ -12,3 +13,24 @@
# C Extension
require "oj"
require "panko/panko_serializer"

module Panko
extend self

# Public: Configure Panko.
#
# Panko.configure do |config|
# config.key_type = 'camelCase'
# end
#
def configure
yield configuration

configuration.validate
end

# Public: Returns Panko::Configuration instance.
def configuration
@configuration ||= Configuration.new
end
end
78 changes: 78 additions & 0 deletions spec/panko/serializer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -741,4 +741,82 @@ def created_at
end
end
end

context "key_type" do
let(:configuration) { Panko::Configuration.new }
let(:data) { {"created_at" => created_at} }
let(:created_at) { "2023-04-18T09:24:41+00:00" }

before do
allow(Panko).to receive_messages(configuration: configuration)
end

context "with camelCase" do
before { configuration.key_type = "camelCase" }

context "with key_type" do
let(:serializer_class) do
Class.new(Panko::Serializer) do
attributes :created_at
end
end

it "has createdAt" do
expect(data).to serialized_as(serializer_class,
"createdAt" => created_at)
end
end

context "with key_type + method_fields" do
let(:serializer_class) do
Class.new(Panko::Serializer) do
attributes :created_at

def created_at
"2023-04-18T09:24:41+00:00"
end
end
end

it "has createdAt" do
expect(data).to serialized_as(serializer_class,
"createdAt" => created_at)
end
end
end

context "with CamelCase" do
before { configuration.key_type = "CamelCase" }

context "with key_type" do
let(:serializer_class) do
Class.new(Panko::Serializer) do
attributes :created_at
end
end

it "has CreatedAt" do
expect(data).to serialized_as(serializer_class,
"CreatedAt" => created_at)
end
end

context "with key_type + method_fields" do
let(:serializer_class) do
Class.new(Panko::Serializer) do
attributes :created_at

def created_at
"2023-04-18T09:24:41+00:00"
end
end
end

it "has CreatedAt" do
expect(data).to serialized_as(serializer_class,
"CreatedAt" => created_at)
end
end
end
end
end