From 58d7506969999b774aaf938e33995a17cbba24b2 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Thu, 18 Oct 2018 10:23:26 +0200 Subject: [PATCH] Channel creation & membership improvements - channel creation service now handles membership payload - channel creation service prevents non-unique membership constalations - adding / removing members is disabled for groups (temp solution) --- internal/payload/util.go | 10 +++++ sam/docs/README.md | 20 ++------- sam/docs/src/spec.json | 31 ++++---------- sam/docs/src/spec/channel.json | 8 +++- sam/docs/src/spec/user.json | 24 ----------- sam/repository/channel.go | 44 ++++++++++---------- sam/rest/channel.go | 7 ++-- sam/rest/handlers/user.go | 12 +----- sam/rest/request/channel.go | 7 ++-- sam/rest/request/user.go | 47 --------------------- sam/rest/user.go | 8 +--- sam/service/channel.go | 76 +++++++++++++++++++++++++++++----- sam/service/message.go | 65 +---------------------------- sam/types/channel_member.go | 11 +++++ 14 files changed, 139 insertions(+), 231 deletions(-) diff --git a/internal/payload/util.go b/internal/payload/util.go index 5cf832c60..f462f4a00 100644 --- a/internal/payload/util.go +++ b/internal/payload/util.go @@ -25,3 +25,13 @@ func ParseUInt64(s string) uint64 { i, _ := strconv.ParseUint(s, 10, 64) return i } + +// ParseUInt64 parses a slice of strings into a slice of uint64s +func ParseUInt64s(ss []string) []uint64 { + uu := make([]uint64, len(ss)) + for i, s := range ss { + uu[i] = ParseUInt64(s) + } + + return uu +} diff --git a/sam/docs/README.md b/sam/docs/README.md index b163b27c6..1b667f506 100644 --- a/sam/docs/README.md +++ b/sam/docs/README.md @@ -216,7 +216,7 @@ An organisation may have many teams. Teams may have many channels available. Acc # Channels -A channel is a representation of a sequence of messages. It has meta data like channel subject. Channels may be public, private or direct (between two users). +A channel is a representation of a sequence of messages. It has meta data like channel subject. Channels may be public, private or group. ## List channels @@ -247,6 +247,7 @@ A channel is a representation of a sequence of messages. It has meta data like c | name | string | POST | Name of Channel | N/A | YES | | topic | string | POST | Subject of Channel | N/A | NO | | type | string | POST | Channel type | N/A | NO | +| members | []string | POST | Initial members of the channel | N/A | NO | ## Update channel details @@ -601,19 +602,4 @@ The following event types may be sent with a message event: | Parameter | Type | Method | Description | Default | Required? | | --------- | ---- | ------ | ----------- | ------- | --------- | -| query | string | GET | Search query to match against users | N/A | NO | - -## Send direct message to user - -#### Method - -| URI | Protocol | Method | Authentication | -| --- | -------- | ------ | -------------- | -| `/users/{userID}/message` | HTTP/S | POST | Client ID, Session ID | - -#### Request parameters - -| Parameter | Type | Method | Description | Default | Required? | -| --------- | ---- | ------ | ----------- | ------- | --------- | -| userID | uint64 | PATH | User ID | N/A | YES | -| message | string | POST | Message contents (markdown) | N/A | YES | \ No newline at end of file +| query | string | GET | Search query to match against users | N/A | NO | \ No newline at end of file diff --git a/sam/docs/src/spec.json b/sam/docs/src/spec.json index af179bd8c..e4ac2919f 100644 --- a/sam/docs/src/spec.json +++ b/sam/docs/src/spec.json @@ -189,7 +189,7 @@ }, { "title": "Channels", - "description": "A channel is a representation of a sequence of messages. It has meta data like channel subject. Channels may be public, private or direct (between two users).", + "description": "A channel is a representation of a sequence of messages. It has meta data like channel subject. Channels may be public, private or group.", "package": "sam", "entrypoint": "channel", "path": "/channels", @@ -213,9 +213,10 @@ "path": "/", "parameters": { "post": [ - { "type": "string", "name": "name", "required": true, "title": "Name of Channel" }, - { "type": "string", "name": "topic", "required": false, "title": "Subject of Channel" }, - { "type": "string", "name": "type", "required": false, "title": "Channel type" } + { "type": "string", "name": "name", "required": true, "title": "Name of Channel" }, + { "type": "string", "name": "topic", "required": false, "title": "Subject of Channel" }, + { "type": "string", "name": "type", "required": false, "title": "Channel type" }, + { "type": "[]string", "name": "members", "required": false, "title": "Initial members of the channel" } ] } }, @@ -229,10 +230,10 @@ { "type": "uint64", "name": "channelID", "required": true, "title": "Channel ID" } ], "post": [ - { "type": "string", "name": "name", "required": false, "title": "Name of Channel" }, - { "type": "string", "name": "topic", "required": false, "title": "Subject of Channel" }, - { "type": "string", "name": "type", "required": false, "title": "Channel type" }, - { "type": "bool", "name": "archive", "required": false, "title": "Request channel to be archived or unarchived" }, + { "type": "string", "name": "name", "required": false, "title": "Name of Channel" }, + { "type": "string", "name": "topic", "required": false, "title": "Subject of Channel" }, + { "type": "string", "name": "type", "required": false, "title": "Channel type" }, + { "type": "bool", "name": "archive", "required": false, "title": "Request channel to be archived or unarchived" }, { "type": "uint64", "name": "organisationID", "required": false, "title": "Move channel to different organisation" } ] } @@ -519,20 +520,6 @@ { "type": "string", "name": "query", "required": false, "title": "Search query to match against users" } ] } - }, - { - "name": "message", - "path": "/{userID}/message", - "method": "POST", - "title": "Send direct message to user", - "parameters": { - "path": [ - { "name": "userID", "type": "uint64", "required": true, "title": "User ID" } - ], - "post": [ - { "type": "string", "name": "message", "required": true, "title": "Message contents (markdown)" } - ] - } } ] } diff --git a/sam/docs/src/spec/channel.json b/sam/docs/src/spec/channel.json index 6c88c26cf..4a1e0abf8 100644 --- a/sam/docs/src/spec/channel.json +++ b/sam/docs/src/spec/channel.json @@ -1,6 +1,6 @@ { "Title": "Channels", - "Description": "A channel is a representation of a sequence of messages. It has meta data like channel subject. Channels may be public, private or direct (between two users).", + "Description": "A channel is a representation of a sequence of messages. It has meta data like channel subject. Channels may be public, private or group.", "Package": "sam", "Interface": "Channel", "Struct": null, @@ -52,6 +52,12 @@ "required": false, "title": "Channel type", "type": "string" + }, + { + "name": "members", + "required": false, + "title": "Initial members of the channel", + "type": "[]string" } ] } diff --git a/sam/docs/src/spec/user.json b/sam/docs/src/spec/user.json index 9ed5c5399..bc08925bf 100644 --- a/sam/docs/src/spec/user.json +++ b/sam/docs/src/spec/user.json @@ -26,30 +26,6 @@ } ] } - }, - { - "Name": "message", - "Method": "POST", - "Title": "Send direct message to user", - "Path": "/{userID}/message", - "Parameters": { - "path": [ - { - "name": "userID", - "required": true, - "title": "User ID", - "type": "uint64" - } - ], - "post": [ - { - "name": "message", - "required": true, - "title": "Message contents (markdown)", - "type": "string" - } - ] - } } ] } \ No newline at end of file diff --git a/sam/repository/channel.go b/sam/repository/channel.go index 9a04e2264..7a836d0d1 100644 --- a/sam/repository/channel.go +++ b/sam/repository/channel.go @@ -2,6 +2,8 @@ package repository import ( "context" + "sort" + "strconv" "time" "github.com/titpetric/factory" @@ -14,7 +16,7 @@ type ( With(ctx context.Context, db *factory.DB) ChannelRepository FindChannelByID(id uint64) (*types.Channel, error) - FindDirectChannelByUserID(fromUserID, toUserID uint64) (*types.Channel, error) + FindChannelByMemberSet(memberID ...uint64) (*types.Channel, error) FindChannels(filter *types.ChannelFilter) ([]*types.Channel, error) CreateChannel(mod *types.Channel) (*types.Channel, error) UpdateChannel(mod *types.Channel) (*types.Channel, error) @@ -38,16 +40,13 @@ const ( FROM channels AS c WHERE ` + sqlChannelValidOnly - // Returns channel (group) with exactly 2 members - sqlChannelDirect = `SELECT * - FROM channels AS c - WHERE c.type = ? - AND c.id IN (SELECT rel_channel - FROM channel_members - GROUP BY rel_channel - HAVING COUNT(*) = 2 - AND MIN(rel_user) = ? - AND MAX(rel_user) = ?)` + sqlChannelGroupByMemberSet = sqlChannelSelect + ` AND c.type = ? AND c.id IN ( + SELECT rel_channel + FROM channel_members + GROUP BY rel_channel + HAVING COUNT(*) = ? + AND CONCAT(GROUP_CONCAT(rel_user ORDER BY 1 ASC SEPARATOR ','),',') = ? + )` // subquery that filters out all channels that current user has access to as a member // or via channel type (public chans) @@ -82,22 +81,21 @@ func (r *channel) FindChannelByID(id uint64) (*types.Channel, error) { return mod, isFound(r.db().Get(mod, sql, id), mod.ID > 0, ErrChannelNotFound) } -func (r *channel) FindDirectChannelByUserID(fromUserID, toUserID uint64) (*types.Channel, error) { +// FindChannelByMemberSet searches for channel (group!) with exactly the same membership structure +func (r *channel) FindChannelByMemberSet(memberIDs ...uint64) (*types.Channel, error) { mod := &types.Channel{} - if fromUserID == toUserID { - // do not waste any cpu cycles for his... - return nil, ErrChannelNotFound + sort.Slice(memberIDs, func(i, j int) bool { + return memberIDs[i] < memberIDs[j] + }) + + membersConcat := "" + for i := range memberIDs { + // Don't panic, we're adding , in the SQL as well + membersConcat += strconv.FormatUint(memberIDs[i], 10) + "," } - // We're grouping and aggregating values by min/max value of the user ID - // so we need to swap valuess - if fromUserID > toUserID { - // Order by user idso we can simplifiy the search - toUserID, fromUserID = fromUserID, toUserID - } - - return mod, isFound(r.db().Get(mod, sqlChannelDirect, types.ChannelTypeGroup, fromUserID, toUserID), mod.ID > 0, ErrChannelNotFound) + return mod, isFound(r.db().Get(mod, sqlChannelGroupByMemberSet, types.ChannelTypeGroup, len(memberIDs), membersConcat), mod.ID > 0, ErrChannelNotFound) } func (r *channel) FindChannels(filter *types.ChannelFilter) ([]*types.Channel, error) { diff --git a/sam/rest/channel.go b/sam/rest/channel.go index bd68e634e..5fb6891ae 100644 --- a/sam/rest/channel.go +++ b/sam/rest/channel.go @@ -32,9 +32,10 @@ func (Channel) New() *Channel { func (ctrl *Channel) Create(ctx context.Context, r *request.ChannelCreate) (interface{}, error) { channel := &types.Channel{ - Name: r.Name, - Topic: r.Topic, - Type: types.ChannelType(r.Type), + Name: r.Name, + Topic: r.Topic, + Type: types.ChannelType(r.Type), + Members: payload.ParseUInt64s(r.Members), } return ctrl.wrap(ctrl.svc.ch.With(ctx).Create(channel)) diff --git a/sam/rest/handlers/user.go b/sam/rest/handlers/user.go index 2ab1cfb98..9306d7d12 100644 --- a/sam/rest/handlers/user.go +++ b/sam/rest/handlers/user.go @@ -28,13 +28,11 @@ import ( // Internal API interface type UserAPI interface { Search(context.Context, *request.UserSearch) (interface{}, error) - Message(context.Context, *request.UserMessage) (interface{}, error) } // HTTP API interface type User struct { - Search func(http.ResponseWriter, *http.Request) - Message func(http.ResponseWriter, *http.Request) + Search func(http.ResponseWriter, *http.Request) } func NewUser(uh UserAPI) *User { @@ -46,13 +44,6 @@ func NewUser(uh UserAPI) *User { return uh.Search(r.Context(), params) }) }, - Message: func(w http.ResponseWriter, r *http.Request) { - defer r.Body.Close() - params := request.NewUserMessage() - resputil.JSON(w, params.Fill(r), func() (interface{}, error) { - return uh.Message(r.Context(), params) - }) - }, } } @@ -61,7 +52,6 @@ func (uh *User) MountRoutes(r chi.Router, middlewares ...func(http.Handler) http r.Use(middlewares...) r.Route("/users", func(r chi.Router) { r.Get("/search", uh.Search) - r.Post("/{userID}/message", uh.Message) }) }) } diff --git a/sam/rest/request/channel.go b/sam/rest/request/channel.go index 013d2e95c..f4378cbe6 100644 --- a/sam/rest/request/channel.go +++ b/sam/rest/request/channel.go @@ -77,9 +77,10 @@ var _ RequestFiller = NewChannelList() // Channel create request parameters type ChannelCreate struct { - Name string - Topic string - Type string + Name string + Topic string + Type string + Members []string } func NewChannelCreate() *ChannelCreate { diff --git a/sam/rest/request/user.go b/sam/rest/request/user.go index 7d0414eff..e4f5b2503 100644 --- a/sam/rest/request/user.go +++ b/sam/rest/request/user.go @@ -74,50 +74,3 @@ func (u *UserSearch) Fill(r *http.Request) error { } var _ RequestFiller = NewUserSearch() - -// User message request parameters -type UserMessage struct { - UserID uint64 `json:",string"` - Message string -} - -func NewUserMessage() *UserMessage { - return &UserMessage{} -} - -func (u *UserMessage) Fill(r *http.Request) error { - var err error - - if strings.ToLower(r.Header.Get("content-type")) == "application/json" { - err = json.NewDecoder(r.Body).Decode(u) - - switch { - case err == io.EOF: - err = nil - case err != nil: - return errors.Wrap(err, "error parsing http request body") - } - } - - r.ParseForm() - get := map[string]string{} - post := map[string]string{} - urlQuery := r.URL.Query() - for name, param := range urlQuery { - get[name] = string(param[0]) - } - postVars := r.Form - for name, param := range postVars { - post[name] = string(param[0]) - } - - u.UserID = parseUInt64(chi.URLParam(r, "userID")) - if val, ok := post["message"]; ok { - - u.Message = val - } - - return err -} - -var _ RequestFiller = NewUserMessage() diff --git a/sam/rest/user.go b/sam/rest/user.go index 10ba609e9..e1af969cf 100644 --- a/sam/rest/user.go +++ b/sam/rest/user.go @@ -3,12 +3,12 @@ package rest import ( "context" + "github.com/pkg/errors" + authService "github.com/crusttech/crust/auth/service" authTypes "github.com/crusttech/crust/auth/types" "github.com/crusttech/crust/sam/rest/request" "github.com/crusttech/crust/sam/service" - "github.com/crusttech/crust/sam/types" - "github.com/pkg/errors" ) var _ = errors.Wrap @@ -33,7 +33,3 @@ func (User) New() *User { func (ctrl *User) Search(ctx context.Context, r *request.UserSearch) (interface{}, error) { return ctrl.svc.user.With(ctx).Find(&authTypes.UserFilter{Query: r.Query}) } - -func (ctrl *User) Message(ctx context.Context, r *request.UserMessage) (interface{}, error) { - return ctrl.svc.message.With(ctx).Direct(r.UserID, &types.Message{Message: r.Message}) -} diff --git a/sam/service/channel.go b/sam/service/channel.go index 771dcc1b1..782a12167 100644 --- a/sam/service/channel.go +++ b/sam/service/channel.go @@ -219,6 +219,17 @@ func (svc *channel) Create(in *types.Channel) (out *types.Channel, err error) { var chCreatorID = repository.Identity(svc.ctx) + mm := svc.buildMemberSet(chCreatorID, in.Members...) + + if in.Type == types.ChannelTypeGroup { + if out, err = svc.checkGroupExistance(mm); err != nil { + return err + } else if out != nil { + // Group already exists so let's just return it + return nil + } + } + // @todo [SECURITY] check if channel topic can be set if in.Topic != "" && false { return errors.New("Not allowed to set channel topic") @@ -253,11 +264,17 @@ func (svc *channel) Create(in *types.Channel) (out *types.Channel, err error) { return } - // Join current user as an member & owner - _, err = svc.cmember.Create(&types.ChannelMember{ - ChannelID: out.ID, - UserID: chCreatorID, - Type: types.ChannelMembershipTypeOwner, + err = mm.Walk(func(m *types.ChannelMember) (err error) { + // Assign channel ID to membership + m.ChannelID = out.ID + + // Create member + if m, err = svc.cmember.Create(m); err != nil { + return err + } + + // Subscribe all members + return svc.evl.Join(m.UserID, out.ID) }) if err != nil { @@ -265,11 +282,8 @@ func (svc *channel) Create(in *types.Channel) (out *types.Channel, err error) { return } - // When broadcasting, make sure we let the subscribers know - // that creator is also a member... - out.Members = append(out.Members, chCreatorID) - - svc.evl.Join(chCreatorID, out.ID) + // Copy all member IDs to channel's member slice + out.Members = mm.AllMemberIDs() // Create the first message, doing this directly with repository to circumvent // message service constraints @@ -292,6 +306,34 @@ func (svc *channel) Create(in *types.Channel) (out *types.Channel, err error) { }) } +func (svc *channel) buildMemberSet(owner uint64, members ...uint64) (mm types.ChannelMemberSet) { + // Join current user as an member & owner + mm = types.ChannelMemberSet{&types.ChannelMember{ + UserID: owner, + Type: types.ChannelMembershipTypeOwner, + }} + + // Add all required members and make sure that list is unique + for _, m := range members { + if mm.FindByUserId(m) == nil { + mm = append(mm, &types.ChannelMember{ + UserID: m, + Type: types.ChannelMembershipTypeMember, + }) + } + } + + return mm +} + +func (svc *channel) checkGroupExistance(mm types.ChannelMemberSet) (*types.Channel, error) { + if c, err := svc.channel.FindChannelByMemberSet(mm.AllMemberIDs()...); err == repository.ErrChannelNotFound { + return nil, nil + } else { + return c, err + } +} + func (svc *channel) Update(ch *types.Channel) (out *types.Channel, err error) { return out, svc.db.Transaction(func() (err error) { // @todo [SECURITY] can user access this channel? @@ -479,6 +521,10 @@ func (svc *channel) InviteUser(channelID uint64, memberIDs ...uint64) (out types return } + if ch.Type == types.ChannelTypeGroup { + return nil, errors.New("Adding members to a group is not currently supported") + } + // @todo [SECURITY] can user add members to this channel? return out, svc.db.Transaction(func() (err error) { @@ -537,6 +583,10 @@ func (svc *channel) AddMember(channelID uint64, memberIDs ...uint64) (out types. return } + if ch.Type == types.ChannelTypeGroup { + return nil, errors.New("Adding members to a group is not currently supported") + } + // @todo [SECURITY] can user add members to this channel? return out, svc.db.Transaction(func() (err error) { @@ -618,6 +668,10 @@ func (svc *channel) DeleteMember(channelID uint64, memberIDs ...uint64) (err err return } + if ch.Type == types.ChannelTypeGroup { + return errors.New("Removign members from a group is not currently supported") + } + // @todo [SECURITY] can user remove members from this channel? return svc.db.Transaction(func() (err error) { @@ -687,7 +741,7 @@ func (svc *channel) sendChannelEvent(ch *types.Channel) (err error) { if mm, err := svc.cmember.Find(&types.ChannelMemberFilter{ChannelID: ch.ID}); err != nil { return err } else { - ch.Members = mm.MembersOf(ch.ID) + ch.Members = mm.AllMemberIDs() } } } diff --git a/sam/service/message.go b/sam/service/message.go index cefc4d4d6..fa1407a3b 100644 --- a/sam/service/message.go +++ b/sam/service/message.go @@ -3,10 +3,10 @@ package service import ( "context" - authService "github.com/crusttech/crust/auth/service" - "github.com/pkg/errors" "github.com/titpetric/factory" + authService "github.com/crusttech/crust/auth/service" + "github.com/crusttech/crust/sam/repository" "github.com/crusttech/crust/sam/types" ) @@ -44,8 +44,6 @@ type ( Flag(messageID uint64) error Unflag(messageID uint64) error - Direct(recipientID uint64, in *types.Message) (out *types.Message, err error) - Delete(ID uint64) error } ) @@ -120,65 +118,6 @@ func (svc *message) loadAttachments(mm types.MessageSet) (err error) { } } -func (svc *message) Direct(recipientID uint64, in *types.Message) (out *types.Message, err error) { - return out, svc.db.Transaction(func() (err error) { - var currentUserID = repository.Identity(svc.ctx) - - // @todo [SECURITY] verify if current user can send direct messages to anyone? - if false { - return errors.New("Not allowed to send direct messages") - } - - // @todo [SECURITY] verify if current user can send direct messages to this user - if false { - return errors.New("Not allowed to send direct messages to this user") - } - - dch, err := svc.channel.FindDirectChannelByUserID(currentUserID, recipientID) - if err == repository.ErrChannelNotFound { - dch, err = svc.channel.CreateChannel(&types.Channel{ - Type: types.ChannelTypeGroup, - }) - - if err != nil { - return - } - - membership := &types.ChannelMember{ChannelID: dch.ID, Type: types.ChannelMembershipTypeOwner} - - membership.UserID = currentUserID - if _, err = svc.cmember.Create(membership); err != nil { - return - } - - membership.UserID = recipientID - if _, err = svc.cmember.Create(membership); err != nil { - return - } - - } else if err != nil { - return errors.Wrap(err, "Could not send direct message") - } - - // Make sure our message is sent to the right channel - in.ChannelID = dch.ID - in.UserID = currentUserID - in.Type = types.MessageTypeSimpleMessage - - // @todo send new msg to the event-loop - out, err = svc.message.CreateMessage(in) - if err != nil { - return - } - - if err = svc.cview.Inc(in.ChannelID, in.UserID); err != nil { - return err - } - - return svc.sendEvent(out) - }) -} - func (svc *message) Create(mod *types.Message) (message *types.Message, err error) { // @todo get user from context var currentUserID uint64 = repository.Identity(svc.ctx) diff --git a/sam/types/channel_member.go b/sam/types/channel_member.go index c347227bc..0e56a9eed 100644 --- a/sam/types/channel_member.go +++ b/sam/types/channel_member.go @@ -55,6 +55,17 @@ func (mm ChannelMemberSet) MembersOf(channelID uint64) []uint64 { return mmof } +// AllMemberIDs returns IDs of all members +func (mm ChannelMemberSet) AllMemberIDs() []uint64 { + var mmof = make([]uint64, 0) + + for i := range mm { + mmof = append(mmof, mm[i].UserID) + } + + return mmof +} + func (uu ChannelMemberSet) FindByUserId(userID uint64) *ChannelMember { for i := range uu { if uu[i].UserID == userID {