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

Add some level of synchronization to the root of the API #283

Merged
merged 3 commits into from
Sep 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/main/java/org/kohsuke/github/GHCommitPointer.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ public class GHCommitPointer {
* This points to the user who owns
* the {@link #getRepository()}.
*/
public GHUser getUser() {
public GHUser getUser() throws IOException {
if (user != null) return user.root.getUser(user.getLogin());
return user;
}

Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/kohsuke/github/GHCommitStatus.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.kohsuke.github;

import java.io.IOException;
import java.net.URL;

/**
Expand Down Expand Up @@ -45,7 +46,8 @@ public String getDescription() {
return description;
}

public GHUser getCreator() {
public GHUser getCreator() throws IOException {
if (creator != null) return creator.root.getUser(creator.getLogin());
return creator;
}

Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/kohsuke/github/GHDeployment.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package org.kohsuke.github;


import java.io.IOException;
import java.net.URL;

public class GHDeployment extends GHObject {
Expand Down Expand Up @@ -41,7 +41,8 @@ public String getPayload() {
public String getEnvironment() {
return environment;
}
public GHUser getCreator() {
public GHUser getCreator() throws IOException {
if(creator != null) return root.getUser(creator.getLogin());
return creator;
}
public String getRef() {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/kohsuke/github/GHGist.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ public class GHGist extends GHObject {
/**
* User that owns this Gist.
*/
public GHUser getOwner() {
return owner;
public GHUser getOwner() throws IOException {
return root.getUser(owner.getLogin());
}

public String getForksUrl() {
Expand Down
11 changes: 6 additions & 5 deletions src/main/java/org/kohsuke/github/GHIssue.java
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,8 @@ protected String getIssuesApiRoute() {
return "/repos/"+owner.getOwnerName()+"/"+owner.getName()+"/issues/"+number;
}

public GHUser getAssignee() {
public GHUser getAssignee() throws IOException {
if (assignee != null) return owner.root.getUser(assignee.getLogin());
return assignee;
}

Expand All @@ -299,8 +300,8 @@ public List<GHUser> getAssignees() {
/**
* User who submitted the issue.
*/
public GHUser getUser() {
return user;
public GHUser getUser() throws IOException {
return owner.root.getUser(user.getLogin());
}

/**
Expand All @@ -311,9 +312,9 @@ public GHUser getUser() {
* even for an issue that's already closed. See
* https://github.com/kohsuke/github-api/issues/60.
*/
public GHUser getClosedBy() {
public GHUser getClosedBy() throws IOException {
if(!"closed".equals(state)) return null;
if(closed_by != null) return closed_by;
if(closed_by != null) return owner.root.getUser(closed_by.getLogin());;

//TODO closed_by = owner.getIssue(number).getClosed_by();
return closed_by;
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/kohsuke/github/GHMilestone.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ public GHRepository getOwner() {
return owner;
}

public GHUser getCreator() {
return creator;
public GHUser getCreator() throws IOException {
return root.getUser(creator.getLogin());
}

public Date getDueOn() {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/kohsuke/github/GHPullRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ public String getMergeCommitSha() throws IOException {
* Depending on the original API call where this object is created, it may not contain everything.
*/
private void populate() throws IOException {
if (merged_by!=null) return; // already populated
if (mergeable_state!=null) return; // already populated
if (root.isOffline()) {
return; // cannot populate, will have to live with what we have
}
Expand Down
28 changes: 17 additions & 11 deletions src/main/java/org/kohsuke/github/GitHub.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.Map;
import java.util.Set;
import java.util.TimeZone;
import java.util.concurrent.ConcurrentHashMap;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
Expand Down Expand Up @@ -79,9 +80,10 @@ public class GitHub {
*/
/*package*/ final String encodedAuthorization;

private final Map<String,GHUser> users = new Hashtable<String, GHUser>();
private final Map<String,GHOrganization> orgs = new Hashtable<String, GHOrganization>();

private final Map<String,GHUser> users;
private final Map<String,GHOrganization> orgs;
// Cache of myself object.
private GHMyself myself;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now cache GHMySelf differently than the other users. This is vs. keeping it in the user's map. The issue is that if the first call to the user's map was for 'foouser' as a GHUser, then subsequent calls to getMsyelf() (if logged in as foouser) would always call the REST API because the type returned from the map would not be the desired type. While theoretically we could just update the map with the more specific GHMyself (since it's a subclass of GHUser), I thought it better for clarity to simply cache it separately.

private final String apiUrl;

/*package*/ final RateLimitHandler rateLimitHandler;
Expand Down Expand Up @@ -146,6 +148,8 @@ public class GitHub {
}
}

users = new ConcurrentHashMap<String, GHUser>();
orgs = new ConcurrentHashMap<String, GHOrganization>();
this.rateLimitHandler = rateLimitHandler;
this.abuseLimitHandler = abuseLimitHandler;

Expand Down Expand Up @@ -264,7 +268,7 @@ public String getApiUrl() {
/**
* Sets the custom connector used to make requests to GitHub.
*/
public void setConnector(HttpConnector connector) {
public synchronized void setConnector(HttpConnector connector) {
this.connector = connector;
}

Expand Down Expand Up @@ -357,13 +361,15 @@ public GHRateLimit rateLimit() throws IOException {
@WithBridgeMethods(GHUser.class)
public GHMyself getMyself() throws IOException {
requireCredential();
synchronized (this) {
if (this.myself != null) return myself;

GHMyself u = retrieve().to("/user", GHMyself.class);

GHMyself u = retrieve().to("/user", GHMyself.class);

u.root = this;
users.put(u.getLogin(), u);

return u;
u.root = this;
this.myself = u;
return u;
}
}

/**
Expand All @@ -379,7 +385,7 @@ public GHUser getUser(String login) throws IOException {
return u;
}


/**
* clears all cached data in order for external changes (modifications and del
*/
Expand Down