Fix IPAddr prefix information missing when write to cache in msgpack serializer

[r-plus]

Motivation / Background

This Pull Request has been created because we notice the msgpack serializer missing the prefix(netmask) data of IPAddr object when dump.

Detail

This Pull Request fixed the missing prefix(netmask) data of IPAddr object when dumping by msgpack serializer.

Additional information

to_s is not contains prefix(netmask) data.

 IPAddr.new('192.168.0.1/24').to_s
=> "192.168.0.0"

So to_s missing the prefix(netmask) data when dump/load by msgpack serializer.

irb(main):032> serializer = ActiveSupport::MessagePack::CacheSerializer
irb(main):033> serializer.load(serializer.dump(IPAddr.new('192.168.0.1/24')))
=> #<IPAddr: IPv4:192.168.0.0/255.255.255.255>

IPAddr object assertion

IPAddr instance equal assertion is not accurate to assert same data. This is reason why I use inspect for test.

IPAddr.new('192.168.0.1/24') == IPAddr.new('192.168.0.0/32')
=> true
irb(main):027> IPAddr.new('192.168.0.1/24').inspect == IPAddr.new('192.168.0.0').inspect
=> false

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
• Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
• Tests are added or updated if you fix a bug or add a feature.
• CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

[byroot]

The problem is that we can't just change the serialization of a type now that the first version was released, as previous payload wouldn't be readable, and old code wouldn't be able to read new payload (cc @jonathanhefner that's the kind of stuff I was worried about :/)

So now we need to define a new IPAddr type, and conditionally enable it, like we do in Paquito.

[r-plus]

Ah, you are right. We should consider backward compatibility.

[r-plus]

write the single string "#{ipaddr.to_s}/#{ipaddr.prefix}" as single packer write, and unpacker back to :new.
I think this way would keep the compatibility, but what do you think?

[r-plus]

okey, did it without deprecation system.
Fortunately, IPAddr.new can be compatible with or without prefixes(netmask).

[jonathanhefner]

@byroot As @r-plus mentioned, a new type isn't necessary in this case.

[byroot]

I see it's not needed for backward compatibility, but what about forward compatibility? We unfortunately need both to allow for smooth upgrades.

[byroot]

Nevermind, I see the unpack is unchanged, so we're good.

[jonathanhefner]

Thank you for the pull request!

The current behavior was derived from IPAddr#as_json, which also does not account for prefix:

rails/activesupport/lib/active_support/core_ext/object/json.rb

Lines 236 to 240 in 0ecfbe2

	class IPAddr # :nodoc:
	def as_json(options = nil)
	to_s
	end
	end

However, MessagePack can roundtrip IPAddr whereas JSON can't, so I think it's reasonable to support prefix. 👍

Also, would you mind adding a CHANGELOG entry?

[r-plus]

@jonathanhefner
Thanks for your review, I taken the suggestions and add the CHANGELOG.

[jonathanhefner]

I pushed a commit to tweak the CHANGELOG entry a bit and to squash the commits.

Thank you, @r-plus! 📦