search

noodlefrenzy / node-amqp10

amqp10 is a promise-based, AMQP 1.0 compliant node.js client
MIT License
134 stars 56 forks source link

refactor(CompositeType): use setters for incoming described types #262

Closed mbroadst closed 8 years ago

mbroadst commented 8 years ago

A missing piece of the type system refactor was that when described types came in, we simply set the underlaying value of the composite type to the value of the described type. This results in loss of type information, specifically related to defaults and other constraints.

This patch fixes that problem, but potentially introduces others. You might notice that some less-than-ideal changes were introduced around symbol's, and having to explicitly use ForcedType. This is because we simply can't overload the comparison operators allowing ForcedTypes to equal their (e.g.) String counterparts.

mbroadst commented 8 years ago

@noodlefrenzy @dnwe I would really appreciate a review on this, if you guys have any opinions.

mbroadst commented 8 years ago

Also FWIW, this problem was elucidated by a refactoring in Connection to store the local and remote connection parameters from open performatives. The server (qpidd in this case) was returning an open performative with null values for fields that had defaults (maxFrameSize), and yet our local copy of it maintained the null value, incorrectly.