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

isAtKeysSizeLimit never returns true #53

Open
Ian1971 opened this issue Oct 18, 2013 · 10 comments
Open

isAtKeysSizeLimit never returns true #53

Ian1971 opened this issue Oct 18, 2013 · 10 comments

Comments

@Ian1971
Copy link

Ian1971 commented Oct 18, 2013

It seems that in the method Keys.ToString() returns the name of the type and so the length ends up being 21

       internal bool isAtKeysSizeLimit
        {
            get { return Keys != null && Keys.Any() && Keys.ToString().Length > KeysLengthLimit; }
        }
@ido-ran
Copy link
Contributor

ido-ran commented Oct 18, 2013

Hi,

I'll take a look at this again, write a unit test for it. 

Ido

On Fri, Oct 18, 2013 at 5:51 PM, Ian1971 [email protected] wrote:

It seems that in the method Keys.ToString() returns the name of the type and so the length ends up being 21

       internal bool isAtKeysSizeLimit
        {
            get { return Keys != null && Keys.Any() && Keys.ToString().Length > KeysLengthLimit; }
        }

Reply to this email directly or view it on GitHub:
#53

@soitgoes
Copy link
Owner

You rock Ido. You're going on my christmas list.

On Fri, Oct 18, 2013 at 10:12 AM, Ido Ran [email protected] wrote:

Hi,

I'll take a look at this again, write a unit test for it.

Ido

On Fri, Oct 18, 2013 at 5:51 PM, Ian1971 [email protected]
wrote:

It seems that in the method Keys.ToString() returns the name of the type
and so the length ends up being 21

internal bool isAtKeysSizeLimit
{
get { return Keys != null && Keys.Any() && Keys.ToString().Length >
KeysLengthLimit; }
}

Reply to this email directly or view it on GitHub:
#53


Reply to this email directly or view it on GitHubhttps://github.com//issues/53#issuecomment-26603603
.

Martin Murphy
Whiteboard-IT
http://whiteboard-it.com
w: (205) 588-7102

@Ian1971
Copy link
Author

Ian1971 commented Oct 18, 2013

Here you go

        [Test]
        public void Should_Use_Post_With_Many_Keys()
        {
            //this test ensures that request with lots of keys still work
            //it should switch from get to post
            var db = client.GetDatabase(baseDatabase);

            var docIds = new HashSet<string>();
            //create 1000 documents
            for (int i = 0; i < 1000; i++)
            {
                var doc = new Document<Bunny>(new Bunny());
                doc.Id = Guid.NewGuid().ToString();

                db.CreateDocument(doc);

                docIds.Add(doc.Id);
            }

            var keys = new Keys();
            keys.Values.AddRange(docIds);

            var docs = db.GetDocuments(keys);
            Assert.IsNotNull(docs, "Should be able to get many docs");

            Assert.AreEqual(1000, docs.Keys.Count());
        }

@Ian1971
Copy link
Author

Ian1971 commented Oct 18, 2013

And here is a fix, though I'm not 100% happy with it

        internal bool isAtKeysSizeLimit
        {
            get
            {
                if (Keys == null)
                    return false;

                var keyString = new string(Keys.SelectMany(k => k.ToRawString()).ToArray());
                return Keys.Any() && keyString.ToString().Length > KeysLengthLimit;
            }
        }

@Ian1971
Copy link
Author

Ian1971 commented Oct 18, 2013

I've committed it in my fork

Ian1971@71fb36e

If you prefer a pull request I can create one.

@soitgoes
Copy link
Owner

A pull request would be great.

On Fri, Oct 18, 2013 at 10:29 AM, Ian1971 [email protected] wrote:

I've committed it in my fork

Ian1971@71fb36ehttps://github.com/Ian1971/LoveSeat/commit/71fb36e66736daeecff1cf6c1d7d14ced0eebe45

If you prefer a pull request I can create one.


Reply to this email directly or view it on GitHubhttps://github.com//issues/53#issuecomment-26605041
.

Martin Murphy
Whiteboard-IT
http://whiteboard-it.com
w: (205) 588-7102

@Ian1971
Copy link
Author

Ian1971 commented Oct 18, 2013

Actually, just had a go and can't work out how to include a single commit in a pull request as it tries to add in a bunch of my other changes which you probably don't want (cloudant support)

@soitgoes
Copy link
Owner

Go ahead and send the pull request. I'll pick out the commit.

On Fri, Oct 18, 2013 at 10:43 AM, Ian1971 [email protected] wrote:

Actually, just had a go and can't work out how to include a single commit
in a pull request as it tries to add in a bunch of my other changes which
you probably don't want (cloudant support)


Reply to this email directly or view it on GitHubhttps://github.com//issues/53#issuecomment-26606160
.

Martin Murphy
Whiteboard-IT
http://whiteboard-it.com
w: (205) 588-7102

@Ian1971
Copy link
Author

Ian1971 commented Oct 18, 2013

Well, github pull request interface confused me a bit, it seems to have added the pull request into an old pull request I made a while ago. Not sure what I did wrong.

#29

@soitgoes
Copy link
Owner

i'll cherry pick some of the commits later. The web interface won't let me
cherry pick. I was hoping for that. Thanks for the submission. I'll
definitely grab that one.

On Fri, Oct 18, 2013 at 11:04 AM, Ian1971 [email protected] wrote:

Well, github pull request interface confused me a bit, it seems to have
added the pull request into an old pull request I made a while ago. Not
sure what I did wrong.

#29 #29


Reply to this email directly or view it on GitHubhttps://github.com//issues/53#issuecomment-26607873
.

Martin Murphy
Whiteboard-IT
http://whiteboard-it.com
w: (205) 588-7102

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

No branches or pull requests

3 participants