You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@camel.apache.org by Babak Vahdat <ba...@swissonline.ch> on 2014/04/15 11:33:46 UTC
About a bug of the camel-mongodb component...
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.
Re: About a bug of the camel-mongodb component...
Posted by Babak Vahdat <ba...@swissonline.ch>.
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.
Re: About a bug of the camel-mongodb component...
Posted by Babak Vahdat <ba...@swissonline.ch>.
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.
Re: About a bug of the camel-mongodb component...
Posted by Raul Kripalani <ra...@evosent.com>.
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
<ba...@swissonline.ch>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.
>
Re: About a bug of the camel-mongodb component...
Posted by Babak Vahdat <ba...@swissonline.ch>.
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.
Re: About a bug of the camel-mongodb component...
Posted by Raul Kripalani <ra...@evosent.com>.
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
<ba...@swissonline.ch>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.
>