[ARVADOS] updated: d3700e50bcaa9db177346199298913b54e37a2d5
Git user
git at public.curoverse.com
Thu Jul 13 18:04:04 EDT 2017
Summary of changes:
services/ws/session_v0.go | 20 +++--
services/ws/session_v0_test.go | 176 ++++++++++++++++++++++++++++++-----------
2 files changed, 145 insertions(+), 51 deletions(-)
via d3700e50bcaa9db177346199298913b54e37a2d5 (commit)
via 938a0cc22ad9d520c653b8b41d5b1bb2163a1020 (commit)
via 5d081c423f314060cefafc7149850ea1dcbe098a (commit)
via d1562916e4792c0a9b4b2a4aea842e7c2848d38a (commit)
via 1be8ac0f887cb1c639ac502c47438a8260735069 (commit)
via 1e7fda3fcfb9b3030c65f56e06764d20496a0610 (commit)
from 187382bb88eee65d887c004d73eddf46cbd86bc2 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit d3700e50bcaa9db177346199298913b54e37a2d5
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Jul 13 17:59:22 2017 -0400
11960: Get the full list of old IDs up front in sendOldEvents.
This avoids keeping the "get list of log IDs" database query open
while each log row gets looked up, queued, permission-checked,
serialized, and sent over the wire to the client.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curoverse.com>
diff --git a/services/ws/session_v0.go b/services/ws/session_v0.go
index db60738..58c6423 100644
--- a/services/ws/session_v0.go
+++ b/services/ws/session_v0.go
@@ -162,7 +162,7 @@ func (sub *v0subscribe) sendOldEvents(sess *v0session) {
if sub.LastLogID == 0 {
return
}
- sess.log.WithField("LastLogID", sub.LastLogID).Debug("getOldEvents")
+ sess.log.WithField("LastLogID", sub.LastLogID).Debug("sendOldEvents")
// Here we do a "select id" query and queue an event for every
// log since the given ID, then use (*event)Detail() to
// retrieve the whole row and decide whether to send it. This
@@ -177,17 +177,26 @@ func (sub *v0subscribe) sendOldEvents(sess *v0session) {
sub.LastLogID,
time.Now().UTC().Add(-10*time.Minute).Format(time.RFC3339Nano))
if err != nil {
- sess.log.WithError(err).Error("db.Query failed")
+ sess.log.WithError(err).Error("sendOldEvents db.Query failed")
return
}
- defer rows.Close()
+
+ var ids []uint64
for rows.Next() {
var id uint64
err := rows.Scan(&id)
if err != nil {
- sess.log.WithError(err).Error("row Scan failed")
+ sess.log.WithError(err).Error("sendOldEvents row Scan failed")
continue
}
+ ids = append(ids, id)
+ }
+ if err := rows.Err(); err != nil {
+ sess.log.WithError(err).Error("sendOldEvents db.Query failed")
+ }
+ rows.Close()
+
+ for _, id := range ids {
for len(sess.sendq)*2 > cap(sess.sendq) {
// Ugly... but if we fill up the whole client
// queue with a backlog of old events, a
@@ -212,9 +221,6 @@ func (sub *v0subscribe) sendOldEvents(sess *v0session) {
}
}
}
- if err := rows.Err(); err != nil {
- sess.log.WithError(err).Error("db.Query failed")
- }
}
type v0subscribe struct {
commit 938a0cc22ad9d520c653b8b41d5b1bb2163a1020
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Jul 13 11:11:14 2017 -0400
11960: Fix races in tests.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curoverse.com>
diff --git a/services/ws/session_v0_test.go b/services/ws/session_v0_test.go
index 1e6abbe..9f743e0 100644
--- a/services/ws/session_v0_test.go
+++ b/services/ws/session_v0_test.go
@@ -34,6 +34,7 @@ type v0Suite struct {
token string
toDelete []string
wg sync.WaitGroup
+ ignoreLogID uint64
}
func (s *v0Suite) SetUpTest(c *check.C) {
@@ -42,6 +43,7 @@ func (s *v0Suite) SetUpTest(c *check.C) {
s.serverSuite.srv.WaitReady()
s.token = arvadostest.ActiveToken
+ s.ignoreLogID = s.lastLogID(c)
}
func (s *v0Suite) TearDownTest(c *check.C) {
@@ -81,14 +83,40 @@ func (s *v0Suite) TestFilters(c *check.C) {
}
func (s *v0Suite) TestLastLogID(c *check.C) {
- var lastID uint64
- c.Assert(testDB().QueryRow(`SELECT MAX(id) FROM logs`).Scan(&lastID), check.IsNil)
+ lastID := s.lastLogID(c)
- conn, r, w := s.testClient()
- defer conn.Close()
+ checkLogs := func(r *json.Decoder, uuid string) {
+ for _, etype := range []string{"create", "blip", "update"} {
+ lg := s.expectLog(c, r)
+ for lg.ObjectUUID != uuid {
+ lg = s.expectLog(c, r)
+ }
+ c.Check(lg.EventType, check.Equals, etype)
+ }
+ }
- uuidChan := make(chan string, 2)
+ // Connecting connEarly (before sending the early events) lets
+ // us confirm all of the "early" events have already passed
+ // through the server.
+ connEarly, rEarly, wEarly := s.testClient()
+ defer connEarly.Close()
+ c.Check(wEarly.Encode(map[string]interface{}{
+ "method": "subscribe",
+ }), check.IsNil)
+ s.expectStatus(c, rEarly, 200)
+
+ // Send the early events.
+ uuidChan := make(chan string, 1)
s.emitEvents(uuidChan)
+ uuidEarly := <-uuidChan
+
+ // Wait for the early events to pass through.
+ checkLogs(rEarly, uuidEarly)
+
+ // Connect the client that wants to get old events via
+ // last_log_id.
+ conn, r, w := s.testClient()
+ defer conn.Close()
c.Check(w.Encode(map[string]interface{}{
"method": "subscribe",
@@ -96,31 +124,9 @@ func (s *v0Suite) TestLastLogID(c *check.C) {
}), check.IsNil)
s.expectStatus(c, r, 200)
- avoidRace := make(chan struct{}, cap(uuidChan))
- go func() {
- // When last_log_id is given, although v0session sends
- // old events in order, and sends new events in order,
- // it doesn't necessarily finish sending all old
- // events before sending any new events. To avoid
- // hitting this bug in the test, we wait for the old
- // events to arrive before emitting any new events.
- <-avoidRace
- s.emitEvents(uuidChan)
- close(uuidChan)
- }()
-
- go func() {
- for uuid := range uuidChan {
- for _, etype := range []string{"create", "blip", "update"} {
- lg := s.expectLog(c, r)
- for lg.ObjectUUID != uuid {
- lg = s.expectLog(c, r)
- }
- c.Check(lg.EventType, check.Equals, etype)
- }
- avoidRace <- struct{}{}
- }
- }()
+ checkLogs(r, uuidEarly)
+ s.emitEvents(uuidChan)
+ checkLogs(r, <-uuidChan)
}
func (s *v0Suite) TestPermission(c *check.C) {
@@ -173,6 +179,7 @@ func (s *v0Suite) TestEventTypeDelete(c *check.C) {
s.expectStatus(c, clients[i].r, 200)
}
+ s.ignoreLogID = s.lastLogID(c)
s.deleteTestObjects(c)
for _, client := range clients {
@@ -191,6 +198,7 @@ func (s *v0Suite) TestTrashedCollection(c *check.C) {
coll := &arvados.Collection{ManifestText: ""}
err := ac.RequestAndDecode(coll, "POST", "arvados/v1/collections", s.jsonBody("collection", coll), map[string]interface{}{"ensure_unique_name": true})
c.Assert(err, check.IsNil)
+ s.ignoreLogID = s.lastLogID(c)
conn, r, w := s.testClient()
defer conn.Close()
@@ -314,7 +322,9 @@ func (s *v0Suite) expectLog(c *check.C, r *json.Decoder) *arvados.Log {
lg := &arvados.Log{}
ok := make(chan struct{})
go func() {
- c.Check(r.Decode(lg), check.IsNil)
+ for lg.ID <= s.ignoreLogID {
+ c.Check(r.Decode(lg), check.IsNil)
+ }
close(ok)
}()
select {
@@ -335,3 +345,9 @@ func (s *v0Suite) testClient() (*websocket.Conn, *json.Decoder, *json.Encoder) {
r := json.NewDecoder(conn)
return conn, r, w
}
+
+func (s *v0Suite) lastLogID(c *check.C) uint64 {
+ var lastID uint64
+ c.Assert(testDB().QueryRow(`SELECT MAX(id) FROM logs`).Scan(&lastID), check.IsNil)
+ return lastID
+}
commit 5d081c423f314060cefafc7149850ea1dcbe098a
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Jul 13 09:39:55 2017 -0400
11960: Test that "delete" permissions are not too permissive.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curoverse.com>
diff --git a/services/ws/session_v0_test.go b/services/ws/session_v0_test.go
index 499ffdb..1e6abbe 100644
--- a/services/ws/session_v0_test.go
+++ b/services/ws/session_v0_test.go
@@ -38,11 +38,15 @@ type v0Suite struct {
func (s *v0Suite) SetUpTest(c *check.C) {
s.serverSuite.SetUpTest(c)
+ go s.serverSuite.srv.Run()
+ s.serverSuite.srv.WaitReady()
+
s.token = arvadostest.ActiveToken
}
func (s *v0Suite) TearDownTest(c *check.C) {
s.wg.Wait()
+ s.serverSuite.srv.Close()
}
func (s *v0Suite) TearDownSuite(c *check.C) {
@@ -62,8 +66,7 @@ func (s *v0Suite) deleteTestObjects(c *check.C) {
}
func (s *v0Suite) TestFilters(c *check.C) {
- srv, conn, r, w := s.testClient()
- defer srv.Close()
+ conn, r, w := s.testClient()
defer conn.Close()
c.Check(w.Encode(map[string]interface{}{
@@ -81,8 +84,7 @@ func (s *v0Suite) TestLastLogID(c *check.C) {
var lastID uint64
c.Assert(testDB().QueryRow(`SELECT MAX(id) FROM logs`).Scan(&lastID), check.IsNil)
- srv, conn, r, w := s.testClient()
- defer srv.Close()
+ conn, r, w := s.testClient()
defer conn.Close()
uuidChan := make(chan string, 2)
@@ -122,8 +124,7 @@ func (s *v0Suite) TestLastLogID(c *check.C) {
}
func (s *v0Suite) TestPermission(c *check.C) {
- srv, conn, r, w := s.testClient()
- defer srv.Close()
+ conn, r, w := s.testClient()
defer conn.Close()
c.Check(w.Encode(map[string]interface{}{
@@ -148,26 +149,41 @@ func (s *v0Suite) TestPermission(c *check.C) {
}
}
+// Two users create private objects; admin deletes both objects; each
+// user receives a "delete" event for their own object (not for the
+// other user's object).
func (s *v0Suite) TestEventTypeDelete(c *check.C) {
- uuidChan := make(chan string, 1)
- s.emitEvents(uuidChan)
- uuid := <-uuidChan
-
- srv, conn, r, w := s.testClient()
- defer srv.Close()
- defer conn.Close()
+ clients := []struct {
+ token string
+ uuid string
+ conn *websocket.Conn
+ r *json.Decoder
+ w *json.Encoder
+ }{{token: arvadostest.ActiveToken}, {token: arvadostest.SpectatorToken}}
+ for i := range clients {
+ uuidChan := make(chan string, 1)
+ s.token = clients[i].token
+ s.emitEvents(uuidChan)
+ clients[i].uuid = <-uuidChan
+ clients[i].conn, clients[i].r, clients[i].w = s.testClient()
- c.Check(w.Encode(map[string]interface{}{
- "method": "subscribe",
- }), check.IsNil)
- s.expectStatus(c, r, 200)
+ c.Check(clients[i].w.Encode(map[string]interface{}{
+ "method": "subscribe",
+ }), check.IsNil)
+ s.expectStatus(c, clients[i].r, 200)
+ }
s.deleteTestObjects(c)
- lg := s.expectLog(c, r)
- c.Check(lg.ObjectUUID, check.Equals, uuid)
- c.Check(lg.EventType, check.Equals, "delete")
+
+ for _, client := range clients {
+ lg := s.expectLog(c, client.r)
+ c.Check(lg.ObjectUUID, check.Equals, client.uuid)
+ c.Check(lg.EventType, check.Equals, "delete")
+ }
}
+// Trashing/deleting a collection produces an "update" event with
+// properties["new_attributes"]["is_trashed"] == true.
func (s *v0Suite) TestTrashedCollection(c *check.C) {
ac := arvados.NewClientFromEnv()
ac.AuthToken = s.token
@@ -176,8 +192,7 @@ func (s *v0Suite) TestTrashedCollection(c *check.C) {
err := ac.RequestAndDecode(coll, "POST", "arvados/v1/collections", s.jsonBody("collection", coll), map[string]interface{}{"ensure_unique_name": true})
c.Assert(err, check.IsNil)
- srv, conn, r, w := s.testClient()
- defer srv.Close()
+ conn, r, w := s.testClient()
defer conn.Close()
c.Check(w.Encode(map[string]interface{}{
@@ -191,12 +206,12 @@ func (s *v0Suite) TestTrashedCollection(c *check.C) {
lg := s.expectLog(c, r)
c.Check(lg.ObjectUUID, check.Equals, coll.UUID)
c.Check(lg.EventType, check.Equals, "update")
+ c.Check(lg.Properties["old_attributes"].(map[string]interface{})["is_trashed"], check.Equals, false)
c.Check(lg.Properties["new_attributes"].(map[string]interface{})["is_trashed"], check.Equals, true)
}
func (s *v0Suite) TestSendBadJSON(c *check.C) {
- srv, conn, r, w := s.testClient()
- defer srv.Close()
+ conn, r, w := s.testClient()
defer conn.Close()
c.Check(w.Encode(map[string]interface{}{
@@ -215,8 +230,7 @@ func (s *v0Suite) TestSendBadJSON(c *check.C) {
}
func (s *v0Suite) TestSubscribe(c *check.C) {
- srv, conn, r, w := s.testClient()
- defer srv.Close()
+ conn, r, w := s.testClient()
defer conn.Close()
s.emitEvents(nil)
@@ -311,9 +325,7 @@ func (s *v0Suite) expectLog(c *check.C, r *json.Decoder) *arvados.Log {
}
}
-func (s *v0Suite) testClient() (*server, *websocket.Conn, *json.Decoder, *json.Encoder) {
- go s.serverSuite.srv.Run()
- s.serverSuite.srv.WaitReady()
+func (s *v0Suite) testClient() (*websocket.Conn, *json.Decoder, *json.Encoder) {
srv := s.serverSuite.srv
conn, err := websocket.Dial("ws://"+srv.listener.Addr().String()+"/websocket?api_token="+s.token, "", "http://"+srv.listener.Addr().String())
if err != nil {
@@ -321,5 +333,5 @@ func (s *v0Suite) testClient() (*server, *websocket.Conn, *json.Decoder, *json.E
}
w := json.NewEncoder(conn)
r := json.NewDecoder(conn)
- return srv, conn, r, w
+ return conn, r, w
}
commit d1562916e4792c0a9b4b2a4aea842e7c2848d38a
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Jul 13 03:26:01 2017 -0400
11960: Test trashed collection.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curoverse.com>
diff --git a/services/ws/session_v0_test.go b/services/ws/session_v0_test.go
index d3a3a8b..499ffdb 100644
--- a/services/ws/session_v0_test.go
+++ b/services/ws/session_v0_test.go
@@ -168,6 +168,32 @@ func (s *v0Suite) TestEventTypeDelete(c *check.C) {
c.Check(lg.EventType, check.Equals, "delete")
}
+func (s *v0Suite) TestTrashedCollection(c *check.C) {
+ ac := arvados.NewClientFromEnv()
+ ac.AuthToken = s.token
+
+ coll := &arvados.Collection{ManifestText: ""}
+ err := ac.RequestAndDecode(coll, "POST", "arvados/v1/collections", s.jsonBody("collection", coll), map[string]interface{}{"ensure_unique_name": true})
+ c.Assert(err, check.IsNil)
+
+ srv, conn, r, w := s.testClient()
+ defer srv.Close()
+ defer conn.Close()
+
+ c.Check(w.Encode(map[string]interface{}{
+ "method": "subscribe",
+ }), check.IsNil)
+ s.expectStatus(c, r, 200)
+
+ err = ac.RequestAndDecode(nil, "DELETE", "arvados/v1/collections/"+coll.UUID, nil, nil)
+ c.Assert(err, check.IsNil)
+
+ lg := s.expectLog(c, r)
+ c.Check(lg.ObjectUUID, check.Equals, coll.UUID)
+ c.Check(lg.EventType, check.Equals, "update")
+ c.Check(lg.Properties["new_attributes"].(map[string]interface{})["is_trashed"], check.Equals, true)
+}
+
func (s *v0Suite) TestSendBadJSON(c *check.C) {
srv, conn, r, w := s.testClient()
defer srv.Close()
commit 1be8ac0f887cb1c639ac502c47438a8260735069
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Jul 13 03:25:47 2017 -0400
11960: Fix events leaking between tests.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curoverse.com>
diff --git a/services/ws/session_v0_test.go b/services/ws/session_v0_test.go
index f2648fe..d3a3a8b 100644
--- a/services/ws/session_v0_test.go
+++ b/services/ws/session_v0_test.go
@@ -11,6 +11,7 @@ import (
"io"
"net/url"
"os"
+ "sync"
"time"
"git.curoverse.com/arvados.git/sdk/go/arvados"
@@ -32,6 +33,7 @@ type v0Suite struct {
serverSuite serverSuite
token string
toDelete []string
+ wg sync.WaitGroup
}
func (s *v0Suite) SetUpTest(c *check.C) {
@@ -39,9 +41,14 @@ func (s *v0Suite) SetUpTest(c *check.C) {
s.token = arvadostest.ActiveToken
}
+func (s *v0Suite) TearDownTest(c *check.C) {
+ s.wg.Wait()
+}
+
func (s *v0Suite) TearDownSuite(c *check.C) {
s.deleteTestObjects(c)
}
+
func (s *v0Suite) deleteTestObjects(c *check.C) {
ac := arvados.NewClientFromEnv()
ac.AuthToken = arvadostest.AdminToken
@@ -214,6 +221,9 @@ func (s *v0Suite) TestSubscribe(c *check.C) {
// created workflow. If uuidChan is not nil, send the new workflow
// UUID to uuidChan as soon as it's known.
func (s *v0Suite) emitEvents(uuidChan chan<- string) {
+ s.wg.Add(1)
+ defer s.wg.Done()
+
ac := arvados.NewClientFromEnv()
ac.AuthToken = s.token
wf := &arvados.Workflow{
commit 1e7fda3fcfb9b3030c65f56e06764d20496a0610
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Jul 13 02:39:48 2017 -0400
11960: Test permission on "delete" event.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curoverse.com>
diff --git a/services/ws/session_v0_test.go b/services/ws/session_v0_test.go
index 1213be5..f2648fe 100644
--- a/services/ws/session_v0_test.go
+++ b/services/ws/session_v0_test.go
@@ -40,6 +40,9 @@ func (s *v0Suite) SetUpTest(c *check.C) {
}
func (s *v0Suite) TearDownSuite(c *check.C) {
+ s.deleteTestObjects(c)
+}
+func (s *v0Suite) deleteTestObjects(c *check.C) {
ac := arvados.NewClientFromEnv()
ac.AuthToken = arvadostest.AdminToken
for _, path := range s.toDelete {
@@ -48,6 +51,7 @@ func (s *v0Suite) TearDownSuite(c *check.C) {
panic(err)
}
}
+ s.toDelete = nil
}
func (s *v0Suite) TestFilters(c *check.C) {
@@ -137,6 +141,26 @@ func (s *v0Suite) TestPermission(c *check.C) {
}
}
+func (s *v0Suite) TestEventTypeDelete(c *check.C) {
+ uuidChan := make(chan string, 1)
+ s.emitEvents(uuidChan)
+ uuid := <-uuidChan
+
+ srv, conn, r, w := s.testClient()
+ defer srv.Close()
+ defer conn.Close()
+
+ c.Check(w.Encode(map[string]interface{}{
+ "method": "subscribe",
+ }), check.IsNil)
+ s.expectStatus(c, r, 200)
+
+ s.deleteTestObjects(c)
+ lg := s.expectLog(c, r)
+ c.Check(lg.ObjectUUID, check.Equals, uuid)
+ c.Check(lg.EventType, check.Equals, "delete")
+}
+
func (s *v0Suite) TestSendBadJSON(c *check.C) {
srv, conn, r, w := s.testClient()
defer srv.Close()
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list