implement user details storage (#469)

* implement (strings) user details storage

* add rpc user details implementation

* return error from getUserDetail, rewrite tests to table tests

* make UserDetails store UserDetailEntry instead of strings

* update comment about user_details

* fix confusing return

* add user details support for memory store

* add engine.UserDetailEntry to service.UserMetaData

* add ListDetails support to memory storage

* add user details support to native migrator, ListDetails func to storage

* go mod tidy for memory storage

* increase memory storage test coverage, fix tests naming

* add ListDetails tests to memory storage

* add engine.ListDetails and  service.[Set]Metas tests

* change Fprintf to Fprint (triggered by explicitly ignoring error)

* remove Delete from engine.UserDetail, implement list via same method

* adjust service.Metas to new engine.UserDetails signature

* introduce engine.UserDetail("all") consonant

* fix Meta user detail retrieval

* extend store implementations Delete method with UserDetail deletion

* make UserDetail test answer order-independent

* fix flaky test check in TestMemData_FlagListBlocked

* delete user details alongside with comments on deleteme request

* add tests to UserDetail store.Delete implementations

* clarify engine module user details consonants names

* update comments to reflect current state of code

* check for value absence instead of it's length

* revert unneeded code change

* add extensive commentary on UserDetail return type

* remove unused check condition

* clarify UserDetail tests to be truly stateless

* add clarifying comment for pre-table test
This commit is contained in:
Dmitry Verkhoturov
2019-11-22 02:24:26 -06:00
committed by Umputun
parent 773da16649
commit ddd466ec41
13 changed files with 659 additions and 56 deletions
+129 -5
View File
@@ -41,6 +41,7 @@ type metaUser struct {
Verified bool
Blocked bool
BlockedUntil time.Time
Details engine.UserDetailEntry
}
// NewMemData makes in-memory engine.
@@ -280,17 +281,51 @@ func (m *MemData) ListFlags(req engine.FlagRequest) (res []interface{}, err erro
return nil, errors.Errorf("flag %s not listable", req.Flag)
}
// Delete post(s), user, comment, or everything
// UserDetail sets or gets single detail value, or gets all details fo§r requested site.
// UserDetail returns list even for single entry request is a compromise in order to have both single detail getting and setting
// and all site's details listing under the same function (and not to extend engine interface by two separate functions).
func (m *MemData) UserDetail(req engine.UserDetailRequest) ([]engine.UserDetailEntry, error) {
switch req.Detail {
case engine.UserEmail:
if req.UserID == "" {
return nil, errors.New("userid cannot be empty in request for single detail")
}
m.Lock()
defer m.Unlock()
if req.Update == "" { // read detail value, no update requested
return m.getUserDetail(req)
}
return m.setUserDetail(req)
case engine.AllUserDetails:
// list of all details returned in case request is a read request
// (Update is not set) and does not have UserID or Detail set
if req.Update == "" && req.UserID == "" { // read list of all details
m.Lock()
defer m.Unlock()
return m.listDetails(req.Locator)
}
return nil, errors.New("unsupported request with userdetail all")
default:
return nil, errors.Errorf("unsupported detail %q", req.Detail)
}
}
// Delete post(s), user, comment, user details, or everything
func (m *MemData) Delete(req engine.DeleteRequest) error {
m.Lock()
defer m.Unlock()
switch {
case req.Locator.URL != "" && req.CommentID != "": // delete comment
case req.UserDetail != "": // delete user detail
return m.deleteUserDetail(req.Locator, req.UserID, req.UserDetail)
case req.Locator.URL != "" && req.CommentID != "" && req.UserDetail == "": // delete comment
return m.deleteComment(req.Locator, req.CommentID, req.DeleteMode)
case req.Locator.SiteID != "" && req.UserID != "" && req.CommentID == "": // delete user
case req.Locator.SiteID != "" && req.UserID != "" && req.CommentID == "" && req.UserDetail == "": // delete user
comments := m.match(m.posts[req.Locator.SiteID], func(c store.Comment) bool {
return c.User.ID == req.UserID && !c.Deleted
})
@@ -299,9 +334,9 @@ func (m *MemData) Delete(req engine.DeleteRequest) error {
return e
}
}
return nil
return m.deleteUserDetail(req.Locator, req.UserID, engine.AllUserDetails)
case req.Locator.SiteID != "" && req.Locator.URL == "" && req.CommentID == "" && req.UserID == "": // delete site
case req.Locator.SiteID != "" && req.Locator.URL == "" && req.CommentID == "" && req.UserID == "" && req.UserDetail == "": // delete site
if _, ok := m.posts[req.Locator.SiteID]; !ok {
return errors.New("not found")
}
@@ -401,6 +436,95 @@ func (m *MemData) setFlag(req engine.FlagRequest) (res bool, err error) {
return status, errors.Wrapf(err, "failed to set flag %+v", req)
}
// getUserDetail returns UserDetailEntry with requested userDetail (omitting other details)
// as an only element of the slice.
func (m *MemData) getUserDetail(req engine.UserDetailRequest) ([]engine.UserDetailEntry, error) {
if meta, ok := m.metaUsers[req.UserID]; ok {
if meta.SiteID != req.Locator.SiteID {
return []engine.UserDetailEntry{}, nil
}
switch req.Detail {
case engine.UserEmail:
return []engine.UserDetailEntry{{UserID: req.UserID, Email: meta.Details.Email}}, nil
}
}
return []engine.UserDetailEntry{}, nil
}
// setUserDetail sets requested userDetail, returning complete updated UserDetailEntry as an onlyIps
// element of the slice in case of success
func (m *MemData) setUserDetail(req engine.UserDetailRequest) ([]engine.UserDetailEntry, error) {
var entry metaUser
if meta, ok := m.metaUsers[req.UserID]; ok {
if meta.SiteID != req.Locator.SiteID {
return []engine.UserDetailEntry{}, nil
}
entry = meta
}
if entry == (metaUser{}) {
entry = metaUser{
UserID: req.UserID,
SiteID: req.Locator.SiteID,
Details: engine.UserDetailEntry{UserID: req.UserID},
}
}
switch req.Detail {
case engine.UserEmail:
entry.Details.Email = req.Update
m.metaUsers[req.UserID] = entry
return []engine.UserDetailEntry{{UserID: req.UserID, Email: req.Update}}, nil
}
return []engine.UserDetailEntry{}, nil
}
// listDetails lists all available users details for given siteID
func (m *MemData) listDetails(loc store.Locator) ([]engine.UserDetailEntry, error) {
var res []engine.UserDetailEntry
for _, u := range m.metaUsers {
if u.SiteID == loc.SiteID {
res = append(res, u.Details)
}
}
return res, nil
}
// deleteUserDetail deletes requested UserDetail or whole UserDetailEntry,
// deletion of the absent entry doesn't produce error.
// Trying to delete user with wrong siteID doesn't to anything and doesn't produce error.
func (m *MemData) deleteUserDetail(locator store.Locator, userID string, userDetail engine.UserDetail) error {
var entry metaUser
if meta, ok := m.metaUsers[userID]; ok {
if meta.SiteID != locator.SiteID {
return nil
}
entry = meta
}
if entry == (metaUser{}) || entry.Details == (engine.UserDetailEntry{}) {
// absent entry means that we should not do anything
return nil
}
switch userDetail {
case engine.UserEmail:
entry.Details.Email = ""
case engine.AllUserDetails:
entry.Details = engine.UserDetailEntry{UserID: userID}
}
if entry.Details == (engine.UserDetailEntry{UserID: userID}) {
// no user details are stored, empty details entry altogether
entry.Details = engine.UserDetailEntry{}
}
m.metaUsers[userID] = entry
return nil
}
func (m *MemData) get(loc store.Locator, commentID string) (store.Comment, error) {
comments := m.match(m.posts[loc.SiteID], func(c store.Comment) bool {
return c.Locator == loc && c.ID == commentID
@@ -658,6 +658,43 @@ func TestMemData_DeleteAll(t *testing.T) {
assert.Equal(t, 0, len(comments), "nothing left")
}
func TestMemData_DeleteUserDetail(t *testing.T) {
var (
createUser = engine.UserDetailRequest{Locator: store.Locator{SiteID: "test-site"}, UserID: "user1", Detail: engine.UserEmail, Update: "value1"}
readUser = engine.UserDetailRequest{Locator: store.Locator{SiteID: "test-site"}, UserID: "user1", Detail: engine.UserEmail}
emailSet = []engine.UserDetailEntry{{UserID: "user1", Email: "value1"}}
emailUnset = []engine.UserDetailEntry{{UserID: "user1", Email: ""}}
)
b := prepMem(t)
var testData = []struct {
delReq engine.DeleteRequest
detailReq engine.UserDetailRequest
expected []engine.UserDetailEntry
}{
{delReq: engine.DeleteRequest{Locator: store.Locator{SiteID: "test-site"}, UserID: "user1", UserDetail: engine.UserEmail},
detailReq: createUser, expected: emailSet},
{delReq: engine.DeleteRequest{Locator: store.Locator{SiteID: "bad"}, UserID: "user1", UserDetail: engine.UserEmail},
detailReq: readUser, expected: emailSet},
{delReq: engine.DeleteRequest{Locator: store.Locator{SiteID: "test-site"}, UserID: "user1", UserDetail: engine.UserEmail},
detailReq: readUser, expected: emailUnset},
{delReq: engine.DeleteRequest{Locator: store.Locator{SiteID: "test-site"}, UserID: "user1", UserDetail: engine.AllUserDetails},
detailReq: createUser, expected: emailSet},
{delReq: engine.DeleteRequest{Locator: store.Locator{SiteID: "test-site"}, UserID: "user1", UserDetail: engine.AllUserDetails},
detailReq: readUser, expected: emailUnset},
}
for i, x := range testData {
err := b.Delete(x.delReq)
require.NoError(t, err, "delete request #%d error", i)
val, err := b.UserDetail(x.detailReq)
require.NoError(t, err, "user request #%d error", i)
require.Equal(t, x.expected, val, "user request #%d result", i)
}
}
func TestMemAdmin_DeleteUserHard(t *testing.T) {
b := prepMem(t)
err := b.Delete(engine.DeleteRequest{Locator: store.Locator{SiteID: "radio-t"}, UserID: "user1",
+24 -11
View File
@@ -28,16 +28,17 @@ func NewRPC(e engine.Interface, a admin.Store, r *jrpc.Server) *RPC {
func (s *RPC) addHandlers() {
// data store handlers
s.Group("store", jrpc.HandlersGroup{
"create": s.createHndl,
"find": s.findHndl,
"get": s.getHndl,
"update": s.updateHndl,
"count": s.countHndl,
"info": s.infoHndl,
"flag": s.flagHndl,
"list_flags": s.listFlagsHndl,
"delete": s.deleteHndl,
"close": s.closeHndl,
"create": s.createHndl,
"find": s.findHndl,
"get": s.getHndl,
"update": s.updateHndl,
"count": s.countHndl,
"info": s.infoHndl,
"flag": s.flagHndl,
"list_flags": s.listFlagsHndl,
"user_detail": s.userDetailHndl,
"delete": s.deleteHndl,
"close": s.closeHndl,
})
// admin store handlers
@@ -129,7 +130,19 @@ func (s *RPC) listFlagsHndl(id uint64, params json.RawMessage) (rr jrpc.Response
return jrpc.EncodeResponse(id, flags, err)
}
// deleteHndl delete post(s), user, comment, or everything
// userDetailHndl sets or gets single detail value, or gets all details for requested site.
// userDetailHndl returns list even for single entry request is a compromise in order to have both single detail getting and setting
// and all site's details listing under the same function (and not to extend engine interface by two separate functions).
func (s *RPC) userDetailHndl(id uint64, params json.RawMessage) (rr jrpc.Response) {
req := engine.UserDetailRequest{}
if err := json.Unmarshal(params, &req); err != nil {
return jrpc.Response{Error: err.Error()}
}
value, err := s.eng.UserDetail(req)
return jrpc.EncodeResponse(id, value, err)
}
// deleteHndl delete post(s), user, comment, user details, or everything
func (s *RPC) deleteHndl(id uint64, params json.RawMessage) (rr jrpc.Response) {
req := engine.DeleteRequest{}
if err := json.Unmarshal(params, &req); err != nil {
@@ -223,6 +223,59 @@ func TestRPC_listFlagsHndl(t *testing.T) {
assert.Equal(t, []interface{}{"u1"}, flags)
}
func TestRPC_userDetailHndl(t *testing.T) {
_, port, teardown := prepTestStore(t)
defer teardown()
api := fmt.Sprintf("http://localhost:%d/test", port)
re := engine.RPC{Client: jrpc.Client{API: api, Client: http.Client{Timeout: 1 * time.Second}}}
// add to entries to DB before we start
result, err := re.UserDetail(engine.UserDetailRequest{Locator: store.Locator{SiteID: "test-site"}, UserID: "u1", Detail: engine.UserEmail, Update: "test@example.com"})
assert.NoError(t, err, "No error inserting entry expected")
assert.ElementsMatch(t, []engine.UserDetailEntry{{UserID: "u1", Email: "test@example.com"}}, result)
result, err = re.UserDetail(engine.UserDetailRequest{Locator: store.Locator{SiteID: "test-site"}, UserID: "u2", Detail: engine.UserEmail, Update: "other@example.com"})
assert.NoError(t, err, "No error inserting entry expected")
assert.ElementsMatch(t, []engine.UserDetailEntry{{UserID: "u2", Email: "other@example.com"}}, result)
// try to change existing entry with wrong SiteID
result, err = re.UserDetail(engine.UserDetailRequest{Locator: store.Locator{SiteID: "bad"}, UserID: "u2", Detail: engine.UserEmail, Update: "not_relevant"})
assert.NoError(t, err, "Updating existing entry with wrong SiteID doesn't produce error")
assert.ElementsMatch(t, []engine.UserDetailEntry{}, result, "Updating existing entry with wrong SiteID doesn't change anything")
// stateless tests without changing the state we set up before
var testData = []struct {
req engine.UserDetailRequest
error string
expected []engine.UserDetailEntry
}{
{req: engine.UserDetailRequest{Locator: store.Locator{SiteID: "test-site"}, UserID: "u1", Detail: engine.UserEmail},
expected: []engine.UserDetailEntry{{UserID: "u1", Email: "test@example.com"}}},
{req: engine.UserDetailRequest{Locator: store.Locator{SiteID: "bad"}, UserID: "u1", Detail: engine.UserEmail},
expected: []engine.UserDetailEntry{}},
{req: engine.UserDetailRequest{Locator: store.Locator{SiteID: "test-site"}, UserID: "u1xyz", Detail: engine.UserEmail},
expected: []engine.UserDetailEntry{}},
{req: engine.UserDetailRequest{Detail: engine.UserEmail, Update: "new_value"},
error: `userid cannot be empty in request for single detail`},
{req: engine.UserDetailRequest{Detail: engine.UserDetail("bad")},
error: `unsupported detail "bad"`},
{req: engine.UserDetailRequest{Update: "not_relevant", Detail: engine.AllUserDetails},
error: `unsupported request with userdetail all`},
{req: engine.UserDetailRequest{Locator: store.Locator{SiteID: "test-site"}, Detail: engine.AllUserDetails},
expected: []engine.UserDetailEntry{{UserID: "u1", Email: "test@example.com"}, {UserID: "u2", Email: "other@example.com"}}},
}
for i, x := range testData {
result, err := re.UserDetail(x.req)
if x.error != "" {
assert.EqualError(t, err, x.error, "Error should match expected for case %d", i)
} else {
assert.NoError(t, err, "Error is not expected expected for case %d", i)
}
assert.ElementsMatch(t, x.expected, result, "Result should match expected for case %d", i)
}
}
func TestRPC_deleteHndl(t *testing.T) {
_, port, teardown := prepTestStore(t)
defer teardown()