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

getRef method bug #794

Closed
hipporello opened this issue Apr 25, 2020 · 7 comments · Fixed by #822
Closed

getRef method bug #794

hipporello opened this issue Apr 25, 2020 · 7 comments · Fixed by #822

Comments

@hipporello
Copy link

hipporello commented Apr 25, 2020

Describe the bug
gitRef method uses wrong api end point. It causes a deserialization problem. It should use git/ref/%d instead of git/ref/%s.

@bitwiseman
Copy link
Member

@hipporello
Which class's getRef() method? Please provide more details.

@hipporello
Copy link
Author

hipporello commented Apr 30, 2020

Hi,

It is GHRepository class and the method signature is:

public GHRef getRef(String refName) throws IOException {

@bitwiseman
Copy link
Member

@hipporello
Why should it use git/ref/%d?
Please provide and example where git/ref/%s does not work correctly.

@hipporello
Copy link
Author

hipporello commented May 13, 2020

Hi,

You are using git/refs/%s currently for getRef method of GHRepository. This brings all matching references instead of a single ref value which I expect getRef should return. Instead of git/refs/%s, you should use git/ref/%s. This will bring exactly specified reference.

@bitwiseman
Copy link
Member

bitwiseman commented May 13, 2020

So, the problem isn't %s but rather refs vs ref? I see.

I agree with your point and that this is bug. I'm somewhat concerned that fixing this might cause a significant behavior change.

@bitwiseman
Copy link
Member

bitwiseman commented May 14, 2020

Having looked a bit more into this, the library has a getRefs() and a listRefs() methods (which as expected call into the git/refs endpoint). If someone expected something other than exactly one GHRef they would use those methods. So, at first glance it seems like this will produce some kind of change in behavior, but it is probably a good change to make.

Inputs

Let's say we have the following input ref path specifiers:

  • heads/gh-pages
  • heads/gh
  • headz

Questions

  • What is currently returned from GitHub API git/ref vs. git/refs for each input?
  • What behavior does each input currently produce from this library when getRef() is called?
  • How does the behavior change if we change from git/refs/%s to git/ref/%s?

GitHub API git/ref vs. git/refs

Here are four queries and the response from them:

git/refs/heads/gh-pages

{
  "ref": "refs/heads/gh-pages",
  "node_id": "MDM6UmVmNjE3MjEwOmdoLXBhZ2Vz",
  "url": "https://api.github.com/repos/hub4j/github-api/git/refs/heads/gh-pages",
  "object": {
    "sha": "bf902ab5c8aade1e5cbefa6db4cf39f5f3692552",
    "type": "commit",
    "url": "https://api.github.com/repos/hub4j/github-api/git/commits/bf902ab5c8aade1e5cbefa6db4cf39f5f3692552"
  }
}

git/refs/heads/gh

This is surprising. We get this same output for git/refs/heads and even git/refs/hea.

[
  {
    "ref": "refs/heads/gh-pages",
    "node_id": "MDM6UmVmNjE3MjEwOmdoLXBhZ2Vz",
    "url": "https://api.github.com/repos/hub4j/github-api/git/refs/heads/gh-pages",
    "object": {
      "sha": "bf902ab5c8aade1e5cbefa6db4cf39f5f3692552",
      "type": "commit",
      "url": "https://api.github.com/repos/hub4j/github-api/git/commits/bf902ab5c8aade1e5cbefa6db4cf39f5f3692552"
    }
  },
  {
    "ref": "refs/heads/master",
    "node_id": "MDM6UmVmNjE3MjEwOm1hc3Rlcg==",
    "url": "https://api.github.com/repos/hub4j/github-api/git/refs/heads/master",
    "object": {
      "sha": "dbd20fe396e13f7f6b7de5d6dfcbd9e6d8775129",
      "type": "commit",
      "url": "https://api.github.com/repos/hub4j/github-api/git/commits/dbd20fe396e13f7f6b7de5d6dfcbd9e6d8775129"
    }
  }
]

git/refs/headz

{
  "message": "Not Found",
  "documentation_url": "https://developer.github.com/v3/git/refs/#get-a-reference"
}

git/ref/heads/gh-pages

Identical to git/refs/heads/gh-pages above.

git/ref/heads/gh

{
  "message": "Not Found",
  "documentation_url": "https://developer.github.com/v3/git/refs/#get-a-single-reference"
}

git/ref/headz

{
  "message": "Not Found",
  "documentation_url": "https://developer.github.com/v3/git/refs/#get-a-single-reference"
}

Current github-api behavior using git/refs

TODO: Write tests verifying the demonstrating the current behavior.

New github-api behavior using git/ref

TODO: Update product code and modify tests to match the new behavior.

bitwiseman added a commit to bitwiseman/github-api that referenced this issue May 21, 2020
bitwiseman added a commit to bitwiseman/github-api that referenced this issue May 21, 2020
bitwiseman added a commit to bitwiseman/github-api that referenced this issue May 21, 2020
@bitwiseman bitwiseman reopened this Jun 11, 2020
@bitwiseman
Copy link
Member

Turns out there's a reason why this was not done: it breaks GHE 2.18 and earlier, possibly later as well.

bitwiseman added a commit to bitwiseman/github-api that referenced this issue Jun 11, 2020
bitwiseman added a commit to bitwiseman/github-api that referenced this issue Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants