collect /find Info for tree and plain types consistently

MakeTree calculated Info locally for historical reasons,
and the results were consistent with the dataService.Info call
but calculated differently.

That change fixes that, ensuring that Info is requested
in the same manner.
This commit is contained in:
Dmitry Verkhoturov
2024-03-16 12:49:45 -05:00
committed by Umputun
parent 1510aec17c
commit e5743185b0
7 changed files with 63 additions and 93 deletions
+5
View File
@@ -98,6 +98,11 @@ type commentsWithInfo struct {
Info store.PostInfo `json:"info,omitempty"`
}
type treeWithInfo struct {
*service.Tree
Info store.PostInfo `json:"info,omitempty"`
}
// Run the lister and request's router, activate rest server
func (s *Rest) Run(address string, port int) {
if address == "*" {
+25 -23
View File
@@ -79,34 +79,36 @@ func (s *public) findCommentsCtrl(w http.ResponseWriter, r *http.Request) {
comments = []store.Comment{} // error should clear comments and continue for post info
}
comments = s.applyView(comments, view)
var commentsInfo store.PostInfo
if info, ee := s.dataService.Info(locator, s.readOnlyAge); ee == nil {
commentsInfo = info
}
if !since.IsZero() { // if since is set, number of comments can be different from total in the DB
commentsInfo.Count = 0
for _, c := range comments {
if !c.Deleted {
commentsInfo.Count++
}
}
}
// post might be readonly without any comments, Info call will fail then and ReadOnly flag should be checked separately
if !commentsInfo.ReadOnly && locator.URL != "" && s.dataService.IsReadOnly(locator) {
commentsInfo.ReadOnly = true
}
var b []byte
switch format {
case "tree":
tree := service.MakeTree(comments, sort, s.readOnlyAge)
if tree.Nodes == nil { // eliminate json nil serialization
tree.Nodes = []*service.Node{}
withInfo := treeWithInfo{Tree: service.MakeTree(comments, sort), Info: commentsInfo}
if withInfo.Nodes == nil { // eliminate json nil serialization
withInfo.Nodes = []*service.Node{}
}
if s.dataService.IsReadOnly(locator) {
tree.Info.ReadOnly = true
}
b, e = encodeJSONWithHTML(tree)
b, e = encodeJSONWithHTML(withInfo)
default:
withInfo := commentsWithInfo{Comments: comments}
if info, ee := s.dataService.Info(locator, s.readOnlyAge); ee == nil {
withInfo.Info = info
}
if !since.IsZero() { // if since is set, number of comments can be different from total in the DB
withInfo.Info.Count = 0
for _, c := range comments {
if !c.Deleted {
withInfo.Info.Count++
}
}
}
// post might be readonly without any comments, Info call will fail then and ReadOnly flag should be checked separately
if !withInfo.Info.ReadOnly && locator.URL != "" && s.dataService.IsReadOnly(locator) {
withInfo.Info.ReadOnly = true
}
withInfo := commentsWithInfo{Comments: comments, Info: commentsInfo}
b, e = encodeJSONWithHTML(withInfo)
}
return b, e
+7 -7
View File
@@ -229,7 +229,7 @@ func TestRest_Find(t *testing.T) {
assert.Equal(t, id2, comments.Comments[0].ID)
// get in tree mode
tree := service.Tree{}
tree := treeWithInfo{}
res, code = get(t, ts.URL+"/api/v1/find?site=remark42&url=https://radio-t.com/blah1&format=tree")
assert.Equal(t, http.StatusOK, code)
err = json.Unmarshal([]byte(res), &tree)
@@ -255,7 +255,7 @@ func TestRest_FindAge(t *testing.T) {
_, err = srv.DataService.Create(c2)
require.NoError(t, err)
tree := service.Tree{}
tree := treeWithInfo{}
res, code := get(t, ts.URL+"/api/v1/find?site=remark42&url=https://radio-t.com/blah1&format=tree")
assert.Equal(t, http.StatusOK, code)
@@ -298,7 +298,7 @@ func TestRest_FindReadOnly(t *testing.T) {
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
tree := service.Tree{}
tree := treeWithInfo{}
res, code := get(t, ts.URL+"/api/v1/find?site=remark42&url=https://radio-t.com/blah1&format=tree")
assert.Equal(t, http.StatusOK, code)
err = json.Unmarshal([]byte(res), &tree)
@@ -306,7 +306,7 @@ func TestRest_FindReadOnly(t *testing.T) {
assert.Equal(t, "https://radio-t.com/blah1", tree.Info.URL)
assert.True(t, tree.Info.ReadOnly, "post is ro")
tree = service.Tree{}
tree = treeWithInfo{}
res, code = get(t, ts.URL+"/api/v1/find?site=remark42&url=https://radio-t.com/blah2&format=tree")
assert.Equal(t, http.StatusOK, code)
err = json.Unmarshal([]byte(res), &tree)
@@ -658,11 +658,11 @@ func TestPublic_FindCommentsCtrl_ConsistentCount(t *testing.T) {
{"url=test-url&since=" + sinceTS[1], fmt.Sprintf(`"info":{"url":"test-url","count":5,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
{"since=" + sinceTS[4], fmt.Sprintf(`"info":{"count":3,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[8])},
{"url=test-url&since=" + sinceTS[4], fmt.Sprintf(`"info":{"url":"test-url","count":2,"first_time":%q,"last_time":%q}`, formattedTS[0], formattedTS[7])},
{"format=tree", `"info":{"url":"test-url","count":7`},
{"format=tree", `"info":{"count":7`},
{"format=tree&url=test-url", `"info":{"url":"test-url","count":6`},
{"format=tree&sort=+time", `"info":{"url":"test-url","count":7`},
{"format=tree&sort=+time", `"info":{"count":7`},
{"format=tree&url=test-url&sort=+time", `"info":{"url":"test-url","count":6`},
{"format=tree&sort=-score", `"info":{"url":"test-url","count":7`},
{"format=tree&sort=-score", `"info":{"count":7`},
{"format=tree&url=test-url&sort=-score", `"info":{"url":"test-url","count":6`},
{"sort=+time", fmt.Sprintf(`"score":-25,"vote":0,"time":%q}],"info":{"count":7`, formattedTS[8])},
{"sort=-time", fmt.Sprintf(`"score":1,"vote":0,"time":%q}],"info":{"count":7`, formattedTS[0])},
+2 -8
View File
@@ -242,11 +242,5 @@
"time": "2017-12-25T19:47:22Z"
}
}
],
"info": {
"url": "url",
"count": 12,
"first_time": "2017-12-25T19:46:01Z",
"last_time": "2017-12-25T19:47:22Z"
}
}
]
}
+2 -8
View File
@@ -176,11 +176,5 @@
}
}]
}]
}],
"info": {
"url": "url",
"count": 8,
"first_time": "2017-12-25T19:46:01Z",
"last_time": "2017-12-25T19:47:05Z"
}
}
}]
}
+8 -29
View File
@@ -10,8 +10,7 @@ import (
// Tree is formatter making tree from the list of comments
type Tree struct {
Nodes []*Node `json:"comments"`
Info store.PostInfo `json:"info,omitempty"`
Nodes []*Node `json:"comments"`
}
// Node is a comment with optional replies
@@ -30,22 +29,12 @@ type recurData struct {
}
// MakeTree gets unsorted list of comments and produces Tree
// It will make store.PostInfo by itself and will mark Info.ReadOnly based on passed readOnlyAge
// Tree maker is local and has no access to the data store. By this reason it has to make Info and won't be able
// to handle store's read-only status. This status should be set by caller.
func MakeTree(comments []store.Comment, sortType string, readOnlyAge int) *Tree {
func MakeTree(comments []store.Comment, sortType string) *Tree {
if len(comments) == 0 {
return &Tree{}
}
res := Tree{
Info: store.PostInfo{
URL: comments[0].Locator.URL,
FirstTS: comments[0].Timestamp,
LastTS: comments[0].Timestamp,
},
}
res.Info.Count = len(res.filter(comments, func(c store.Comment) bool { return !c.Deleted }))
res := Tree{}
topComments := res.filter(comments, func(c store.Comment) bool { return c.ParentID == "" })
@@ -54,23 +43,12 @@ func MakeTree(comments []store.Comment, sortType string, readOnlyAge int) *Tree
node := Node{Comment: rootComment}
rd := recurData{}
commentsTree, tsModified, tsCreated := res.proc(comments, &node, &rd, rootComment.ID)
// skip deleted with no sub-comments ar all sub-comments deleted
commentsTree := res.proc(comments, &node, &rd, rootComment.ID)
// skip deleted with no sub-comments and all sub-comments deleted
if rootComment.Deleted && (len(commentsTree.Replies) == 0 || !rd.visible) {
continue
}
commentsTree.tsModified, commentsTree.tsCreated = tsModified, tsCreated
if commentsTree.tsCreated.Before(res.Info.FirstTS) {
res.Info.FirstTS = commentsTree.tsCreated
}
if commentsTree.tsModified.After(res.Info.LastTS) {
res.Info.LastTS = commentsTree.tsModified
}
res.Info.ReadOnly = readOnlyAge > 0 && !res.Info.FirstTS.IsZero() &&
res.Info.FirstTS.AddDate(0, 0, readOnlyAge).Before(time.Now())
res.Nodes = append(res.Nodes, commentsTree)
}
@@ -79,7 +57,7 @@ func MakeTree(comments []store.Comment, sortType string, readOnlyAge int) *Tree
}
// proc makes tree for one top-level comment recursively
func (t *Tree) proc(comments []store.Comment, node *Node, rd *recurData, parentID string) (result *Node, modified, created time.Time) {
func (t *Tree) proc(comments []store.Comment, node *Node, rd *recurData, parentID string) (result *Node) {
if rd.tsModified.IsZero() || rd.tsCreated.IsZero() {
rd.tsModified, rd.tsCreated = node.Comment.Timestamp, node.Comment.Timestamp
}
@@ -106,7 +84,8 @@ func (t *Tree) proc(comments []store.Comment, node *Node, rd *recurData, parentI
sort.Slice(node.Replies, func(i, j int) bool {
return node.Replies[i].Comment.Timestamp.Before(node.Replies[j].Comment.Timestamp)
})
return node, rd.tsModified, rd.tsCreated
node.tsModified, node.tsCreated = rd.tsModified, rd.tsCreated
return node
}
// filter returns comments for parentID
+14 -18
View File
@@ -37,19 +37,15 @@ func TestMakeTree(t *testing.T) {
{Locator: loc, ID: "611", ParentID: "61", Deleted: true},
}
res := MakeTree(comments, "time", 0)
res := MakeTree(comments, "time")
resJSON, err := json.Marshal(&res)
require.NoError(t, err)
expJSON := mustLoadJSONFile(t, "testdata/tree.json")
assert.Equal(t, expJSON, resJSON)
assert.Equal(t, store.PostInfo{URL: "url", Count: 12, FirstTS: ts(46, 1), LastTS: ts(47, 22)}, res.Info)
res = MakeTree([]store.Comment{}, "time", 0)
res = MakeTree([]store.Comment{}, "time")
assert.Equal(t, &Tree{}, res)
res = MakeTree(comments, "time", 10)
assert.Equal(t, store.PostInfo{URL: "url", Count: 12, FirstTS: ts(46, 1), LastTS: ts(47, 22), ReadOnly: true}, res.Info)
}
func TestMakeEmptySubtree(t *testing.T) {
@@ -79,7 +75,7 @@ func TestMakeEmptySubtree(t *testing.T) {
{Locator: loc, ID: "3", Timestamp: ts(48, 1), Deleted: true}, // deleted top level
}
res := MakeTree(comments, "time", 0)
res := MakeTree(comments, "time")
resJSON, err := json.Marshal(&res)
require.NoError(t, err)
t.Log(string(resJSON))
@@ -108,50 +104,50 @@ func TestTreeSortNodes(t *testing.T) {
{ID: "5", Deleted: true, Timestamp: time.Date(2017, 12, 25, 19, 47, 22, 150, time.UTC)},
}
res := MakeTree(comments, "+active", 0)
res := MakeTree(comments, "+active")
assert.Equal(t, "2", res.Nodes[0].Comment.ID)
t.Log(res.Nodes[0].Comment.ID, res.Nodes[0].tsModified)
res = MakeTree(comments, "-active", 0)
res = MakeTree(comments, "-active")
t.Log(res.Nodes[0].Comment.ID, res.Nodes[0].tsModified)
assert.Equal(t, "1", res.Nodes[0].Comment.ID)
res = MakeTree(comments, "+time", 0)
res = MakeTree(comments, "+time")
t.Log(res.Nodes[0].Comment.ID, res.Nodes[0].tsModified)
assert.Equal(t, "1", res.Nodes[0].Comment.ID)
res = MakeTree(comments, "-time", 0)
res = MakeTree(comments, "-time")
assert.Equal(t, "6", res.Nodes[0].Comment.ID)
res = MakeTree(comments, "score", 0)
res = MakeTree(comments, "score")
assert.Equal(t, "4", res.Nodes[0].Comment.ID)
assert.Equal(t, "3", res.Nodes[1].Comment.ID)
assert.Equal(t, "6", res.Nodes[2].Comment.ID)
assert.Equal(t, "1", res.Nodes[3].Comment.ID)
res = MakeTree(comments, "+score", 0)
res = MakeTree(comments, "+score")
assert.Equal(t, "4", res.Nodes[0].Comment.ID)
res = MakeTree(comments, "-score", 0)
res = MakeTree(comments, "-score")
assert.Equal(t, "2", res.Nodes[0].Comment.ID)
assert.Equal(t, "1", res.Nodes[1].Comment.ID)
assert.Equal(t, "3", res.Nodes[2].Comment.ID)
assert.Equal(t, "6", res.Nodes[3].Comment.ID)
res = MakeTree(comments, "+controversy", 0)
res = MakeTree(comments, "+controversy")
assert.Equal(t, "3", res.Nodes[0].Comment.ID)
assert.Equal(t, "6", res.Nodes[1].Comment.ID)
assert.Equal(t, "2", res.Nodes[2].Comment.ID)
assert.Equal(t, "4", res.Nodes[3].Comment.ID)
assert.Equal(t, "1", res.Nodes[4].Comment.ID)
res = MakeTree(comments, "-controversy", 0)
res = MakeTree(comments, "-controversy")
assert.Equal(t, "1", res.Nodes[0].Comment.ID)
assert.Equal(t, "4", res.Nodes[1].Comment.ID)
assert.Equal(t, "2", res.Nodes[2].Comment.ID)
assert.Equal(t, "3", res.Nodes[3].Comment.ID)
res = MakeTree(comments, "undefined", 0)
res = MakeTree(comments, "undefined")
t.Log(res.Nodes[0].Comment.ID, res.Nodes[0].tsModified)
assert.Equal(t, "1", res.Nodes[0].Comment.ID)
}
@@ -164,7 +160,7 @@ func BenchmarkTree(b *testing.B) {
assert.NoError(b, err)
for i := 0; i < b.N; i++ {
res := MakeTree(comments, "time", 0)
res := MakeTree(comments, "time")
assert.NotNil(b, res)
}
}