facebookarchive / swift

An annotation-based Java library for creating Thrift serializable types and services.
Apache License 2.0
900 stars 297 forks source link

Add checks to verify that we don't have multiple ways to inject field values #131

Closed andrewcox closed 6 years ago

andrewcox commented 11 years ago

As of a few days ago, if you add a @ThriftField-annotated public field, a @ThriftConstructor constructor with a @ThriftField-annotated parameter for setting that same field, and a @ThriftField-annotated setter method for setting that field, swift will call ALL of them when reading the type off the wire (assuming that field is read from the wire).

We should add checks to the metadata builders that there is only one annotated way to set each field. It's still ok for a class to have a two ways to actually set the method, as long as they aren't annotated.

dain commented 11 years ago

That was as an intentional feature, much like guice allows you to inject the same thing multiple times. Is there a case where this causes problems for you?

andrewcox commented 11 years ago

It isnt a big problem, just means the reader is doing a little more work for a field than it has to do. If there are legitimate cases maybe I can make it just a warning we log somewhere.

I've seen where you have inject constructors and also inject setters, but usually for setting up different bits of data. What's an example where you'd need to pass in an a field to the ctor and also call the setter for that field, when reading it in from a message?

On Sep 18, 2013, at 11:18, "Dain Sundstrom" notifications@github.com<mailto:notifications@github.com> wrote:

That was as an intentional feature, much like guice allows you to inject the same thing multiple times. Is there a case where this causes problems for you?

— Reply to this email directly or view it on GitHubhttps://urldefense.proofpoint.com/v1/url?u=https://github.com/facebook/swift/issues/131%23issuecomment-24686968&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=sJ61cr6u3yYT6%2FsOZjgMEw%3D%3D%0A&m=kUzsJFUMgtEWZUBetTTlfJPn3NPGpDqmoNCB76b6BHU%3D%0A&s=cbb7bdc9166af0c14ccbbf72b5bd8a91c4dcbcfc3dd4f00726e9dd77a33e13e1.

ghost commented 9 years ago

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.