Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better support for CouchDB instances requiring authentication #1

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

cdybedahl
Copy link

Hello. Putting authentication data directly into the URL doesn't work, and building your own UA object with login credentials is a bit convoluted, so I added a way to specify login info when creating a CouchDB::Client object.

There is documentation but no tests, since I couldn't figure out a way to do tests that require significant changes in the configuration of the CouchDB instance the tests are run against.

@maverick
Copy link
Owner

maverick commented Dec 8, 2010

Sorry for the slow reply,

I can't think of an elegant way of testing the auth either. Something similar to what Apache::Test does would solve the problem, but that solution is rather complex and potentially error prone getting it to work on every platform.

Someone else has suggested fixing the numeric bug in fixViewArgs by using a better regexp (https://github.com/igit/couchdb-client/commit/8141625fff7d3910ede8284f314b7f21b4a8e50a). Your solution is very interesting, I've never seen someone use B to test scalars in this manner. Why did you take this approach? Is there a gotcha with trying to use regular expressions to solve this problem?

@igit
Copy link

igit commented Dec 8, 2010

Hi all,

maverick I've read your comment #2 (comment) and the one about cdybedahl's solution (B module). For me the use of B module is "too much". Maybe we could find a solution between my "simple" regexp and the big B module.

I've (re)read the perl documentation about scalars (http://perldoc.perl.org/perldata.html#Scalar-values), and I found some usefull informations : "... At other times, you might prefer to determine whether string data can be used numerically by calling the POSIX::strtod() function or by inspecting your string with a regular expression (as documented in perlre). ...". So why not use a good regexp as documented or POSIX::strtod() ?

Would you like me to test something ?

@cdybedahl
Copy link
Author

The issue here is not if a scalar value can be used numerically or not. The issue is that on the CouchDB side, numbers and strings are not the same. To CouchDB, 7 and "7" are two different keys. Perl, on the other hand, goes to great length to treat them as the same, and no regexp no matter how clever will ever be able to tell the difference. So in order to be able to fully communicate with CouchDB, we need some way in the Perl code to specify if we want something to be treated as a number or a string. Since this goes against Perl's nature, it takes some trickery. By using the introspective B modules, we can look at the internal flags Perl uses to keep track of if a value was last used as a number or a string, and use that to tell the difference. We can then write things like 0+$a or ''.$a to force number-ness or string-ness, and CouchDB::Client will translate it to the proper JSON number or string.

@cdybedahl
Copy link
Author

Also, I made this pull request before understanding how Github pull requests work. If you look at my fork of the code, all my various changes are separated out into their own branches instead of all jumbled up together as they are here. You may want to pull things from there instead, if you chose to accept them.

Modified _is_currently_numeric to be called like a method and to
explicitly return 0 and 1;

Added tests to validate its behavior.
@maverick
Copy link
Owner

maverick commented Dec 9, 2010

cdybedahl, I suspected that something like that might have been the cause. I modified your is_currently_numeric function to behave like a method to match everything else, and I added some tests for it. One thing I noticed is that if you have a scalar that only contains digits, '' . it, and then ++ it; is_currently_numeric returns false. However, if you do the same thing with a scalar that looks like a floating point it returns true.

I'm not sure if this means that there is another condition that needs to be accounted for in that method, or if that's just how Perl behaves. I've pushed your patches and my tweak into a branch in my repository so you can take a look.

@igit
Copy link

igit commented Dec 10, 2010

My 2 cents, after reading yours comments : I agree with cdybedahl's solution about his is_currently_numeric method. I'm going to patch with this code 85dc654 and continue to test.

Dear maverick, you can close my Pull Request.

@cdybedahl
Copy link
Author

maverick: the ++ operator is (very) magical and works on both strings and numbers, so using it on a currently stringy value won't numify it. If you try -- instead, you'll see that that one does numify. That it doesn't behave the same for a float is weird but documented behaviour (magical ++ only acts on strings that consist entirely of alphanumerics, so the decimal dot forces numeric behaviour). In short, yes, it is just how Perl behaves.

I've had a look at the branch you mention, and it looks fine to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants