Well, I don't see any extra advantage when using ReadPreference#[primary/secondary/nearest/etc.] utilities compared to ReadPreference#valueOf() as:
- ReadPreference#valueOf() is also part of the public API, the same as ReadPreference#[primary/secondary/nearest/etc.] API - ReadPreference#[primary/secondary/nearest/etc.] API also lacks a proper Javadoc :( The patch has been already applied to the branches and the documentation is now reflecting this fix. Would you please mind to take a look at another ticket I've raised as well and maybe comment directly into that ticket? https://issues.apache.org/jira/browse/CAMEL-7370 Also feel free to mark it as "Won't Fix" if you've doubts about that. Thanks! Babak Raul Kripalani wrote > Hi Babak, > > You're right. On revisiting this logic it seems like this functionality > was > indeed unfinished. Even in the case where the Read Preference could be > resolved, we would still throw an exception. > I don't think there's much discussion here. Backwards-compatibility is not > beneficial here because it means non-working functionality. I highly doubt > anybody is using this functionality of the component. > > Therefore +0.5 to your change. The reason I don't go full +1 is because > the > Javadoc does not make any statements regarding how the > ReadPreference#valueOf(String) method actually resolves the read > preference. And ReadPreference is not an Enum, so it's not like the > behaviour is defined by the Java Specs anyway. They try to mimic Enums > without explaining it. > > In a nutshell: by using this method, we make ourselves fragile to > undocumented implementation changes inside that method. > > An alternative approach would be to use reflection and invoke the > appropriate ReadPreference#[primary/nearest/etc.] method to obtain the > ReadPreference. At least that's part of the public API and implicitly > documented by Javadocs, so it cannot be such an easy target for future > sneaky changes by the Mongo team. > > Regards, > > *Raúl Kripalani* > Apache Camel PMC Member & Committer | Enterprise Architect, Open Source > Integration specialist > http://about.me/raulkripalani | http://www.linkedin.com/in/raulkripalani > http://blog.raulkr.net | twitter: @raulvk > > On Tue, Apr 15, 2014 at 3:17 PM, Babak Vahdat > < > babak.vahdat@ > >wrote: > >> Hi Raúl, >> >> Thanks a lot for your feedback but it's still not clear to me how this >> URI >> option used to work properly in the previous versions as from very first >> day >> (Camel 2.10.0) the preexisting logic used to throw an >> IllegalArgumentException *unconditionally* in all cases at the end of the >> method, see: >> >> >> https://github.com/apache/camel/commit/096c866aec42fec18b9eddb5809a24e27e43f902#diff-7744e9b913507ceca68769e9acd928c2R343 >> >> See also this fix from the last weekend: >> >> >> https://github.com/apache/camel/commit/d7347cec8859ef7142d5017f4d839c7fc9478ffc#diff-7744e9b913507ceca68769e9acd928c2 >> >> The reason why I'm insisting on this is because I want to understand if >> we >> would have enough good reasons to *backport* this fix (which "would >> break" >> the backward compatibility of this option, meaning the possible values to >> be >> passed for this option, e.g. "readPreference=primaryPreferred” instead of >> >> "readPreference=com.mongodb.TaggableReadPreference$PrimaryPreferredReadPreference" >> etc.). >> >> In the meanwhile I've commited the fix *only* on the master branch but >> would >> need to get this fixed on camel-2.13.x as well as 2.12.x branches. >> >> Also both Camel 2.13.x as well as 2.12.x branches make use of the driver >> version 2.11.4 which provides the ReadPreference#valueOf() utility we >> would >> need for this change, so technically the way is open for backporting the >> fix, see also this commit: >> >> >> https://github.com/mongodb/mongo-java-driver/commit/222d660e33015bce54b08845000c2ea77b944b32#diff-27c3bd4e56af669071c576ccec394864R227 >> >> Babak >> >> >> Raul Kripalani wrote >> > Hello Babak, >> > >> > Thanks for pointing this out! >> > >> > Just some background... The ReadPreference class in the MongoDB Java >> API >> > has changed substantially between the driver versions: >> > >> > * The initial contribution of camel-mongodb was done against driver >> > version >> > 2.7.3 [1]. >> > * Currently we are on driver version 2.12.0 in Camel 2.14-SNAPSHOT [2]. >> > >> > The logic to resolve the Read Preference by reflection makes sense with >> > driver version 2.7.3, but not with 2.12.0. >> > >> > I do remember testing this logic manually when I first contributed the >> > component. But clearly I missed making a unit test, which would have >> > helped >> > us detect this non-backwards compatible change. >> > >> > +1 to your adjustment proposal. >> > >> > [1] http://api.mongodb.org/java/2.7.3/com/mongodb/ReadPreference.html >> > [2] http://api.mongodb.org/java/2.12/com/mongodb/ReadPreference.html >> > >> > Regards, >> > >> > *Raúl Kripalani* >> > Apache Camel PMC Member & Committer | Enterprise Architect, Open Source >> > Integration specialist >> > http://about.me/raulkripalani | >> http://www.linkedin.com/in/raulkripalani >> > http://blog.raulkr.net | twitter: @raulvk >> > >> > On Tue, Apr 15, 2014 at 10:33 AM, Babak Vahdat >> > < >> >> > babak.vahdat@ >> >> > >wrote: >> > >> >> Hi >> >> >> >> There was a flaw by MongoDbEndpoint#setReadPreference() I tried to fix >> >> last >> >> week. See also the documentation about the readPreference option: >> >> >> >> https://camel.apache.org/mongodb.html >> >> >> >> In the meanwhile I've noticed that this option doesn't work *at all* >> >> anyway >> >> because: >> >> >> >> - For example if you try to use >> >> "readPreference=com.mongodb.ReadPreference$PrimaryReadPreference” the >> >> reflection hack will not work (with the current logic) as the static >> >> member >> >> class PrimaryReadPreference is private anyway (one would end up with >> >> IllegalAccessException etc.) >> >> - One can't for example make use of the value >> >> "com.mongodb.ReadPreference$TaggedReadPreference" (as the >> documentation >> >> says) because this preference has no default constructor! >> >> >> >> We also don't have any test case in the code base about this option. >> >> >> >> As this option doesn't work anyway, my suggestion is to change the >> >> possible >> >> values for this option to the ones >> com.mongodb.ReadPreference#valueOf() >> >> allows. I've raised a JIRA including a proposed fix for this: >> >> >> >> https://issues.apache.org/jira/browse/CAMEL-7369 >> >> >> >> I also think an option value like "readPreference=primaryPreferred" >> would >> >> read much easier than >> >> >> >> >> "readPreference=com.mongodb.TaggableReadPreference$PrimaryPreferredReadPreference" >> >> etc. >> >> >> >> The price we would pay for this would be non-backward compatibility >> with >> >> the >> >> current behaviour but as the current implementation doesn't work >> anyway, >> >> I >> >> guess this's not an issue at all. >> >> >> >> Thoughts? >> >> >> >> Babak >> >> >> >> >> >> >> >> -- >> >> View this message in context: >> >> >> http://camel.465427.n5.nabble.com/About-a-bug-of-the-camel-mongodb-component-tp5750234.html >> >> Sent from the Camel Development mailing list archive at Nabble.com. >> >> >> >> >> >> >> >> -- >> View this message in context: >> http://camel.465427.n5.nabble.com/About-a-bug-of-the-camel-mongodb-component-tp5750234p5750247.html >> Sent from the Camel Development mailing list archive at Nabble.com. >> -- View this message in context: http://camel.465427.n5.nabble.com/About-a-bug-of-the-camel-mongodb-component-tp5750234p5750268.html Sent from the Camel Development mailing list archive at Nabble.com.
