(GH-431) Add Count to List Command API #430

RichiCoder1 · 2015-09-25T18:55:01Z

Adds the ability to efficiently retrieve a "count" for a given list command. Usefully for when you want to total number of given packages for a command that meets certain search criteria, but you're also using paging and need a reliable "total". Mostly only relevant to NuGet at this point.

Closes #431

ferventcoder · 2015-09-25T19:05:07Z

src/chocolatey/infrastructure.app/nuget/NugetList.cs

+                        .Where(p => configuration.Prerelease || p.IsReleaseVersion())
+                        .distinct_last(PackageEqualityComparer.Id, PackageComparer.Version)
+                        .Count();
+        }


This method could literally call return GetPackages(config, nugetLogger).Count();. Thoughts?

If they specified paging, then Count() would be inaccurate. The though process here is you could use the same config for both List and Count, and count would return the correct result.

You could change the config momentarily. I'm more of a fan of that than I am duplicating code.

ferventcoder · 2015-09-25T19:06:10Z

This needs a ticket filed on it.

ferventcoder · 2015-09-25T19:06:37Z

So, this is interesting. Two calls out then? :/

RichiCoder1 · 2015-09-25T19:08:33Z

Only when paging is used, and when it is, Count will be called once while List will be called multiple times with given constraints.

RichiCoder1 · 2015-09-25T19:09:48Z

I also discovered that the way we handle filtering out unlisted packages is straight up wrong when dealing with paging and count. Looking into fixing that.

RichiCoder1 · 2015-09-25T19:15:28Z

Hrmm. Chocolatey's nuget is out of date, even for v2. They added and explicit switch on search to handle delisted packages.

ferventcoder · 2015-09-25T19:16:18Z

Hrmm. Chocolatey's nuget is out of date, even for v2. They added and explicit switch on search to handle delisted packages.

They also broke other things. That's why we are back on 2.8.2

RichiCoder1 · 2015-09-25T19:21:45Z

Hrmm. Chocolatey's nuget is out of date, even for v2. They added and explicit switch on search to handle delisted packages.

They also broke other things. That's why we are back on 2.8.2

Are you up to date with nuget-chocolatey too? It has the extensions I need to make this work efficiently. Failing that, I'm going to have to drop to a lower level and just build the query myself.

ferventcoder · 2015-09-25T19:24:53Z

Take a look at the 2.8.2_adds branch and submit fixes there

RichiCoder1 · 2015-09-25T19:25:16Z

Yup, just realized I was looking at the wrong branch.

RichiCoder1 · 2015-09-25T19:59:50Z

I was hoping that this wouldn't require a server change but it looks like NuGet's OData doesn't support datetime comparsions in it's IQueryable, and Chocolatey's server claims that IsListed doesn't exist. Server needs to either support an IsListed property or add isListed to it's search.

RichiCoder1 · 2015-09-25T20:23:27Z

nuget-chocolatey doesn't support searching against unlisted packages. Why was that check even there?

RichiCoder1 · 2015-09-25T20:45:06Z

So the isListed filter was completely superfluous unless I'm missing something. The results from the choco I just pushed return identical to the results from Choco v0.9.9.

Count now returns in an average of 1-2 seconds, and using Page is ever so slightly faster too.

RichiCoder1 · 2015-09-25T20:46:15Z

src/chocolatey/infrastructure.app/nuget/NugetList.cs

+
+        public static int GetCount(ChocolateyConfiguration configuration, ILogger nugetLogger)
+        {
+            return SearchPackagesImpl(configuration, nugetLogger).Count();


If you're wondering why I put Count() here, it's because I need IQueryable's Count. Otherwise it pulls down all the results, and gets the count of that.

ferventcoder · 2015-09-25T21:08:12Z

I was hoping that this wouldn't require a server change but it looks like NuGet's OData doesn't support datetime comparsions in it's IQueryable, and Chocolatey's server claims that IsListed doesn't exist. Server needs to either support an IsListed property or add isListed to it's search.

We should add it.

nuget-chocolatey doesn't support searching against unlisted packages. Why was that check even there?

I was not aware that nuget could support searching against unlisted packages. Does it allow it?

RichiCoder1 · 2015-09-26T04:11:42Z

I was not aware that nuget could support searching against unlisted packages. Does it allow it?

Nope. That was me misunderstanding something.

RichiCoder1 · 2015-09-28T07:34:20Z

@ferventcoder developed a somewhat intelligent work around. Remote servers, aka OData chocolatey servers, seem to be very uniform and with my changes produce the same results. Other sources however behave....odd. So I now check if all the repositories we're searching against are remote. If they are, I let the server handle the heavy lifting. If not, we run the blocking filters that produce the correct results. The end result is --page and Count() are blazing fast against OData Servers, and produce the correct behavior elsewhere.

ferventcoder · 2015-09-28T15:46:03Z

src/chocolatey/infrastructure.app/nuget/NugetList.cs

+            // Whether or not the package is remote determines two things:
+            // 1. Does the repository have a notion of "listed"?
+            // 2. Does it support prerelease in a straight-forward way?
+            // Choco previosly dealt with this by taking the path of least resistance and manually filtering out and sort unwanted packages


ferventcoder · 2015-09-28T15:56:53Z

Do you want to start squashing down/cleaning up your commits and adding the prepend (GH-431) and notes to the commits?

ferventcoder · 2015-09-28T15:58:02Z

Example of what I'm looking for with commit summary and message body - e15c3eb

RichiCoder1 · 2015-09-28T15:59:01Z

Yup! Wanted to get something we're happy with before cleaning up my commits :)

RichiCoder1 · 2015-09-28T16:05:58Z

Squashed.

RichiCoder1 · 2015-09-28T16:12:47Z

(and fixed type now that I think of it)

ferventcoder · 2015-09-28T16:13:49Z

I thought I saw at least two distinct commits in there. For squashing, I meant like the commit "Semicolon" and others

RichiCoder1 · 2015-09-28T16:20:13Z

Hows that?

ferventcoder · 2015-09-28T17:26:50Z

Looks good! Now if there was some integration testing surrounding this, I wouldn't feel a need to manually test it. I'll pull it in for now and we can adjust if we need to later. :)

