From ecb10ed62fdff179e34f82b0ff2569da8390f4ad Mon Sep 17 00:00:00 2001 From: Saturnino Abril Date: Sat, 22 Apr 2017 21:52:03 +0900 Subject: APIv4 DELETE /users/{user_id}/posts/{post_id}/reactions/name (#6117) * APIv4 DELETE /users/{user_id}/posts/{post_id}/reactions/name * updated v3 deleteReaction endpoint * update parameter of app.DeleteReactionForPost() * update utils.IsValidAlphaNum, add utils.IsValidAlphaNumHyphenUnderscore, and add related tests --- api/reaction.go | 23 ++---- api4/api.go | 6 ++ api4/context.go | 16 ++++- api4/params.go | 5 ++ api4/reaction.go | 42 +++++++++++ api4/reaction_test.go | 124 ++++++++++++++++++++++++++++++++ app/reaction.go | 17 +++++ model/client4.go | 10 +++ model/emoji.go | 2 +- model/emoji_test.go | 20 ++++++ model/reaction.go | 2 +- model/reaction_test.go | 15 ++++ model/team.go | 2 +- model/utils.go | 24 +++---- model/utils_test.go | 188 +++++++++++++++++++++++++++++++++++++++++++++++++ 15 files changed, 459 insertions(+), 37 deletions(-) diff --git a/api/reaction.go b/api/reaction.go index 9def7274a..e6f7e845d 100644 --- a/api/reaction.go +++ b/api/reaction.go @@ -106,28 +106,13 @@ func deleteReaction(c *Context, w http.ResponseWriter, r *http.Request) { return } - var post *model.Post - - if result := <-app.Srv.Store.Post().Get(reaction.PostId); result.Err != nil { - c.Err = result.Err - return - } else if post = result.Data.(*model.PostList).Posts[postId]; post.ChannelId != channelId { - c.Err = model.NewLocAppError("deleteReaction", "api.reaction.delete_reaction.mismatched_channel_id.app_error", - nil, "channelId="+channelId+", post.ChannelId="+post.ChannelId+", postId="+postId) - c.Err.StatusCode = http.StatusBadRequest + err := app.DeleteReactionForPost(reaction) + if err != nil { + c.Err = err return } - if result := <-app.Srv.Store.Reaction().Delete(reaction); result.Err != nil { - c.Err = result.Err - return - } else { - go sendReactionEvent(model.WEBSOCKET_EVENT_REACTION_REMOVED, channelId, reaction, post) - - app.InvalidateCacheForReactions(reaction.PostId) - - ReturnStatusOK(w) - } + ReturnStatusOK(w) } func sendReactionEvent(event string, channelId string, reaction *model.Reaction, post *model.Post) { diff --git a/api4/api.go b/api4/api.go index fd9b679d2..e25494f93 100644 --- a/api4/api.go +++ b/api4/api.go @@ -48,6 +48,7 @@ type Routes struct { Post *mux.Router // 'api/v4/posts/{post_id:[A-Za-z0-9]+}' PostsForChannel *mux.Router // 'api/v4/channels/{channel_id:[A-Za-z0-9]+}/posts' PostsForUser *mux.Router // 'api/v4/users/{user_id:[A-Za-z0-9]+}/posts' + PostForUser *mux.Router // 'api/v4/users/{user_id:[A-Za-z0-9]+}/posts/{post_id:[A-Za-z0-9]+}' Files *mux.Router // 'api/v4/files' File *mux.Router // 'api/v4/files/{file_id:[A-Za-z0-9]+}' @@ -89,6 +90,8 @@ type Routes struct { Emojis *mux.Router // 'api/v4/emoji' Emoji *mux.Router // 'api/v4/emoji/{emoji_id:[A-Za-z0-9]+}' + ReactionByNameForPostForUser *mux.Router // 'api/v4/users/{user_id:[A-Za-z0-9]+}/posts/{post_id:[A-Za-z0-9]+}/reactions/{emoji_name:[A-Za-z0-9_-]+}' + Webrtc *mux.Router // 'api/v4/webrtc' } @@ -132,6 +135,7 @@ func InitApi(full bool) { BaseRoutes.Post = BaseRoutes.Posts.PathPrefix("/{post_id:[A-Za-z0-9]+}").Subrouter() BaseRoutes.PostsForChannel = BaseRoutes.Channel.PathPrefix("/posts").Subrouter() BaseRoutes.PostsForUser = BaseRoutes.User.PathPrefix("/posts").Subrouter() + BaseRoutes.PostForUser = BaseRoutes.PostsForUser.PathPrefix("/{post_id:[A-Za-z0-9]+}").Subrouter() BaseRoutes.Files = BaseRoutes.ApiRoot.PathPrefix("/files").Subrouter() BaseRoutes.File = BaseRoutes.Files.PathPrefix("/{file_id:[A-Za-z0-9]+}").Subrouter() @@ -166,6 +170,8 @@ func InitApi(full bool) { BaseRoutes.Emojis = BaseRoutes.ApiRoot.PathPrefix("/emoji").Subrouter() BaseRoutes.Emoji = BaseRoutes.Emojis.PathPrefix("/{emoji_id:[A-Za-z0-9]+}").Subrouter() + BaseRoutes.ReactionByNameForPostForUser = BaseRoutes.PostForUser.PathPrefix("/reactions/{emoji_name:[A-Za-z0-9_-]+}").Subrouter() + BaseRoutes.Webrtc = BaseRoutes.ApiRoot.PathPrefix("/webrtc").Subrouter() InitUser() diff --git a/api4/context.go b/api4/context.go index f492f2b99..c7fba4f5f 100644 --- a/api4/context.go +++ b/api4/context.go @@ -468,7 +468,7 @@ func (c *Context) RequireCategory() *Context { return c } - if !model.IsValidAlphaNum(c.Params.Category, true) { + if !model.IsValidAlphaNumHyphenUnderscore(c.Params.Category, true) { c.SetInvalidUrlParam("category") } @@ -492,13 +492,25 @@ func (c *Context) RequirePreferenceName() *Context { return c } - if !model.IsValidAlphaNum(c.Params.PreferenceName, true) { + if !model.IsValidAlphaNumHyphenUnderscore(c.Params.PreferenceName, true) { c.SetInvalidUrlParam("preference_name") } return c } +func (c *Context) RequireEmojiName() *Context { + if c.Err != nil { + return c + } + + if len(c.Params.EmojiName) == 0 || len(c.Params.EmojiName) > 64 || !model.IsValidAlphaNumHyphenUnderscore(c.Params.EmojiName, false) { + c.SetInvalidUrlParam("emoji_name") + } + + return c +} + func (c *Context) RequireHookId() *Context { if c.Err != nil { return c diff --git a/api4/params.go b/api4/params.go index a1c829f1c..5febf06fb 100644 --- a/api4/params.go +++ b/api4/params.go @@ -32,6 +32,7 @@ type ApiParams struct { TeamName string ChannelName string PreferenceName string + EmojiName string Category string Service string Page int @@ -111,6 +112,10 @@ func ApiParamsFromRequest(r *http.Request) *ApiParams { params.PreferenceName = val } + if val, ok := props["emoji_name"]; ok { + params.EmojiName = val + } + if val, err := strconv.Atoi(r.URL.Query().Get("page")); err != nil || val < 0 { params.Page = PAGE_DEFAULT } else { diff --git a/api4/reaction.go b/api4/reaction.go index 7d5952eea..6605eb070 100644 --- a/api4/reaction.go +++ b/api4/reaction.go @@ -17,6 +17,7 @@ func InitReaction() { BaseRoutes.Reactions.Handle("", ApiSessionRequired(saveReaction)).Methods("POST") BaseRoutes.Post.Handle("/reactions", ApiSessionRequired(getReactions)).Methods("GET") + BaseRoutes.ReactionByNameForPostForUser.Handle("", ApiSessionRequired(deleteReaction)).Methods("DELETE") } func saveReaction(c *Context, w http.ResponseWriter, r *http.Request) { @@ -71,3 +72,44 @@ func getReactions(c *Context, w http.ResponseWriter, r *http.Request) { return } } + +func deleteReaction(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireUserId() + if c.Err != nil { + return + } + + c.RequirePostId() + if c.Err != nil { + return + } + + c.RequireEmojiName() + if c.Err != nil { + return + } + + if !app.SessionHasPermissionToChannelByPost(c.Session, c.Params.PostId, model.PERMISSION_READ_CHANNEL) { + c.SetPermissionError(model.PERMISSION_READ_CHANNEL) + return + } + + if c.Params.UserId != c.Session.UserId && !app.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) { + c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM) + return + } + + reaction := &model.Reaction{ + UserId: c.Params.UserId, + PostId: c.Params.PostId, + EmojiName: c.Params.EmojiName, + } + + err := app.DeleteReactionForPost(reaction) + if err != nil { + c.Err = err + return + } + + ReturnStatusOK(w) +} diff --git a/api4/reaction_test.go b/api4/reaction_test.go index 980a96d68..b80c96118 100644 --- a/api4/reaction_test.go +++ b/api4/reaction_test.go @@ -193,3 +193,127 @@ func TestGetReactions(t *testing.T) { _, resp = th.SystemAdminClient.GetReactions(postId) CheckNoError(t, resp) } + +func TestDeleteReaction(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer TearDown() + Client := th.Client + userId := th.BasicUser.Id + user2Id := th.BasicUser2.Id + postId := th.BasicPost.Id + + r1 := &model.Reaction{ + UserId: userId, + PostId: postId, + EmojiName: "smile", + } + + app.SaveReactionForPost(r1) + if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 1 { + t.Fatal("didn't save reaction correctly") + } + + ok, resp := Client.DeleteReaction(r1) + CheckNoError(t, resp) + + if !ok { + t.Fatal("should have returned true") + } + + if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 0 { + t.Fatal("should have deleted reaction") + } + + // deleting one reaction when a post has multiple reactions + r2 := &model.Reaction{ + UserId: userId, + PostId: postId, + EmojiName: "smile-", + } + + app.SaveReactionForPost(r1) + app.SaveReactionForPost(r2) + if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 2 { + t.Fatal("didn't save reactions correctly") + } + + _, resp = Client.DeleteReaction(r2) + CheckNoError(t, resp) + + if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 1 || *reactions[0] != *r1 { + t.Fatal("should have deleted 1 reaction only") + } + + // deleting a reaction made by another user + r3 := &model.Reaction{ + UserId: user2Id, + PostId: postId, + EmojiName: "smile_", + } + + th.LoginBasic2() + app.SaveReactionForPost(r3) + if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 2 { + t.Fatal("didn't save reaction correctly") + } + + th.LoginBasic() + + ok, resp = Client.DeleteReaction(r3) + CheckForbiddenStatus(t, resp) + + if ok { + t.Fatal("should have returned false") + } + + if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 2 { + t.Fatal("should have not deleted a reaction") + } + + r1.PostId = GenerateTestId() + _, resp = Client.DeleteReaction(r1) + CheckForbiddenStatus(t, resp) + + r1.PostId = "junk" + + _, resp = Client.DeleteReaction(r1) + CheckBadRequestStatus(t, resp) + + r1.PostId = postId + r1.UserId = GenerateTestId() + + _, resp = Client.DeleteReaction(r1) + CheckForbiddenStatus(t, resp) + + r1.UserId = "junk" + + _, resp = Client.DeleteReaction(r1) + CheckBadRequestStatus(t, resp) + + r1.UserId = userId + r1.EmojiName = "" + + _, resp = Client.DeleteReaction(r1) + CheckNotFoundStatus(t, resp) + + r1.EmojiName = strings.Repeat("a", 65) + + _, resp = Client.DeleteReaction(r1) + CheckBadRequestStatus(t, resp) + + Client.Logout() + r1.EmojiName = "smile" + + _, resp = Client.DeleteReaction(r1) + CheckUnauthorizedStatus(t, resp) + + _, resp = th.SystemAdminClient.DeleteReaction(r1) + CheckNoError(t, resp) + + _, resp = th.SystemAdminClient.DeleteReaction(r3) + CheckNoError(t, resp) + + if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 0 { + t.Fatal("should have deleted both reactions") + } +} diff --git a/app/reaction.go b/app/reaction.go index eb542286f..cc57e1c4c 100644 --- a/app/reaction.go +++ b/app/reaction.go @@ -34,6 +34,23 @@ func GetReactionsForPost(postId string) ([]*model.Reaction, *model.AppError) { } } +func DeleteReactionForPost(reaction *model.Reaction) *model.AppError { + post, err := GetSinglePost(reaction.PostId) + if err != nil { + return err + } + + if result := <-Srv.Store.Reaction().Delete(reaction); result.Err != nil { + return result.Err + } else { + go sendReactionEvent(model.WEBSOCKET_EVENT_REACTION_REMOVED, reaction, post) + + InvalidateCacheForReactions(reaction.PostId) + } + + return nil +} + func sendReactionEvent(event string, reaction *model.Reaction, post *model.Post) { // send out that a reaction has been added/removed message := model.NewWebSocketEvent(event, "", post.ChannelId, "", nil) diff --git a/model/client4.go b/model/client4.go index ac5ebf03e..1a2ce523b 100644 --- a/model/client4.go +++ b/model/client4.go @@ -2533,3 +2533,13 @@ func (c *Client4) GetReactions(postId string) ([]*Reaction, *Response) { return ReactionsFromJson(r.Body), BuildResponse(r) } } + +// DeleteReaction deletes reaction of a user in a post. +func (c *Client4) DeleteReaction(reaction *Reaction) (bool, *Response) { + if r, err := c.DoApiDelete(c.GetUserRoute(reaction.UserId) + c.GetPostRoute(reaction.PostId) + fmt.Sprintf("/reactions/%v", reaction.EmojiName)); err != nil { + return false, &Response{StatusCode: r.StatusCode, Error: err} + } else { + defer closeBody(r) + return CheckStatusOK(r), BuildResponse(r) + } +} diff --git a/model/emoji.go b/model/emoji.go index 7f2792777..5ade868cd 100644 --- a/model/emoji.go +++ b/model/emoji.go @@ -34,7 +34,7 @@ func (emoji *Emoji) IsValid() *AppError { return NewLocAppError("Emoji.IsValid", "model.emoji.user_id.app_error", nil, "") } - if len(emoji.Name) == 0 || len(emoji.Name) > 64 { + if len(emoji.Name) == 0 || len(emoji.Name) > 64 || !IsValidAlphaNumHyphenUnderscore(emoji.Name, false) { return NewLocAppError("Emoji.IsValid", "model.emoji.name.app_error", nil, "") } diff --git a/model/emoji_test.go b/model/emoji_test.go index 81de50c6b..1e1c46714 100644 --- a/model/emoji_test.go +++ b/model/emoji_test.go @@ -56,8 +56,28 @@ func TestEmojiIsValid(t *testing.T) { t.Fatal() } + emoji.Name = "" + if err := emoji.IsValid(); err == nil { + t.Fatal(err) + } + emoji.Name = strings.Repeat("1", 64) if err := emoji.IsValid(); err != nil { t.Fatal(err) } + + emoji.Name = "name-" + if err := emoji.IsValid(); err != nil { + t.Fatal(err) + } + + emoji.Name = "name_" + if err := emoji.IsValid(); err != nil { + t.Fatal(err) + } + + emoji.Name = "name:" + if err := emoji.IsValid(); err == nil { + t.Fatal(err) + } } diff --git a/model/reaction.go b/model/reaction.go index 4a02df8cb..5dbf07152 100644 --- a/model/reaction.go +++ b/model/reaction.go @@ -60,7 +60,7 @@ func (o *Reaction) IsValid() *AppError { return NewLocAppError("Reaction.IsValid", "model.reaction.is_valid.post_id.app_error", nil, "post_id="+o.PostId) } - if len(o.EmojiName) == 0 || len(o.EmojiName) > 64 { + if len(o.EmojiName) == 0 || len(o.EmojiName) > 64 || !IsValidAlphaNumHyphenUnderscore(o.EmojiName, false) { return NewLocAppError("Reaction.IsValid", "model.reaction.is_valid.emoji_name.app_error", nil, "emoji_name="+o.EmojiName) } diff --git a/model/reaction_test.go b/model/reaction_test.go index e980b106d..e447e4329 100644 --- a/model/reaction_test.go +++ b/model/reaction_test.go @@ -57,6 +57,21 @@ func TestReactionIsValid(t *testing.T) { t.Fatal(err) } + reaction.EmojiName = "emoji-" + if err := reaction.IsValid(); err != nil { + t.Fatal(err) + } + + reaction.EmojiName = "emoji_" + if err := reaction.IsValid(); err != nil { + t.Fatal(err) + } + + reaction.EmojiName = "emoji:" + if err := reaction.IsValid(); err == nil { + t.Fatal(err) + } + reaction.CreateAt = 0 if err := reaction.IsValid(); err == nil { t.Fatal("create at should be invalid") diff --git a/model/team.go b/model/team.go index 74d371ac2..4fe03f2fc 100644 --- a/model/team.go +++ b/model/team.go @@ -233,7 +233,7 @@ func IsReservedTeamName(s string) bool { func IsValidTeamName(s string) bool { - if !IsValidAlphaNum(s, false) { + if !IsValidAlphaNum(s) { return false } diff --git a/model/utils.go b/model/utils.go index 6d8fafeea..cddf38166 100644 --- a/model/utils.go +++ b/model/utils.go @@ -297,7 +297,7 @@ var reservedName = []string{ func IsValidChannelIdentifier(s string) bool { - if !IsValidAlphaNum(s, true) { + if !IsValidAlphaNumHyphenUnderscore(s, true) { return false } @@ -308,22 +308,20 @@ func IsValidChannelIdentifier(s string) bool { return true } -var validAlphaNumUnderscore = regexp.MustCompile(`^[a-z0-9]+([a-z\-\_0-9]+|(__)?)[a-z0-9]+$`) -var validAlphaNum = regexp.MustCompile(`^[a-z0-9]+([a-z\-0-9]+|(__)?)[a-z0-9]+$`) +func IsValidAlphaNum(s string) bool { + validAlphaNum := regexp.MustCompile(`^[a-z0-9]+([a-z\-0-9]+|(__)?)[a-z0-9]+$`) -func IsValidAlphaNum(s string, allowUnderscores bool) bool { - var match bool - if allowUnderscores { - match = validAlphaNumUnderscore.MatchString(s) - } else { - match = validAlphaNum.MatchString(s) - } + return validAlphaNum.MatchString(s) +} - if !match { - return false +func IsValidAlphaNumHyphenUnderscore(s string, withFormat bool) bool { + if withFormat { + validAlphaNumHyphenUnderscore := regexp.MustCompile(`^[a-z0-9]+([a-z\-\_0-9]+|(__)?)[a-z0-9]+$`) + return validAlphaNumHyphenUnderscore.MatchString(s) } - return true + validSimpleAlphaNumHyphenUnderscore := regexp.MustCompile(`^[a-zA-Z0-9\-_]+$`) + return validSimpleAlphaNumHyphenUnderscore.MatchString(s) } func Etag(parts ...interface{}) string { diff --git a/model/utils_test.go b/model/utils_test.go index e77ce80fb..94ee55aa9 100644 --- a/model/utils_test.go +++ b/model/utils_test.go @@ -137,3 +137,191 @@ func TestParseHashtags(t *testing.T) { } } } + +func TestIsValidAlphaNum(t *testing.T) { + cases := []struct { + Input string + Result bool + }{ + { + Input: "test", + Result: true, + }, + { + Input: "test-name", + Result: true, + }, + { + Input: "test--name", + Result: true, + }, + { + Input: "test__name", + Result: true, + }, + { + Input: "-", + Result: false, + }, + { + Input: "__", + Result: false, + }, + { + Input: "test-", + Result: false, + }, + { + Input: "test--", + Result: false, + }, + { + Input: "test__", + Result: false, + }, + { + Input: "test:name", + Result: false, + }, + } + + for _, tc := range cases { + actual := IsValidAlphaNum(tc.Input) + if actual != tc.Result { + t.Fatalf("case: %v\tshould returned: %#v", tc, tc.Result) + } + } +} + +func TestIsValidAlphaNumHyphenUnderscore(t *testing.T) { + casesWithFormat := []struct { + Input string + Result bool + }{ + { + Input: "test", + Result: true, + }, + { + Input: "test-name", + Result: true, + }, + { + Input: "test--name", + Result: true, + }, + { + Input: "test__name", + Result: true, + }, + { + Input: "test_name", + Result: true, + }, + { + Input: "test_-name", + Result: true, + }, + { + Input: "-", + Result: false, + }, + { + Input: "__", + Result: false, + }, + { + Input: "test-", + Result: false, + }, + { + Input: "test--", + Result: false, + }, + { + Input: "test__", + Result: false, + }, + { + Input: "test:name", + Result: false, + }, + } + + for _, tc := range casesWithFormat { + actual := IsValidAlphaNumHyphenUnderscore(tc.Input, true) + if actual != tc.Result { + t.Fatalf("case: %v\tshould returned: %#v", tc, tc.Result) + } + } + + casesWithoutFormat := []struct { + Input string + Result bool + }{ + { + Input: "test", + Result: true, + }, + { + Input: "test-name", + Result: true, + }, + { + Input: "test--name", + Result: true, + }, + { + Input: "test__name", + Result: true, + }, + { + Input: "test_name", + Result: true, + }, + { + Input: "test_-name", + Result: true, + }, + { + Input: "-", + Result: true, + }, + { + Input: "_", + Result: true, + }, + { + Input: "test-", + Result: true, + }, + { + Input: "test--", + Result: true, + }, + { + Input: "test__", + Result: true, + }, + { + Input: ".", + Result: false, + }, + + { + Input: "test,", + Result: false, + }, + { + Input: "test:name", + Result: false, + }, + } + + for _, tc := range casesWithoutFormat { + actual := IsValidAlphaNumHyphenUnderscore(tc.Input, false) + if actual != tc.Result { + t.Fatalf("case: '%v'\tshould returned: %#v", tc.Input, tc.Result) + } + } +} -- cgit v1.2.3-1-g7c22