Skip to content

Commit 22ce6b1

Browse files
KirillStoKirillSto
authored
Merge pull request #1460 from anyproto/go-3192-fix-table-normalization-release-6
[release 6] GO-3192 - Fix table normalization
2 parents e1e3c49 + eaf7e1b commit 22ce6b1

File tree

5 files changed

+258
-98
lines changed

5 files changed

+258
-98
lines changed

‎core/block/editor/table/block.go

+115-44
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package table
22

33
import (
4+
"errors"
45
"fmt"
56
"sort"
67

@@ -28,8 +29,8 @@ func NewBlock(b *model.Block) simple.Block {
2829

2930
type Block interface {
3031
simple.Block
31-
Normalize(s *state.State) error
3232
Duplicate(s *state.State) (newID string, visitedIds []string, blocks []simple.Block, err error)
33+
Normalize(s *state.State) error
3334
}
3435

3536
type block struct {
@@ -40,37 +41,6 @@ func (b *block) Copy() simple.Block {
4041
return NewBlock(pbtypes.CopyBlock(b.Model()))
4142
}
4243

43-
func (b *block) Normalize(s *state.State) error {
44-
tb, err := NewTable(s, b.Id)
45-
if err != nil {
46-
log.Errorf("normalize table %s: broken table state: %s", b.Model().Id, err)
47-
return nil
48-
}
49-
50-
colIdx := map[string]int{}
51-
for i, c := range tb.ColumnIDs() {
52-
colIdx[c] = i
53-
}
54-
55-
for _, rowID := range tb.RowIDs() {
56-
row := s.Get(rowID)
57-
// Fix data integrity by adding missing row
58-
if row == nil {
59-
row = makeRow(rowID)
60-
if !s.Add(row) {
61-
return fmt.Errorf("add missing row block %s", rowID)
62-
}
63-
continue
64-
}
65-
normalizeRow(s, colIdx, row)
66-
}
67-
68-
if err := normalizeRows(s, tb); err != nil {
69-
return fmt.Errorf("normalize rows: %w", err)
70-
}
71-
return nil
72-
}
73-
7444
func (b *block) Duplicate(s *state.State) (newID string, visitedIds []string, blocks []simple.Block, err error) {
7545
tb, err := NewTable(s, b.Id)
7646
if err != nil {
@@ -144,6 +114,25 @@ func (b *block) Duplicate(s *state.State) (newID string, visitedIds []string, bl
144114
return block.Model().Id, visitedIds, blocks, nil
145115
}
146116

117+
func (b *block) Normalize(s *state.State) error {
118+
tb, err := NewTable(s, b.Id)
119+
if err != nil {
120+
log.Errorf("normalize table %s: broken table state: %s", b.Id, err)
121+
if !s.Unlink(b.Id) {
122+
log.Errorf("failed to unlink table block: %s", b.Id)
123+
}
124+
return nil
125+
}
126+
127+
tb.normalizeColumns()
128+
tb.normalizeRows()
129+
if err = tb.normalizeHeaderRows(); err != nil {
130+
// actually we cannot get error here, as all rows are checked in normalizeRows
131+
log.Errorf("normalize header rows: %v", err)
132+
}
133+
return nil
134+
}
135+
147136
type rowSort struct {
148137
indices []int
149138
cells []string
@@ -164,13 +153,13 @@ func (r *rowSort) Swap(i, j int) {
164153
r.cells[i], r.cells[j] = r.cells[j], r.cells[i]
165154
}
166155

167-
func normalizeRows(s *state.State, tb *Table) error {
168-
rows := s.Get(tb.Rows().Id)
156+
func (tb Table) normalizeHeaderRows() error {
157+
rows := tb.s.Get(tb.Rows().Id)
169158

170159
var headers []string
171160
regular := make([]string, 0, len(rows.Model().ChildrenIds))
172161
for _, rowID := range rows.Model().ChildrenIds {
173-
row, err := pickRow(s, rowID)
162+
row, err := pickRow(tb.s, rowID)
174163
if err != nil {
175164
return fmt.Errorf("pick row %s: %w", rowID, err)
176165
}
@@ -182,14 +171,19 @@ func normalizeRows(s *state.State, tb *Table) error {
182171
}
183172
}
184173

185-
s.SetChildrenIds(rows.Model(), append(headers, regular...))
174+
tb.s.SetChildrenIds(rows.Model(), append(headers, regular...))
186175
return nil
187176
}
188177

189-
func normalizeRow(s *state.State, colIdx map[string]int, row simple.Block) {
178+
func (tb Table) normalizeRow(colIdx map[string]int, row simple.Block) {
190179
if row == nil || row.Model() == nil {
191180
return
192181
}
182+
183+
if colIdx == nil {
184+
colIdx = tb.MakeColumnIndex()
185+
}
186+
193187
rs := &rowSort{
194188
cells: make([]string, 0, len(row.Model().ChildrenIds)),
195189
indices: make([]int, 0, len(row.Model().ChildrenIds)),
@@ -198,15 +192,15 @@ func normalizeRow(s *state.State, colIdx map[string]int, row simple.Block) {
198192
for _, id := range row.Model().ChildrenIds {
199193
_, colID, err := ParseCellID(id)
200194
if err != nil {
201-
log.Warnf("normalize row %s: discard cell %s: invalid id", row.Model().Id, id)
195+
log.Warnf("normalize row %s: move cell %s under the table: invalid id", row.Model().Id, id)
202196
toRemove = append(toRemove, id)
203197
rs.touched = true
204198
continue
205199
}
206200

207201
v, ok := colIdx[colID]
208202
if !ok {
209-
log.Warnf("normalize row %s: discard cell %s: column %s not found", row.Model().Id, id, colID)
203+
log.Warnf("normalize row %s: move cell %s under the table: column %s not found", row.Model().Id, id, colID)
210204
toRemove = append(toRemove, id)
211205
rs.touched = true
212206
continue
@@ -217,11 +211,88 @@ func normalizeRow(s *state.State, colIdx map[string]int, row simple.Block) {
217211
sort.Sort(rs)
218212

219213
if rs.touched {
220-
if s == nil {
221-
row.Model().ChildrenIds = rs.cells
222-
} else {
223-
s.RemoveFromCache(toRemove)
224-
s.SetChildrenIds(row.Model(), rs.cells)
214+
tb.MoveBlocksUnderTheTable(toRemove...)
215+
tb.s.SetChildrenIds(row.Model(), rs.cells)
216+
}
217+
}
218+
219+
func (tb Table) normalizeColumns() {
220+
var (
221+
invalidFound bool
222+
colIds = make([]string, 0)
223+
toRemove = make([]string, 0)
224+
)
225+
226+
for _, colId := range tb.ColumnIDs() {
227+
if _, err := pickColumn(tb.s, colId); err != nil {
228+
invalidFound = true
229+
switch {
230+
case errors.Is(err, errColumnNotFound):
231+
// Fix data integrity by adding missing column
232+
log.Warnf("normalize columns '%s': column '%s' is not found: recreating it", tb.Columns().Id, colId)
233+
col := makeColumn(colId)
234+
if !tb.s.Add(col) {
235+
log.Errorf("add missing column block %s", colId)
236+
toRemove = append(toRemove, colId)
237+
continue
238+
}
239+
colIds = append(colIds, colId)
240+
case errors.Is(err, errNotAColumn):
241+
log.Warnf("normalize columns '%s': block '%s' is not a column: move it under the table", tb.Columns().Id, colId)
242+
tb.MoveBlocksUnderTheTable(colId)
243+
default:
244+
log.Errorf("pick column %s: %v", colId, err)
245+
toRemove = append(toRemove, colId)
246+
}
247+
continue
225248
}
249+
colIds = append(colIds, colId)
250+
}
251+
252+
if invalidFound {
253+
tb.s.RemoveFromCache(toRemove)
254+
tb.s.SetChildrenIds(tb.Columns(), colIds)
255+
}
256+
}
257+
258+
func (tb Table) normalizeRows() {
259+
var (
260+
invalidFound bool
261+
rowIds = make([]string, 0)
262+
toRemove = make([]string, 0)
263+
colIdx = tb.MakeColumnIndex()
264+
)
265+
266+
for _, rowId := range tb.RowIDs() {
267+
row, err := getRow(tb.s, rowId)
268+
if err != nil {
269+
invalidFound = true
270+
switch {
271+
case errors.Is(err, errRowNotFound):
272+
// Fix data integrity by adding missing row
273+
log.Warnf("normalize rows '%s': row '%s' is not found: recreating it", tb.Rows().Id, rowId)
274+
row = makeRow(rowId)
275+
if !tb.s.Add(row) {
276+
log.Errorf("add missing row block %s", rowId)
277+
toRemove = append(toRemove, rowId)
278+
continue
279+
}
280+
rowIds = append(rowIds, rowId)
281+
case errors.Is(err, errNotARow):
282+
log.Warnf("normalize rows '%s': block '%s' is not a row: move it under the table", tb.Rows().Id, rowId)
283+
tb.MoveBlocksUnderTheTable(rowId)
284+
default:
285+
log.Errorf("get row %s: %v", rowId, err)
286+
toRemove = append(toRemove, rowId)
287+
}
288+
continue
289+
}
290+
tb.normalizeRow(colIdx, row)
291+
rowIds = append(rowIds, rowId)
292+
}
293+
294+
if invalidFound {
295+
tb.s.RemoveFromCache(toRemove)
296+
tb.s.SetChildrenIds(tb.Rows(), rowIds)
226297
}
227298
}

‎core/block/editor/table/block_test.go

+76-39
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/stretchr/testify/require"
88

99
"github.com/anyproto/anytype-heart/core/block/editor/state"
10+
"github.com/anyproto/anytype-heart/core/block/simple"
1011
"github.com/anyproto/anytype-heart/core/block/simple/base"
1112
"github.com/anyproto/anytype-heart/pkg/lib/pb/model"
1213
)
@@ -18,34 +19,41 @@ func TestNormalize(t *testing.T) {
1819
want *state.State
1920
}{
2021
{
21-
name: "empty",
22+
name: "empty table should remain empty",
2223
source: mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2"}, [][]string{}),
2324
want: mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2"}, [][]string{}),
2425
},
2526
{
26-
name: "invalid ids",
27+
name: "cells with invalid ids are moved under the table",
2728
source: mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2"}, [][]string{
2829
{"row1-c11", "row1-col2"},
29-
{"row2-col3"},
30+
{"row2-col3", "cell"},
3031
}),
3132
want: mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2"}, [][]string{
32-
{"row1-col2"},
33-
{},
34-
}),
33+
{"row1-c11", "row1-col2"},
34+
{"row2-col3", "cell"},
35+
}, withChangedChildren(map[string][]string{
36+
"root": {"table", "row2-col3", "cell", "row1-c11"},
37+
"row1": {"row1-col2"},
38+
"row2": {},
39+
})),
3540
},
3641
{
37-
name: "wrong column order",
42+
name: "wrong cells order -> do sorting and move invalid cells under the table",
3843
source: mkTestTable([]string{"col1", "col2", "col3"}, []string{"row1", "row2"}, [][]string{
3944
{"row1-col3", "row1-col1", "row1-col2"},
4045
{"row2-col3", "row2-c1", "row2-col1"},
4146
}),
4247
want: mkTestTable([]string{"col1", "col2", "col3"}, []string{"row1", "row2"}, [][]string{
4348
{"row1-col1", "row1-col2", "row1-col3"},
44-
{"row2-col1", "row2-col3"},
45-
}),
49+
{"row2-col3", "row2-c1", "row2-col1"},
50+
}, withChangedChildren(map[string][]string{
51+
"root": {"table", "row2-c1"},
52+
"row2": {"row2-col1", "row2-col3"},
53+
})),
4654
},
4755
{
48-
name: "wrong place for header rows",
56+
name: "wrong place for header rows -> do sorting",
4957
source: mkTestTable([]string{"col1", "col2", "col3"}, []string{"row1", "row2", "row3"}, nil,
5058
withRowBlockContents(map[string]*model.BlockContentTableRow{
5159
"row3": {IsHeader: true},
@@ -55,45 +63,74 @@ func TestNormalize(t *testing.T) {
5563
"row3": {IsHeader: true},
5664
})),
5765
},
66+
{
67+
name: "cell is a child of rows, not row -> move under the table",
68+
source: mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2"}, [][]string{
69+
{"row1-col1", "row1-col2"}, {"row2-col1", "row2-col2"},
70+
}, withChangedChildren(map[string][]string{
71+
"rows": {"row1", "row1-col2", "row2"},
72+
"row1": {"row1-col1"},
73+
})),
74+
want: mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2"}, [][]string{
75+
{"row1-col1", "row1-col2"}, {"row2-col1", "row2-col2"},
76+
}, withChangedChildren(map[string][]string{
77+
"root": {"table", "row1-col2"},
78+
"row1": {"row1-col1"},
79+
})),
80+
},
81+
{
82+
name: "columns contain invalid children -> move under the table",
83+
source: mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2"}, [][]string{
84+
{"row1-col1", "row1-col2"}, {"row2-col1", "row2-col2"},
85+
}, withChangedChildren(map[string][]string{
86+
"columns": {"col1", "col2", "row1-col2"},
87+
"row1": {"row1-col1"},
88+
})),
89+
want: mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2"}, [][]string{
90+
{"row1-col1", "row1-col2"}, {"row2-col1", "row2-col2"},
91+
}, withChangedChildren(map[string][]string{
92+
"root": {"table", "row1-col2"},
93+
"row1": {"row1-col1"},
94+
})),
95+
},
96+
{
97+
name: "table block contains invalid children -> table is dropped",
98+
source: mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2"}, [][]string{}, withChangedChildren(map[string][]string{
99+
"table": {"columns"},
100+
})),
101+
want: state.NewDoc("root", map[string]simple.Block{"root": simple.New(&model.Block{Id: "root"})}).NewState(),
102+
},
103+
{
104+
name: "missed column is recreated",
105+
source: mkTestTable([]string{"col1"}, []string{"row1", "row2"}, [][]string{}, withChangedChildren(map[string][]string{
106+
"columns": {"col1", "col2"},
107+
})),
108+
want: mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2"}, [][]string{}),
109+
},
110+
{
111+
name: "missed row is recreated",
112+
source: mkTestTable([]string{"col1", "col2"}, []string{"row1"}, [][]string{}, withChangedChildren(map[string][]string{
113+
"rows": {"row1", "row2"},
114+
})),
115+
want: mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2"}, [][]string{}),
116+
},
58117
} {
59118
t.Run(tc.name, func(t *testing.T) {
60-
tb, err := NewTable(tc.source, "table")
119+
// given
120+
st := tc.source.Copy()
121+
tb := st.Pick("table")
122+
require.NotNil(t, tb)
61123

62-
require.NoError(t, err)
124+
// when
125+
err := tb.(Block).Normalize(st)
63126

64-
st := tc.source.Copy()
65-
err = tb.block.(Block).Normalize(st)
127+
// then
66128
require.NoError(t, err)
67-
68129
assert.Equal(t, tc.want.Blocks(), st.Blocks())
69130
})
70131
}
71132
}
72133

73-
func TestNormalizeAbsentRow(t *testing.T) {
74-
source := mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2", "row3"}, [][]string{
75-
{"row1-c11", "row1-col2"},
76-
{"row2-col3"},
77-
})
78-
source.CleanupBlock("row3")
79-
80-
want := mkTestTable([]string{"col1", "col2"}, []string{"row1", "row2", "row3"}, [][]string{
81-
{"row1-col2"},
82-
{},
83-
{},
84-
})
85-
86-
tb, err := NewTable(source, "table")
87-
88-
require.NoError(t, err)
89-
90-
st := source.Copy()
91-
err = tb.block.(Block).Normalize(st)
92-
require.NoError(t, err)
93-
94-
assert.Equal(t, want.Blocks(), st.Blocks())
95-
}
96-
97134
func TestDuplicate(t *testing.T) {
98135
s := mkTestTable([]string{"col1", "col2", "col3"}, []string{"row1", "row2"},
99136
[][]string{

0 commit comments

Comments
 (0)