Add statistics API. #477

martinvanzijl · 2018-12-02T18:26:05Z

Fixes #330 - Implements the statistics API as per https://developer.github.com/v3/repos/statistics/.
Adds the following to the GHRepository class:

getContributorStats()
getCommitActivity()
getCodeFrequency()
getParticipation()
getPunchCard()

Fixes issue hub4j#330

bitwiseman

Cool! I haven't looked at the code yet, but can you add some tests? That would help with reviewing.

martinvanzijl · 2019-08-12T00:11:53Z

@bitwiseman Thanks for your comment. I apologize for taking so long to respond. (I've updated my Github notification settings now.)

Do you mean unit tests?

bitwiseman · 2019-09-11T00:21:11Z

@martinvanzijl
My turn to be slow to respond. :)
Yes, I unit tests. We now have the start of WireMock based framework where you can make sure the values from the server are being processed correctly.

Fixed issue with getCodeFrequency() where it would occasionally throw an exception when the statistics were still being generated. Added comment about throwing an exception as a possibility when 202 is returned.

martinvanzijl · 2019-09-14T03:43:34Z

@bitwiseman I've added some unit tests (and pushed a fix). So far the tests just check that the methods run successfully. I'll aim to add more detailed tests to see that the statistics returned are correct.

Should I be using WireMock for these?

Added another accessor method.

martinvanzijl · 2019-09-20T17:51:00Z

@bitwiseman I've added more detail to the unit tests. Could you please review?

GitHub caches statistics, and if they are out of date it will return "202" while it refreshes them. I return "null" in that case, but I could instead throw an exception, or simply call sleep() and try again in five seconds within the method itself. What do you think?

bitwiseman · 2019-09-24T21:40:23Z

@martinvanzijl
Please use the WireMock testing utility, similar to the GistTest or PullRequestTest. This will let us run tests in CI.

I've also add you as a maintainer on
https://github.com/github-api-test-org/github-api . Please point your tests at that repository if possible, and if not let's discuss if there is anything else you need.

bitwiseman · 2019-09-24T21:42:48Z

@martinvanzijl

GitHub caches statistics, and if they are out of date it will return "202" while it refreshes them. I return "null" in that case, but I could instead throw an exception, or simply call sleep() and try again in five seconds within the method itself. What do you think?

I'm not sure, I'm not familiar with this part of the code yet.

bitwiseman

Now that I've had a chance to look at it, here's what I think needs to happen:

Move your added code from GHRepository to a new class called GHRepositoryStatistics. GHRepository will have a method called getStatistics(). This will keep these classes separate from the rest of GHRepository for clarity.

GHRepositoryStatistics will cache the results of any query locally - see the GHPullRequest class and how it handles merge state. The methods can still return null, but once one gets a non-null value it keeps them until you get a new instance of GHRepositoryStatistics.

This seems like a reasonable balance to me. I'm open to alternatives.

martinvanzijl · 2019-09-26T03:00:06Z

@bitwiseman Thanks, makes sense. I'll try to make the changes as suggested.

- Moved statistics methods from GHRepository to new class GHRepositoryStatistics - Updated StatisticsTest to suit. - For ContributorStats, put the "wait-till-ready" loop in the accessor method. I'm planning to do something similar for the rest.

martinvanzijl · 2019-10-04T03:40:48Z

I've uploaded my progress so far. I moved the code to a new class GHRepositoryStatistics as suggested, and used the Wiremock framework.

There is more I would like to do:

getContributorStats() has the "wait-till-ready" loop in the method itself, which can be turned off with a boolean parameter. Do the same for the other "get..." methods.
Return classes with accessor methods (e.g. ContributorStats.getAuthor("name")) instead of just the PagedIterables or Lists.
Cache the statistics and add a "refresh()" method to force a reload of data.

bitwiseman

@martinvanzijl
I'm happy to wait for you to make any changes you'd like. On the other hand, I'm a fan of incremental improvement. This PR has been open for months. I rather see a basic version of these changes go live and then have you iterate on them. I had a couple more tweak requests, but otherwise, It seems to me like we could merge these changes and further improvements in another PR.

Great work!

bitwiseman · 2019-10-04T07:16:35Z

src/main/java/org/kohsuke/github/GHRepositoryStatistics.java

