panko_serializer icon indicating copy to clipboard operation
panko_serializer copied to clipboard

Aliases for custom method

Open xchi opened this issue 5 years ago • 14 comments

We want to implement a custom method with payload key name as metricValues, but the proper ruby method name should be snake_case.

class MetricPeriodSerializer < Panko::Serializer
        attributes :period, :metricValues

        def metricValues
           [{
            'name' => 'metric1',	
            'value' => object.metric_1	
          },	
           {	
             'name' => 'metric2',	
             'value' => object.metric_2	
           }]
        end

end

We have tried the following way, but having no luck. Just wondering does panko support alias for custom method by any chance?

class MetricPeriodSerializer < Panko::Serializer
        attributes :period, :metricValues
        aliases metric_values: :metricValues

        def metric_values
           [{
            'name' => 'metric1',	
            'value' => object.metric_1	
          },	
           {	
             'name' => 'metric2',	
             'value' => object.metric_2	
           }]
        end

end

xchi avatar Feb 21 '20 03:02 xchi

Hi @xchi !

Looks like what you need is support for casing, so you can write something like this:

class MetricPeriodSerializer < Panko::Serializer
  attributes :period, :metric_values, :first_name

  def metric_values
      [{
      'name' => 'metric1',	
      'value' => object.metric_1	
    },	
      {	
        'name' => 'metric2',	
        'value' => object.metric_2	
      }]
  end
end

and the output of the keys will be period, metricValues and firstName.

Now the only question is how we want to support it, currently, Panko don't have global configuration and I want to delay it.

So we can have: specify it on the serializer level like attributes or when serializing. I prefer on the serializer level.

If this makes sense to you, let me know and I'll implement it.

yosiat avatar Feb 27 '20 23:02 yosiat

Hey @yosiat ,

It will be very helpful if you can support casing for our project, as we basically need to create aliases for almost all fields we want to parse.

On the serializer level as one attribute is fine for us as well. Thanks a lot for your replying.

Vivian

xchi avatar Feb 28 '20 04:02 xchi

@xchi let's make sure the api makes sense to you and I'll implement it.

class MetricPeriodSerializer < Panko::Serializer
  keys_format :camel_case

  attributes :period, :metric_values, :first_name

  def metric_values
      [{
      'name' => 'metric1',	
      'value' => object.metric_1	
    },	
      {	
        'name' => 'metric2',	
        'value' => object.metric_2	
      }]
  end
end

We will start by defining the serializer class and maybe in the future we will create a global configuration. Until then, you can create a base class for serializer (like ApplicationSerializer) which will have only the key_format definition.

yosiat avatar Mar 07 '20 17:03 yosiat

In my mind, the simplest and most obvious way to achieve this is to just not use snake_case in your serializers. No other code should be referencing the attribute methods in your serializer, and I think the clearest possible way of doing it, is just using the attribute names as you expect them to come out after serialization.

Also I think global configuration should not be a thing. It's just another name for a global variable that affects the result of something that was pure before.

Using a base serializer should be all that is needed to support that.

Side note @yosiat: I'd like to pick this feature up, if you disagree with my first statement and think it has value.

mikebaldry avatar Mar 09 '20 18:03 mikebaldry

@mikebaldry about global configuration, I totally agree with you.

About specifying the names of the attributes as snake case - this can be really problematic for a developer - database column names are not snake case and linters will yell that methods are not snake case.

In addition to that, think about serializer that is used for external API and some specific integration needs to be output as snake case with a subset of fields. In today's case, you need another serializer for this, in ideal place - you will re-use the same serializers and pass options for filters and key format.

I am ok with you taking this feature up (and really happy with that)! but let's make sure we are aligned on the public API for panko and let's get @xchi on board with us.

yosiat avatar Mar 09 '20 20:03 yosiat

Sure, I see your points :)

Happy to implement like the example above and we can go from there?

mikebaldry avatar Mar 09 '20 21:03 mikebaldry

@mikebaldry cool, let's go for it!

as how I see the implementation, there is no need to change something in the C-extension part (except aliases I think, need to test it). since each attribute have name_sym and name_str , the name_sym used for accessing the data and name_str for the key - see this function - https://github.com/yosiat/panko_serializer/blob/master/ext/panko_serializer/attributes_writer/common.c#L3

Please make sure that your PR includes testing for - attributes (both methods and simple attributes), aliases and associations (both has_one and has_many) and docs. If you want me to join and help with docs & tests, let me know.

yosiat avatar Mar 09 '20 21:03 yosiat

I've had a little first attempt working.. https://github.com/yosiat/panko_serializer/commit/9d352a45aab5e0fce5091d7b7b267c4436059fc5

I have a few things:

  • I just added upcase and downcase as they were simplest to write and test with, probably not any use case for them and I'll remove them when we decide on the below:
  • Adding camelcase would mean:
    • depending on ActiveSupport or something to provide camelize (not ideal)
    • writing our own camelize and friends (not exactly terribly hard, but doesn't seem the best way)
    • "passing through" the transformer - e.g. key_transformer :camelize would just call camelize on String and if it wasn't there, it would cause a NoMethodError.
  • Also supporting calling a block from C code (I haven't researched how to do this yet) like: key_transformer { |key| key[0..5] } or something.
  • key_transformer may not be a great name, I didn't think too hard, I like english though: transform_keys_with :camelize or something seems nicer to me.
  • That commit works for simplest case, but may not work with associations, as I have not written tests for that case.
  • See any problems with my C code? My Ruby C API knowledge is learned as I go (and my C is rusty from 10+ years ago mostly) :)

Many thanks

mikebaldry avatar Mar 10 '20 00:03 mikebaldry

Hey @mikebaldry ,

I had look the implementation and it seems can fix our problem. The only question I had is whether you need to provide all support for key_transformer , or this one is going to be an user defined thing?

xchi avatar Mar 12 '20 02:03 xchi

@xchi I don't know yet. Waiting on @yosiat for guidance :)

mikebaldry avatar Mar 12 '20 14:03 mikebaldry

@mikebaldry hi! sorry, another long work-week :)

If you can make a PR and it will be simpler to discuss on the code.

Some notes:

  • I don't think we need to change the C code, the descriptor already contains all attributes and associations and all of them, as far as I remember a name for serialization and name for fetching relevant data - once we serialize, as we do for the filters we should run the key transformer (in Ruby side).
  • I didn't think about key_transformer accepting a Proc, but this is really cool!
  • Camelize - I don't want a dependency on ActiveSupport, it will be too much for just two functions and I think we can use simple gems for achieving inflections - this looks really nice - https://github.com/dry-rb/dry-inflector
  • transform_keys_with sounds like a good choice, let's go with it!

yosiat avatar Mar 13 '20 15:03 yosiat

@mikebaldry hi! any update on this? can I help you?

yosiat avatar Mar 18 '20 12:03 yosiat

@yosiat Just adding +1 to the camelize support, that would be super helpful!

oyeanuj avatar Mar 21 '20 00:03 oyeanuj

Just want to check any update to this one?

xchi avatar Apr 22 '20 00:04 xchi