-
-
Notifications
You must be signed in to change notification settings - Fork 41
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 support for querying the events-log plugin #10
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
0aea9fc
initial commit of EventsLogService
opalmer 1f730e7
only strip the magic prefix if it's present
opalmer 5d8fb6b
handle response in GetEvents() rather than passing it into events.cli…
opalmer 13c9442
some minor cleanups to var names
opalmer cb50e8d
adding test for new behavior in RemoveMagicPrefixLine
opalmer eca621f
fix check for From/To
opalmer 8ec2f65
fixing lint issues
opalmer eff38ce
fix variable name which is shadowing package
opalmer d6d7c7f
adding note in README about events-log support
opalmer b48c378
add note to the FAQ about plugin support
opalmer a0e5b42
url -> requestURL so we don't shadow the url package name
opalmer 968de84
add url arguments as part of the raw query
opalmer 0c4174c
adding tests
opalmer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,147 @@ | ||
package gerrit | ||
|
||
import ( | ||
"bytes" | ||
"encoding/json" | ||
"io/ioutil" | ||
"net/url" | ||
"time" | ||
) | ||
|
||
// PatchSet contains detailed information about a specific patch set. | ||
// | ||
// Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/json.html#patchSet | ||
type PatchSet struct { | ||
Number string `json:"number"` | ||
Revision string `json:"revision"` | ||
Parents []string `json:"parents"` | ||
Ref string `json:"ref"` | ||
Uploader AccountInfo `json:"uploader"` | ||
Author AccountInfo `json:"author"` | ||
CreatedOn int `json:"createdOn"` | ||
IsDraft bool `json:"isDraft"` | ||
Kind string `json:"kind"` | ||
} | ||
|
||
// RefUpdate contains data about a reference update. | ||
// | ||
// Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/json.html#refUpdate | ||
type RefUpdate struct { | ||
OldRev string `json:"oldRev"` | ||
NewRev string `json:"newRev"` | ||
RefName string `json:"refName"` | ||
Project string `json:"project"` | ||
} | ||
|
||
// EventInfo contains information about an event emitted by Gerrit. This | ||
// structure can be used either when parsing streamed events or when reading | ||
// the output of the events-log plugin. | ||
// | ||
// Gerrit API docs: https://gerrit-review.googlesource.com/Documentation/cmd-stream-events.html#events | ||
type EventInfo struct { | ||
Type string `json:"type"` | ||
Change ChangeInfo `json:"change,omitempty"` | ||
PatchSet PatchSet `json:"patchSet,omitempty"` | ||
EventCreatedOn int `json:"eventCreatedOn,omitempty"` | ||
Reason string `json:"reason,omitempty"` | ||
Abandoner AccountInfo `json:"abandoner,omitempty"` | ||
Restorer AccountInfo `json:"restorer,omitempty"` | ||
Submitter AccountInfo `json:"submitter,omitempty"` | ||
Author AccountInfo `json:"author,omitempty"` | ||
Uploader AccountInfo `json:"uploader,omitempty"` | ||
Approvals []AccountInfo `json:"approvals,omitempty"` | ||
Comment string `json:"comment,omitempty"` | ||
Editor AccountInfo `json:"editor,omitempty"` | ||
Added []string `json:"added,omitempty"` | ||
Removed []string `json:"removed,omitempty"` | ||
Hashtags []string `json:"hashtags,omitempty"` | ||
RefUpdate RefUpdate `json:"refUpdate,omitempty"` | ||
Project string `json:"project,omitempty"` | ||
Reviewer AccountInfo `json:"reviewer,omitempty"` | ||
OldTopic string `json:"oldTopic,omitempty"` | ||
Changer AccountInfo `json:"changer,omitempty"` | ||
} | ||
|
||
// EventsLogService contains functions for querying the API provided | ||
// by the optional events-log plugin. | ||
type EventsLogService struct { | ||
client *Client | ||
} | ||
|
||
// EventsLogOptions contains options for querying events from the events-logs | ||
// plugin. | ||
type EventsLogOptions struct { | ||
From time.Time | ||
To time.Time | ||
} | ||
|
||
// getURL returns the url that should be used in the request. This will vary | ||
// depending on the options provided to GetEvents. | ||
func (events *EventsLogService) getURL(options *EventsLogOptions) (string, error) { | ||
parsed, err := url.Parse("/plugins/events-log/events/") | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
query := parsed.Query() | ||
|
||
if !options.From.IsZero() { | ||
query.Set("t1", options.From.Format("2006-01-02 15:04:05")) | ||
} | ||
|
||
if !options.To.IsZero() { | ||
query.Set("t2", options.To.Format("2006-01-02 15:04:05")) | ||
} | ||
|
||
encoded := query.Encode() | ||
if len(encoded) > 0 { | ||
parsed.RawQuery = encoded | ||
} | ||
|
||
return parsed.String(), nil | ||
} | ||
|
||
// GetEvents returns a list of events for the given input options. Use of this | ||
// function an authenticated user. | ||
// | ||
// Gerrit API docs: https://<yourserver>/plugins/events-log/Documentation/rest-api-events.html | ||
func (events *EventsLogService) GetEvents(options *EventsLogOptions) (*[]EventInfo, *Response, error) { | ||
requestURL, err := events.getURL(options) | ||
if err != nil { | ||
return nil, nil, err | ||
} | ||
|
||
request, err := events.client.NewRequest("GET", requestURL, nil) | ||
if err != nil { | ||
return nil, nil, err | ||
} | ||
|
||
// Perform the request but do not pass in a structure to unpack | ||
// the response into. The format of the response is one EventInfo | ||
// object per line so we need to manually handle the response here. | ||
response, err := events.client.Do(request, nil) | ||
if err != nil { | ||
return nil, nil, err | ||
} | ||
|
||
body, err := ioutil.ReadAll(response.Body) | ||
|
||
defer response.Body.Close() | ||
if err != nil { | ||
return nil, nil, err | ||
} | ||
|
||
eventInfo := new([]EventInfo) | ||
for _, line := range bytes.Split(body, []byte("\n")) { | ||
if len(line) > 0 { | ||
event := EventInfo{} | ||
err := json.Unmarshal(line, &event) | ||
if err != nil { | ||
return nil, nil, err | ||
} | ||
*eventInfo = append(*eventInfo, event) | ||
} | ||
} | ||
|
||
return eventInfo, response, err | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,135 @@ | ||
package gerrit_test | ||
|
||
import ( | ||
"net/http" | ||
"testing" | ||
"time" | ||
|
||
"github.com/andygrunwald/go-gerrit" | ||
) | ||
|
||
var ( | ||
fakeEvents = []byte(` | ||
{"submitter":{"name":"Foo Bar","email":"fbar@example.com","username":"fbar"},"newRev":"0000000000000000000000000000000000000000","patchSet":{"number":"1","revision":"0000000000000000000000000000000000000000","parents":["0000000000000000000000000000000000000000"],"ref":"refs/changes/1/1/1","uploader":{"name":"Foo Bar","email":"fbar@example.com","username":"fbar"},"createdOn":1470000000,"author":{"name":"Foo Bar","email":"fbar@example.com","username":"fbar"},"isDraft":false,"kind":"TRIVIAL_REBASE","sizeInsertions":10,"sizeDeletions":0},"change":{"project":"test","branch":"master","id":"Iffffffffffffffffffffffffffffffffffffffff","number":"1","subject":"subject","owner":{"name":"Foo Bar","email":"fbar@example.com","username":"fbar"},"url":"https://localhost/1","commitMessage":"commitMessage\n\nline2\n\nChange-Id: Iffffffffffffffffffffffffffffffffffffffff\n","status":"MERGED"},"type":"change-merged","eventCreatedOn":1470000000} | ||
{"author":{"name":"Foo Bar","email":"fbar@example.com","username":"fbar"},"comment":"Patch Set 1:\n\n(2 comments)\n\nSome comment","patchSet":{"number":"1","revision":"0000000000000000000000000000000000000000","parents":["0000000000000000000000000000000000000000"],"ref":"refs/changes/1/1/1","uploader":{"name":"Foo Bar","email":"fbar@example.com","username":"fbar"},"createdOn":1470000000,"author":{"name":"Foo Bar","email":"fbar@example.com","username":"fbar"},"isDraft":false,"kind":"REWORK","sizeInsertions":4,"sizeDeletions":-2},"change":{"project":"test","branch":"master","id":"Iffffffffffffffffffffffffffffffffffffffff","number":"1","subject":"subject","owner":{"name":"Foo Bar","email":"fbar@example.com","username":"fbar"},"url":"https://localhost/1","commitMessage":"commitMessage\n\nChange-Id: Iffffffffffffffffffffffffffffffffffffffff\n","status":"NEW"},"type":"comment-added","eventCreatedOn":1470000000}`) | ||
) | ||
|
||
func TestEventsLogService_GetEvents_NoDateRange(t *testing.T) { | ||
setup() | ||
defer teardown() | ||
|
||
testMux.HandleFunc("/plugins/events-log/events/", func(writer http.ResponseWriter, request *http.Request) { | ||
writer.Write(fakeEvents) | ||
}) | ||
|
||
options := &gerrit.EventsLogOptions{} | ||
events, _, err := testClient.EventsLog.GetEvents(options) | ||
if err != nil { | ||
t.Error(err) | ||
} | ||
|
||
if len(*events) != 2 { | ||
t.Error("Expected 2 events") | ||
} | ||
|
||
// Basic test | ||
for i, event := range *events { | ||
switch i { | ||
case 0: | ||
if event.Type != "change-merged" { | ||
t.Error("Expected event type to be `change-merged`") | ||
} | ||
case 1: | ||
if event.Type != "comment-added" { | ||
t.Error("Expected event type to be `comment-added`") | ||
} | ||
} | ||
} | ||
} | ||
|
||
func TestEventsLogService_GetEvents_DateRangeFromAndTo(t *testing.T) { | ||
setup() | ||
defer teardown() | ||
|
||
to := time.Now() | ||
from := to.AddDate(0, 0, -7) | ||
|
||
testMux.HandleFunc("/", func(writer http.ResponseWriter, request *http.Request) { | ||
query := request.URL.Query() | ||
|
||
fromFormat := from.Format("2006-01-02 15:04:05") | ||
if query.Get("t1") != fromFormat { | ||
t.Errorf("%s != %s", query.Get("t1"), fromFormat) | ||
} | ||
|
||
toFormat := to.Format("2006-01-02 15:04:05") | ||
if query.Get("t2") != toFormat { | ||
t.Errorf("%s != %s", query.Get("t2"), toFormat) | ||
} | ||
|
||
writer.Write(fakeEvents) | ||
}) | ||
|
||
options := &gerrit.EventsLogOptions{From: from, To: to} | ||
_, _, err := testClient.EventsLog.GetEvents(options) | ||
if err != nil { | ||
t.Error(err) | ||
} | ||
} | ||
|
||
func TestEventsLogService_GetEvents_DateRangeFromOnly(t *testing.T) { | ||
setup() | ||
defer teardown() | ||
|
||
to := time.Now() | ||
from := to.AddDate(0, 0, -7) | ||
|
||
testMux.HandleFunc("/", func(writer http.ResponseWriter, request *http.Request) { | ||
query := request.URL.Query() | ||
|
||
fromFormat := from.Format("2006-01-02 15:04:05") | ||
if query.Get("t1") != fromFormat { | ||
t.Errorf("%s != %s", query.Get("t1"), fromFormat) | ||
} | ||
|
||
if query.Get("t2") != "" { | ||
t.Error("Did not expect t2 to be set") | ||
} | ||
|
||
writer.Write(fakeEvents) | ||
}) | ||
|
||
options := &gerrit.EventsLogOptions{From: from} | ||
_, _, err := testClient.EventsLog.GetEvents(options) | ||
if err != nil { | ||
t.Error(err) | ||
} | ||
} | ||
|
||
func TestEventsLogService_GetEvents_DateRangeToOnly(t *testing.T) { | ||
setup() | ||
defer teardown() | ||
|
||
to := time.Now() | ||
|
||
testMux.HandleFunc("/", func(writer http.ResponseWriter, request *http.Request) { | ||
query := request.URL.Query() | ||
|
||
toFormat := to.Format("2006-01-02 15:04:05") | ||
if query.Get("t2") != toFormat { | ||
t.Errorf("%s != %s", query.Get("t2"), toFormat) | ||
} | ||
|
||
if query.Get("t1") != "" { | ||
t.Error("Did not expect t1 to be set") | ||
} | ||
|
||
writer.Write(fakeEvents) | ||
}) | ||
|
||
options := &gerrit.EventsLogOptions{To: to} | ||
_, _, err := testClient.EventsLog.GetEvents(options) | ||
if err != nil { | ||
t.Error(err) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code can be significantly simplified without changing behavior.
If body has
")]}'\n"
prefix, there's no need to usebytes.IndexByte
to find the first'\n'
character, it's known to be at index 4 becausebytes.HasPrefix(body, []byte(")]}'\n"))
was true.Also, since this is called often, it's probably a good idea to factor out
[]byte(")]}'\n")
into a single package scope variable, instead of potentially allocating once perRemoveMagicPrefixLine
call.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shurcooL, no issues here if you want to go ahead and make the changes. I'd do it myself but I don't currently have my local host setup to make the change and test it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@opalmer I already made a PR that applies this suggestion, see #36.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah my bad sorry about that. Reviewed, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks both of you @opalmer and @shurcooL