ferventcoder · 2015-09-28T17:29:05Z

src/chocolatey/GetChocolatey.cs

@@ -222,6 +222,15 @@ public IEnumerable<T> List<T>()
            var runner = new GenericRunner();
            return runner.list<T>(configuration, _container, isConsole: false, parseArgs: null);
        }
+
+        public int Count()


Actually, the name of this throws me. It's not clear what this method returns.

This method needs some xml documentation with it as well (looks like I missed calling that out in the list command PR).

What about ListCount()?

Hrmm. Normally I'd be against it, but I list ListCount works. Or rather GetListCount()

GetListCount sounds fine to. Count isn't clear on what it is doing, that's why I was hoping for more. It would be different if you were calling GetChocolatey().Setup().List().Count

ferventcoder · 2015-10-03T20:39:29Z

Commit cleanup?

… makes sense. Updated Nuget List to attempt to use IQueryable all the way down for queries executed against service based repositories. This allows chocolatey to defer filtering, sorting, and paging to the server rather than the client. Reverts back to the old logic, though, for everything else.

Uses the IQueryable changes to add an efficient count for retrieving the number of results that would be returned by a list, usually in a much faster, more efficient way.

… and ListCount.

RichiCoder1 · 2015-10-03T22:54:01Z

Ohh, and we're all green again :D

Edit: Darn't teamcity.

(GH-431) Add Count to List Command API

ferventcoder · 2015-10-07T14:50:01Z

Thanks!

RichiCoder1 force-pushed the feature/Add_Count branch from 3042c74 to 47e2b5c Compare September 25, 2015 18:56

ferventcoder reviewed Sep 25, 2015
View reviewed changes

RichiCoder1 changed the title ~~Add Count to List Command API~~ (GH-431) Add Count to List Command API Sep 25, 2015

RichiCoder1 reviewed Sep 25, 2015
View reviewed changes

ferventcoder reviewed Sep 28, 2015
View reviewed changes

RichiCoder1 force-pushed the feature/Add_Count branch 2 times, most recently from 8d06218 to 5433f8d Compare September 28, 2015 16:05

RichiCoder1 force-pushed the feature/Add_Count branch from 5433f8d to ee2fe39 Compare September 28, 2015 16:12

RichiCoder1 force-pushed the feature/Add_Count branch from ee2fe39 to 973bfcc Compare September 28, 2015 16:19

ferventcoder reviewed Sep 28, 2015
View reviewed changes

RichiCoder1 force-pushed the feature/Add_Count branch from 6f503c9 to b27f3b0 Compare September 29, 2015 15:05

RichiCoder1 mentioned this pull request Oct 3, 2015

[API] Add the ability to retrieve package count for a Source #431

Closed

Richard Simpson added 3 commits October 3, 2015 17:45

(chocolateyGH-431) Add Count to IListCommand

3bd5d64

Uses the IQueryable changes to add an efficient count for retrieving the number of results that would be returned by a list, usually in a much faster, more efficient way.

(chocolateyGH-431) Changed Count to ListCount. Added xml docs to List…

981d1c5

… and ListCount.

RichiCoder1 force-pushed the feature/Add_Count branch from b27f3b0 to 981d1c5 Compare October 3, 2015 22:45

ferventcoder added a commit that referenced this pull request Oct 7, 2015

Merge pull request #430 from RichiCoder1/feature/Add_Count

2e72ad0

(GH-431) Add Count to List Command API

ferventcoder merged commit 2e72ad0 into chocolatey:master Oct 7, 2015

RichiCoder1 deleted the feature/Add_Count branch June 3, 2016 22:45

(GH-431) Add Count to List Command API #430

(GH-431) Add Count to List Command API #430

Conversation

RichiCoder1 commented Sep 25, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferventcoder commented Sep 25, 2015

ferventcoder commented Sep 25, 2015

RichiCoder1 commented Sep 25, 2015

RichiCoder1 commented Sep 25, 2015

RichiCoder1 commented Sep 25, 2015

ferventcoder commented Sep 25, 2015

RichiCoder1 commented Sep 25, 2015

ferventcoder commented Sep 25, 2015

RichiCoder1 commented Sep 25, 2015

RichiCoder1 commented Sep 25, 2015

RichiCoder1 commented Sep 25, 2015

RichiCoder1 commented Sep 25, 2015

Choose a reason for hiding this comment

ferventcoder commented Sep 25, 2015

RichiCoder1 commented Sep 26, 2015

RichiCoder1 commented Sep 28, 2015

Choose a reason for hiding this comment

ferventcoder commented Sep 28, 2015

ferventcoder commented Sep 28, 2015

RichiCoder1 commented Sep 28, 2015

RichiCoder1 commented Sep 28, 2015

RichiCoder1 commented Sep 28, 2015

ferventcoder commented Sep 28, 2015

RichiCoder1 commented Sep 28, 2015

ferventcoder commented Sep 28, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferventcoder commented Oct 3, 2015

RichiCoder1 commented Oct 3, 2015

ferventcoder commented Oct 7, 2015