Hi Ràul, Now the other ticket (CAMEL-7370) has been fixed as well.
And regarding the reason why I went for ReadPreference#valueOf() instead of ReadPreference#[primary/secondary/nearest/etc.] (that's the +0.5 points you gave about it): - The logic to be implemented in this way is much easier, as it's one single line of code compared to other option which one would need to do if/elseif etc. - If the feature versions of MongoDB-Java-Driver provides more read preference options we would NOT need to change anything on our side. However using ReadPreference#[primary/secondary/nearest/etc.] would require one more "else if" block to be added while upgrading the driver ;) Thanks again for your comments on this thread! Babak Babak Vahdat wrote > 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-tp5750234p5750280.html Sent from the Camel Development mailing list archive at Nabble.com.