+     */
+    @SuppressWarnings("SleepWhileInLoop")
+    @SuppressFBWarnings(value = {"RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"}, justification = "JSON API")
+    public PagedIterable<ContributorStats> getContributorStats(boolean waitTillReady) throws IOException, InterruptedException {


There's a similar need for retry loop with PullRequests and checking for whether they are mergable (and getting mergeCommitSha). Onc we publish this, we are commited to supporting it going forward, but I'm not sure yet that this is model I'd like for follow.

Would you be willing to mark this as @Preview @Deprecated? That will give the project the freedom to make breaking changes (such as moving this behavior to a common helper the does pollFor(...)).

Makes sense, I've marked it as suggested.

Should I make the default value false, to bring it in line with the other getters?

Meh. Let's leave it. It is definitely better behavior.

src/main/java/org/kohsuke/github/GHRepositoryStatistics.java

- Marked getContributorStats(bool) as deprecated, preview - Moved "stats/" string to getApiTailUrl()

Rerecorded WireMock files with newer framework

Add statistics API.

9d955d2

Fixes issue hub4j#330

bitwiseman requested changes May 17, 2019

View reviewed changes

Unit tests and fixes.

ad281ad

Fixed issue with getCodeFrequency() where it would occasionally throw an exception when the statistics were still being generated. Added comment about throwing an exception as a possibility when 202 is returned.

Added more detail to unit tests.

3452a08

Added another accessor method.

martinvanzijl requested a review from bitwiseman September 20, 2019 17:50

bitwiseman requested changes Sep 25, 2019

View reviewed changes

bitwiseman added 2 commits September 24, 2019 18:19

Merge branch 'master' into issue_330_statistics

3d04765

Merge branch 'master' into issue_330_statistics

e22c6d3

martinvanzijl added 2 commits October 4, 2019 16:23

Changes per review on GitHub.

c7af56d

- Moved statistics methods from GHRepository to new class GHRepositoryStatistics - Updated StatisticsTest to suit. - For ContributorStats, put the "wait-till-ready" loop in the accessor method. I'm planning to do something similar for the rest.

Added Wiremock test files.

bd23ba6

Merge branch 'master' into issue_330_statistics

e81e228

bitwiseman requested changes Oct 4, 2019

View reviewed changes

Updates per GH review.

cb0facc

- Marked getContributorStats(bool) as deprecated, preview - Moved "stats/" string to getApiTailUrl()

martinvanzijl requested a review from bitwiseman October 4, 2019 16:46

bitwiseman added 3 commits October 4, 2019 21:12

Merge branch 'master' into issue_330_statistics

4e56b8b

Minor fixups after merge from master

5feffdf

Rerecorded WireMock files with newer framework

Fix JSON API FB warning

f9aa1ad

bitwiseman approved these changes Oct 5, 2019

View reviewed changes

bitwiseman merged commit 7c065c1 into hub4j:master Oct 5, 2019

martinvanzijl deleted the issue_330_statistics branch October 7, 2019 00:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add statistics API. #477

Add statistics API. #477

martinvanzijl commented Dec 2, 2018

bitwiseman left a comment •

edited

Loading

martinvanzijl commented Aug 12, 2019

bitwiseman commented Sep 11, 2019

martinvanzijl commented Sep 14, 2019

martinvanzijl commented Sep 20, 2019

bitwiseman commented Sep 24, 2019

bitwiseman commented Sep 24, 2019

bitwiseman left a comment •

edited

Loading

martinvanzijl commented Sep 26, 2019

martinvanzijl commented Oct 4, 2019

bitwiseman left a comment

bitwiseman Oct 4, 2019

martinvanzijl Oct 4, 2019

bitwiseman Oct 5, 2019

Add statistics API. #477

Add statistics API. #477

Conversation

martinvanzijl commented Dec 2, 2018

bitwiseman left a comment • edited Loading

Choose a reason for hiding this comment

martinvanzijl commented Aug 12, 2019

bitwiseman commented Sep 11, 2019

martinvanzijl commented Sep 14, 2019

martinvanzijl commented Sep 20, 2019

bitwiseman commented Sep 24, 2019

bitwiseman commented Sep 24, 2019

bitwiseman left a comment • edited Loading

Choose a reason for hiding this comment

martinvanzijl commented Sep 26, 2019

martinvanzijl commented Oct 4, 2019

bitwiseman left a comment

Choose a reason for hiding this comment

bitwiseman Oct 4, 2019

Choose a reason for hiding this comment

martinvanzijl Oct 4, 2019

Choose a reason for hiding this comment

bitwiseman Oct 5, 2019

Choose a reason for hiding this comment

bitwiseman left a comment •

edited

Loading

bitwiseman left a comment •

edited

Loading