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

Post only non-null values in request body for createTree #642

Merged
merged 3 commits into from
Jan 9, 2020

Conversation

asthinasthi
Copy link

@asthinasthi asthinasthi commented Dec 11, 2019

Description

Github release 2.18+ doesn't accept sha:null in the request body. This change fixes that. More details here: #636

Before submitting a PR:

We love getting PRs, but we hate asking people for the same basic changes every time.

  • Push your changes to a branch other than master. Create your PR from that branch.
  • Add JavaDocs and other comments
  • Write tests that run and pass in CI. See CONTRIBUTING.md for details on how to capture snapshot data.
  • Run mvn install site locally. This may reformat your code, commit those changes. If this command doesn't succeed, your change will not pass CI.

@bitwiseman
Copy link
Member

@asthinasthi
This is really good. Thanks! This is the behavior for GHE. Is it the same for GitHub.com?

@asthinasthi
Copy link
Author

Actually I see that we have deprecated this https://github.com/github-api/github-api/blob/d1507f26668950508e0bf242c34cdb599003991a/src/main/java/org/kohsuke/github/GHTreeBuilder.java#L111
in the latest release (I was using 1.93).

So when we use the recommended
https://github.com/github-api/github-api/blob/d1507f26668950508e0bf242c34cdb599003991a/src/main/java/org/kohsuke/github/GHTreeBuilder.java#L129


    public GHTreeBuilder add(String path, byte[] content, boolean executable) {
        try {
            String dataSha = repo.createBlob().binaryContent(content).create().getSha();
            return shaEntry(path, dataSha, executable);
        } catch (IOException e) {
            throw new GHException("Cannot create binary content of '" + path + "'", e);
        }
    }

We create a sha & then use it in the API. We never send a null sha field.

Also, we have deprecated shaEntry(path, dataSha, executable) so I think we are good.

@bitwiseman
Copy link
Member

@asthinasthi
Um, so, do you want to close this or still merge it?

@asthinasthi
Copy link
Author

We can close this. As long as I stay away from deprecated methods I should be fine.

@Xcelled
Copy link

Xcelled commented Jan 6, 2020

Can this be reopened and merged? I just got bit by this issue in deployed production code after a GH:E upgrade and it took me forever to track it down.

@bitwiseman bitwiseman reopened this Jan 7, 2020
@@ -16,8 +18,10 @@

private final List<TreeEntry> treeEntries = new ArrayList<TreeEntry>();

@JsonInclude(Include.NON_NULL)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@JsonInclude(Include.NON_NULL)
// Issue #636: Create Tree no longer accepts null value in sha field
@JsonInclude(Include.NON_NULL)

@bitwiseman bitwiseman merged commit 31d4eef into hub4j:master Jan 9, 2020
@asthinasthi asthinasthi deleted the create-tree-null-sha-field branch January 13, 2020 05:11
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

Successfully merging this pull request may close these issues.

3 participants