Clean up comment HasReplies cache on child comment deletion

backend/app/rest/api/rest_private_test.go

@@ -373,6 +373,109 @@ func TestRest_UpdateDelete(t *testing.T) {
 		{URL: "https://radio-t.com/blah2", Count: 0}}, j)
 }
 
+func TestRest_DeleteChildThenParent(t *testing.T) {
+	ts, _, teardown := startupT(t)
+	defer teardown()
+
+	p := store.Comment{Text: "test test #1",
+		Locator: store.Locator{SiteID: "remark42", URL: "https://radio-t.com/blah1"}}
+	idP := addComment(t, p, ts)
+
+	c1 := store.Comment{Text: "test test #1", ParentID: idP,
+		Locator: store.Locator{SiteID: "remark42", URL: "https://radio-t.com/blah1"}}
+	idC1 := addComment(t, c1, ts)
+
+	c2 := store.Comment{Text: "test test #1", ParentID: idP,
+		Locator: store.Locator{SiteID: "remark42", URL: "https://radio-t.com/blah1"}}
+	idC2 := addComment(t, c2, ts)
+
+	// check multi count equals two
+	resp, err := post(t, ts.URL+"/api/v1/counts?site=remark42", `["https://radio-t.com/blah1"]`)
+	require.NoError(t, err)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+	bb, err := io.ReadAll(resp.Body)
+	require.NoError(t, err)
+	assert.NoError(t, resp.Body.Close())
+	j := []store.PostInfo{}
+	err = json.Unmarshal(bb, &j)
+	assert.NoError(t, err)
+	assert.Equal(t, []store.PostInfo{{URL: "https://radio-t.com/blah1", Count: 3}}, j)
+
+	// update a parent comment fails after child is created
+	client := http.Client{}
+	defer client.CloseIdleConnections()
+	req, err := http.NewRequest(http.MethodPut, ts.URL+"/api/v1/comment/"+idP+"?site=remark42&url=https://radio-t.com/blah1",
+		strings.NewReader(`{"text": "updated text", "summary":"updated by user"}`))
+	require.NoError(t, err)
+	req.Header.Add("X-JWT", devToken)
+	b, err := client.Do(req)
+	require.NoError(t, err)
+	body, err := io.ReadAll(b.Body)
+	require.NoError(t, err)
+	assert.Equal(t, http.StatusBadRequest, b.StatusCode, string(body))
+	assert.NoError(t, b.Body.Close())
+
+	// delete first child comment
+	req, err = http.NewRequest(http.MethodPut, ts.URL+"/api/v1/comment/"+idC1+"?site=remark42&url=https://radio-t.com/blah1",
+		strings.NewReader(`{"delete": true, "summary":"removed by user"}`))
+	require.NoError(t, err)
+	req.Header.Add("X-JWT", devToken)
+	b, err = client.Do(req)
+	require.NoError(t, err)
+	body, err = io.ReadAll(b.Body)
+	require.NoError(t, err)
+	assert.Equal(t, http.StatusOK, b.StatusCode, string(body))
+	assert.NoError(t, b.Body.Close())
+
+	// delete a parent comment, fails as one comment child still present
+	defer client.CloseIdleConnections()
+	req, err = http.NewRequest(http.MethodPut, ts.URL+"/api/v1/comment/"+idP+"?site=remark42&url=https://radio-t.com/blah1",
+		strings.NewReader(`{"delete": true, "summary":"removed by user"}`))
+	require.NoError(t, err)
+	req.Header.Add("X-JWT", devToken)
+	b, err = client.Do(req)
+	require.NoError(t, err)
+	body, err = io.ReadAll(b.Body)
+	require.NoError(t, err)
+	assert.Equal(t, http.StatusBadRequest, b.StatusCode, string(body))
+	assert.NoError(t, b.Body.Close())
+
+	// delete second child comment, as an admin to check both deletion methods
+	req, err = http.NewRequest(http.MethodDelete,
+		fmt.Sprintf("%s/api/v1/admin/comment/%s?site=remark42&url=https://radio-t.com/blah1", ts.URL, idC2), http.NoBody)
+	require.NoError(t, err)
+	requireAdminOnly(t, req)
+	resp, err = sendReq(t, req, adminUmputunToken)
+	assert.NoError(t, err)
+	assert.NoError(t, resp.Body.Close())
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+
+	// delete a parent comment, shouldn't fail after children are deleted
+	defer client.CloseIdleConnections()
+	req, err = http.NewRequest(http.MethodPut, ts.URL+"/api/v1/comment/"+idP+"?site=remark42&url=https://radio-t.com/blah1",
+		strings.NewReader(`{"delete": true, "summary":"removed by user"}`))
+	require.NoError(t, err)
+	req.Header.Add("X-JWT", devToken)
+	b, err = client.Do(req)
+	require.NoError(t, err)
+	body, err = io.ReadAll(b.Body)
+	require.NoError(t, err)
+	assert.Equal(t, http.StatusOK, b.StatusCode, string(body))
+	assert.NoError(t, b.Body.Close())
+
+	// check multi count decremented to zero
+	resp, err = post(t, ts.URL+"/api/v1/counts?site=remark42", `["https://radio-t.com/blah1"]`)
+	require.NoError(t, err)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+	bb, err = io.ReadAll(resp.Body)
+	require.NoError(t, err)
+	assert.NoError(t, resp.Body.Close())
+	j = []store.PostInfo{}
+	err = json.Unmarshal(bb, &j)
+	assert.NoError(t, err)
+	assert.Equal(t, []store.PostInfo{{URL: "https://radio-t.com/blah1", Count: 0}}, j)
+}
+
 func TestRest_UpdateNotOwner(t *testing.T) {
 	ts, srv, teardown := startupT(t)
 	defer teardown()
@@ -396,8 +499,6 @@ func TestRest_UpdateNotOwner(t *testing.T) {
 	assert.Equal(t, http.StatusForbidden, b.StatusCode, string(body), "update from non-owner")
 	assert.Equal(t, `{"code":3,"details":"can not edit comments for other users","error":"rejected"}`+"\n", string(body))
 
-	client = http.Client{}
-	defer client.CloseIdleConnections()
 	req, err = http.NewRequest(http.MethodPut, ts.URL+"/api/v1/comment/"+id1+
 		"?site=remark42&url=https://radio-t.com/blah1", strings.NewReader(`ERRR "text":"updated text", "summary":"my"}`))
 	assert.NoError(t, err)

backend/app/store/service/service.go

@@ -496,6 +496,12 @@ func (s *DataStore) EditComment(locator store.Locator, commentID string, req Edi
 		if e := s.AdminStore.OnEvent(comment.Locator.SiteID, admin.EvDelete); e != nil {
 			log.Printf("[WARN] failed to send delete event, %s", e)
 		}
+		// clean up the comment and it's parent from cache, so that
+		// after cleaning up the child, parent won't be stuck non-deletable till cache expires
+		if s.repliesCache.LoadingCache != nil {
+			s.repliesCache.Delete(commentID)
+			s.repliesCache.Delete(comment.ParentID)
+		}
 		comment.Deleted = true
 		delReq := engine.DeleteRequest{Locator: locator, CommentID: commentID, DeleteMode: store.SoftDelete}
 		return comment, s.Engine.Delete(delReq)
@@ -739,6 +745,17 @@ func (s *DataStore) Delete(locator store.Locator, commentID string, mode store.D
 	if e := s.AdminStore.OnEvent(locator.SiteID, admin.EvDelete); e != nil {
 		log.Printf("[WARN] failed to send delete event, %s", e)
 	}
+	// get comment to learn it's parent ID
+	comment, err := s.Engine.Get(engine.GetRequest{Locator: locator, CommentID: commentID})
+	if err != nil {
+		return err
+	}
+	// clean up the comment and it's parent from cache, so that
+	// after cleaning up the child, parent won't be stuck non-deletable till cache expires
+	if s.repliesCache.LoadingCache != nil {
+		s.repliesCache.Delete(commentID)
+		s.repliesCache.Delete(comment.ParentID)
+	}
 	req := engine.DeleteRequest{Locator: locator, CommentID: commentID, DeleteMode: mode}
 	return s.Engine.Delete(req)
 }

backend/app/store/service/service_test.go

@@ -971,7 +971,7 @@ func TestService_HasReplies(t *testing.T) {
 	// two comments for https://radio-t.com, no reply
 	eng, teardown := prepStoreEngine(t)
 	defer teardown()
-	b := DataStore{Engine: eng, EditDuration: 100 * time.Millisecond,
+	b := DataStore{Engine: eng,
 		AdminStore: admin.NewStaticStore("secret 123", []string{"radio-t"}, []string{"user2"}, "[email protected]")}
 	defer b.Close()
 
@@ -982,11 +982,10 @@ func TestService_HasReplies(t *testing.T) {
 		Locator:   store.Locator{URL: "https://radio-t.com", SiteID: "radio-t"},
 		User:      store.User{ID: "user1", Name: "user name"},
 	}
-
 	assert.False(t, b.HasReplies(comment))
 
 	reply := store.Comment{
-		ID:        "123456",
+		ID:        "c-1",
 		ParentID:  "id-1",
 		Text:      "some text",
 		Timestamp: time.Date(2017, 12, 20, 15, 18, 22, 0, time.Local),
@@ -995,7 +994,45 @@ func TestService_HasReplies(t *testing.T) {
 	}
 	_, err := b.Create(reply)
 	assert.NoError(t, err)
+	_, found := b.repliesCache.Peek(comment.ID)
+	assert.False(t, found, "not yet checked")
 	assert.True(t, b.HasReplies(comment))
+	_, found = b.repliesCache.Peek(reply.ParentID)
+	assert.True(t, found, "checked and has replies")
+
+	// deletion of the parent comment shouldn't work as the comment has replies
+	_, err = b.EditComment(reply.Locator, comment.ID, EditRequest{Orig: comment.ID, Delete: true, Summary: "user deletes the comment"})
+	assert.EqualError(t, err, "parent comment with reply can't be edited, "+comment.ID)
+	_, found = b.repliesCache.Peek(reply.ParentID)
+	assert.True(t, found, "checked and has replies")
+
+	// should not report replies after deletion of the child
+	err = b.Delete(reply.Locator, reply.ID, store.HardDelete)
+	assert.NoError(t, err)
+	_, found = b.repliesCache.Peek(reply.ParentID)
+	assert.False(t, found, "cleaned up from cache by Delete call")
+	assert.False(t, b.HasReplies(comment))
+	_, found = b.repliesCache.Peek(reply.ParentID)
+	assert.False(t, found, "checked and has no replies")
+
+	// recreate reply with the new ID
+	reply.ID = "c-2"
+	_, err = b.Create(reply)
+	assert.NoError(t, err)
+	_, found = b.repliesCache.Peek(reply.ParentID)
+	assert.False(t, found, "not yet checked")
+	assert.True(t, b.HasReplies(comment))
+	_, found = b.repliesCache.Peek(reply.ParentID)
+	assert.True(t, found, "checked and has replies again")
+
+	// should not report replies after deletion of the child using Edit mechanism
+	_, err = b.EditComment(reply.Locator, reply.ID, EditRequest{Orig: reply.ID, Delete: true, Summary: "user deletes the comment"})
+	assert.NoError(t, err)
+	_, found = b.repliesCache.Peek(reply.ParentID)
+	assert.False(t, found, "cleaned up from cache by EditComment call")
+	assert.False(t, b.HasReplies(comment))
+	_, found = b.repliesCache.Peek(reply.ParentID)
+	assert.False(t, found, "checked and has no replies")
 }
 
 func TestService_UserReplies(t *testing.T) {