[OpenAPI]: Extend @response / @request tag syntax to accept serializer options

[Abishekcs]

What this PR does

Extends the tag parser to recognize serializer options like (Few Examples) :
a) UserBlueprint
b) UserBlueprint(view: :extended)
c) UserBlueprint(view: :extended, root: :user)
d) Array<UserBlueprint>
e) Array<UserBlueprint(view: :extended, root: :user)
... .

As, recmoneded in the issue description changes have been made in a way so Alba and any future serializer can use the same syntax.

References

• Blueprinter Usage Documentation.

AI usage

• Used AI for generating the the test cases. I also went through all the test cases that was generated by the AI just to be sure it's okay.

[rsamoilov]

Can we substitute this logic with YAML parsing? For example:

Suggested change

	key, value = part.split(":", 2).map(&:strip)
	next unless key
	if value.nil?
	hash[key.to_sym] = nil
	elsif value.start_with?(":")
	hash[key.to_sym] = value[1..].to_sym
	elsif value.start_with?('"') && value.end_with?('"')
	hash[key.to_sym] = value[1..-2]
	elsif value == "true"
	hash[key.to_sym] = true
	elsif value == "false"
	hash[key.to_sym] = false
	else
	hash[key.to_sym] = value
	end
	option = YAML.load(part)
	return nil unless option.is_a?(Hash) # TODO: handle the nil response in `OpenAPI::Parser`
	hash.merge!(option.transform_keys!(&:to_sym))

[Abishekcs]

Okay, I didn't knew about YAML.load ... 😅 make things much simpler. Thanks.

[rsamoilov]

If you check the existing parsers, you'll see that each of them internally calls the similar __try_parse_collection call.

This creates a fair amount of duplication, but this is by design - parsers are intended to be independent systems that receive raw response tags and decide what to do with them.

That being said, let's move this call inside the Blueprinter parser. It's currently the only one using these options, which will also allow us to leave out the ** signature updates for parsers that don't support options. This will also allow the parsers to correctly issue a warning if someone passes options to the parser that doesn't support them.

[Abishekcs]

Got it.

[rsamoilov]

What about args instead of options?

Suggested change

	def self.__parse_serializer_options(str)
	def self.__parse_serializer_args(str)

[Abishekcs]

Hi @rsamoilov can you review the changes again whenever you're free. Thank you! Here's a quick summary:

• Moved the .__parse_serializer_args call (previously .__parse_serializer_options) from parser.rb into blueprinter.rb
• Removed ** from methods where it was added
• .__parse_serializer_args (previously .__parse_serializer_options) used to return [str, options], now changed to return [is_collection, klass_str, options] where klass_str is the clean serializer class name stripped of any collection syntax or options
• Updated test case.

[rsamoilov]

Hey @Abishekcs,

This looks really nice.

I left a couple of minor suggestions - one to add a YARD comment and another to update the name of the variable.

There're also multiple statements not covered with tests. Let's either remove them if the situations they guard against can't happen or add tests to cover them.

[rsamoilov]

Suggested change

	_, clean_inner, options = __parse_serializer_args(inner)
	if options.any?
	[is_collection, clean_inner, options]
	_, clean_inner, args = __parse_serializer_args(inner)
	if args.any?
	[is_collection, clean_inner, args]

[Abishekcs]

Done

[rsamoilov]

Suggested change

	# @private
	def self.__parse_serializer_args(str)
	# @private
	# @return [Array<Boolean, String, Hash>] a tuple of (is_collection, serializer, args)
	def self.__parse_serializer_args(str)

[Abishekcs]

Done

[rsamoilov]

This branch is not covered with tests.

[Abishekcs]

It's covered by this two test (Line 94) and (Line 99) cases:

context "with a collection using square brackets without options" do
  let(:str) { "[UserBlueprint]" }
  it { is_expected.to eq([true, "UserBlueprint", {}]) }
end

context "with a collection using Array syntax without options" do
  let(:str) { "Array<UserBlueprint>" }
  it { is_expected.to eq([true, "UserBlueprint", {}]) }
end

• https://rage-rb.dev/docs/openapi#collections

[rsamoilov]

The str.empty? clause is not covered with tests.

[Abishekcs]

This is the test case (Line 148):

context "with empty string" do
  let(:str) { "" }
  it { is_expected.to eq({}) }
end

While poking around I noticed removing str.empty? clause didn't break the test context "with empty string" because "".split(",") returns an empty array [], so each_with_object({}) just returns {} naturally without the block ever running.

So str.empty? is technically redundant, but I think keeping it will be nice since it's not immediately obvious that "".split(",").each_with_object({}) returns {} , the explicit check makes the intent clearer for anyone reading the code later.

[Abishekcs]

Does this make sense😅 ? or should i remove str.empty?

[rsamoilov]

This condition is not covered with tests.

[Abishekcs]

Added a new test case for this (Line 183)

context "with an invalid option (not a key value pair)" do
  let(:str) { "extended" }
    it { is_expected.to be_nil }
  end

[Abishekcs]

There're also multiple statements not covered with tests. Let's either remove them if the situations they guard against can't happen or add tests to cover them.

I have to check this one to be sure. Thanks for the review.

[roman-softserve]

Looks great!