Skip to content

Commit

Permalink
fix(server): plugin update cause layer corruption (#431)
Browse files Browse the repository at this point in the history
  • Loading branch information
yk-eukarya committed Jun 1, 2023
1 parent 7c89bfd commit f2dd7be
Show file tree
Hide file tree
Showing 17 changed files with 455 additions and 150 deletions.
6 changes: 3 additions & 3 deletions server/internal/usecase/interactor/scene_plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ func (i *Scene) UpgradePlugin(ctx context.Context, sid id.SceneID, oldPluginID,
}

result, err := pluginMigrator.MigratePlugins(ctx, s, oldPluginID, newPluginID)
if err != nil {
return nil, err
}

if err := i.sceneRepo.Save(ctx, result.Scene); err != nil {
return nil, err
Expand All @@ -230,9 +233,6 @@ func (i *Scene) UpgradePlugin(ctx context.Context, sid id.SceneID, oldPluginID,
if err := i.layerRepo.SaveAll(ctx, result.Layers); err != nil {
return nil, err
}
if err := i.layerRepo.RemoveAll(ctx, result.RemovedLayers); err != nil {
return nil, err
}
if err := i.propertyRepo.RemoveAll(ctx, result.RemovedProperties); err != nil {
return nil, err
}
Expand Down
52 changes: 40 additions & 12 deletions server/internal/usecase/interactor/scene_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,14 +346,24 @@ func TestScene_UpgradePlugin(t *testing.T) {
pl2ps := property.NewSchema().ID(id.NewPropertySchemaID(pid2, "@")).MustBuild()
psr := memory.NewPropertySchemaWith(pl1ps, pl2ps)

pl1 := plugin.New().ID(pid1).Schema(pl1ps.ID().Ref()).MustBuild()
pl2 := plugin.New().ID(pid2).Schema(pl2ps.ID().Ref()).MustBuild()
pl1 := plugin.New().ID(pid1).Schema(pl1ps.ID().Ref()).Extensions([]*plugin.Extension{
plugin.NewExtension().ID("a").Type(plugin.ExtensionTypeBlock).Schema(pl1ps.ID()).MustBuild(),
}).MustBuild()
pl2 := plugin.New().ID(pid2).Schema(pl2ps.ID().Ref()).Extensions([]*plugin.Extension{
plugin.NewExtension().ID("a").Type(plugin.ExtensionTypeBlock).Schema(pl2ps.ID()).MustBuild(),
}).MustBuild()
pr := memory.NewPluginWith(pl1, pl2)

pl1p := property.New().NewID().Scene(sid).Schema(*pl1.Schema()).MustBuild()
prr := memory.NewPropertyWith(pl1p)

lr := memory.NewLayerWith()
pl2p := property.New().NewID().Scene(sid).Schema(*pl1.Schema()).MustBuild()
prr := memory.NewPropertyWith(pl1p, pl2p)

ibf1 := layer.NewInfoboxField().NewID().Plugin(plugin.OfficialPluginID).Extension("textblock").Property(id.NewPropertyID()).MustBuild()
ibf2 := layer.NewInfoboxField().NewID().Plugin(pid1).Extension("a").Property(pl2p.ID()).MustBuild()
ib := layer.NewInfobox([]*layer.InfoboxField{ibf1, ibf2}, id.NewPropertyID())
l1 := layer.New().NewID().Plugin(plugin.OfficialPluginID.Ref()).Scene(sid).Infobox(ib).Item().MustBuild()
l2 := layer.New().NewID().Plugin(plugin.OfficialPluginID.Ref()).Scene(sid).Group().Layers(layer.NewIDList([]layer.ID{l1.ID()})).MustBuild()
lr := memory.NewLayerWith(l1, l2)

dsr := memory.NewDataset()

Expand Down Expand Up @@ -384,14 +394,32 @@ func TestScene_UpgradePlugin(t *testing.T) {
if tt.wantErr != nil {
assert.Equal(tt.wantErr, err)
assert.Nil(gotSc)
} else {
assert.NoError(err)
assert.Same(sc, gotSc)
assert.False(gotSc.Plugins().Has(tt.args.old))
assert.True(gotSc.Plugins().Has(tt.args.new))
p, _ := prr.FindByID(ctx, *gotSc.Plugins().Plugin(tt.args.new).Property())
assert.Equal(*pl2.Schema(), p.Schema())
return
}

assert.NoError(err)
assert.Same(sc, gotSc)
assert.False(gotSc.Plugins().Has(tt.args.old))
assert.True(gotSc.Plugins().Has(tt.args.new))
p, _ := prr.FindByID(ctx, *gotSc.Plugins().Plugin(tt.args.new).Property())
assert.Equal(*pl2.Schema(), p.Schema())

// layers plugin id should not be changed
ls, err := lr.FindByScene(ctx, sid)
assert.NoError(err)
for _, l := range ls {
assert.Equal(plugin.OfficialPluginID.Ref(), (*l).Plugin())
}

// layer > infobox > field
ll1 := *ls.Find(l1.ID())
assert.NotNil(ll1.Infobox().Field(ibf1.ID()))
assert.Equal(id.OfficialPluginID, ll1.Infobox().Field(ibf1.ID()).Plugin())
assert.NotNil(ll1.Infobox().Field(ibf2.ID()))
assert.Equal(tt.args.new, ll1.Infobox().Field(ibf2.ID()).Plugin())
prop, err := prr.FindByID(ctx, ll1.Infobox().Field(ibf2.ID()).Property())
assert.NoError(err)
assert.Equal(tt.args.new, prop.Schema().Plugin())
})
}
}
Expand Down
58 changes: 58 additions & 0 deletions server/pkg/id/property_schema_list.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package id

type PropertySchemaIDList []PropertySchemaID

// Clone duplicates the PropertySchemaIDList
func (l PropertySchemaIDList) Clone() PropertySchemaIDList {
if l == nil {
return nil
}
l2 := make(PropertySchemaIDList, len(l))
for i, id := range l {
l2[i] = id.Clone()
}
return l2
}

// Merge merges PropertySchemaIDList
func (l PropertySchemaIDList) Merge(l2 PropertySchemaIDList) PropertySchemaIDList {
if l == nil {
return l2.Clone()
}
l3 := l.Clone()
if l2 == nil {
return l3
}
l3 = append(l3, l2...)
return l3
}

// MergeUnique merges PropertySchemaIDList
func (l PropertySchemaIDList) MergeUnique(l2 PropertySchemaIDList) PropertySchemaIDList {
if l == nil {
return l2.Clone()
}
l3 := l.Clone()
if l2 == nil {
return l3
}
for _, id := range l2 {
if !l3.Contains(id) {
l3 = append(l3, id)
}
}
return l3
}

// Contains checks if PropertySchemaIDList contains PropertySchemaID
func (l PropertySchemaIDList) Contains(id PropertySchemaID) bool {
if l == nil {
return false
}
for _, id2 := range l {
if id2 == id {
return true
}
}
return false
}
182 changes: 182 additions & 0 deletions server/pkg/id/property_schema_list_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
package id

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestPropertySchemaIDList_Clone(t *testing.T) {
tests := []struct {
name string
l PropertySchemaIDList
want PropertySchemaIDList
}{
{
name: "nil",
l: nil,
want: nil,
},
{
name: "empty",
l: PropertySchemaIDList{},
want: PropertySchemaIDList{},
},
{
name: "normal",
l: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
want: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := tt.l.Clone()
assert.Equal(t, tt.want, got)
assert.NotSame(t, tt.want, got)
})
}
}

func TestPropertySchemaIDList_Contains(t *testing.T) {
type args struct {
id PropertySchemaID
}
tests := []struct {
name string
l PropertySchemaIDList
args args
want bool
}{
{
name: "nil",
l: nil,
args: args{MustPropertySchemaID("hoge~0.1.0/a")},
want: false,
},
{
name: "empty",
l: PropertySchemaIDList{},
args: args{MustPropertySchemaID("hoge~0.1.0/a")},
want: false,
},
{
name: "normal",
l: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
args: args{MustPropertySchemaID("hoge~0.1.0/a")},
want: true,
},
{
name: "not found",
l: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
args: args{MustPropertySchemaID("hoge~0.1.0/c")},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equalf(t, tt.want, tt.l.Contains(tt.args.id), "Contains(%v)", tt.args.id)
})
}
}

func TestPropertySchemaIDList_Merge(t *testing.T) {
type args struct {
l2 PropertySchemaIDList
}
tests := []struct {
name string
l PropertySchemaIDList
args args
want PropertySchemaIDList
}{
{
name: "nil",
l: nil,
args: args{PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")}},
want: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
},
{
name: "empty",
l: PropertySchemaIDList{},
args: args{PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")}},
want: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
},
{
name: "normal",
l: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
args: args{PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/c"), MustPropertySchemaID("hoge~0.1.0/d")}},
want: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b"), MustPropertySchemaID("hoge~0.1.0/c"), MustPropertySchemaID("hoge~0.1.0/d")},
},
{
name: "duplicated",
l: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
args: args{PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")}},
want: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b"), MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
},
{
name: "duplicated2",
l: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
args: args{PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/b"), MustPropertySchemaID("hoge~0.1.0/c")}},
want: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b"), MustPropertySchemaID("hoge~0.1.0/b"), MustPropertySchemaID("hoge~0.1.0/c")},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, tt.l.Merge(tt.args.l2))
})
}
}

func TestPropertySchemaIDList_MergeUnique(t *testing.T) {
type args struct {
l2 PropertySchemaIDList
}
tests := []struct {
name string
l PropertySchemaIDList
args args
want PropertySchemaIDList
}{
{
name: "nil",
l: nil,
args: args{PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")}},
want: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
},
{
name: "empty",
l: PropertySchemaIDList{},
args: args{PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")}},
want: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
},
{
name: "normal",
l: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
args: args{PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/c"), MustPropertySchemaID("hoge~0.1.0/d")}},
want: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b"), MustPropertySchemaID("hoge~0.1.0/c"), MustPropertySchemaID("hoge~0.1.0/d")},
},
{
name: "duplicated",
l: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
args: args{PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")}},
want: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
},
{
name: "duplicated2",
l: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b")},
args: args{PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/b"), MustPropertySchemaID("hoge~0.1.0/c")}},
want: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/b"), MustPropertySchemaID("hoge~0.1.0/c")},
},
{
name: "duplicated3",
l: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/a")},
args: args{PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/a")}},
want: PropertySchemaIDList{MustPropertySchemaID("hoge~0.1.0/a"), MustPropertySchemaID("hoge~0.1.0/a")},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equalf(t, tt.want, tt.l.MergeUnique(tt.args.l2), "MergeUnique(%v)", tt.args.l2)
})
}
}
8 changes: 5 additions & 3 deletions server/pkg/layer/id_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,21 +165,23 @@ func (l *IDList) MoveLayerAt(fromIndex int, toIndex int) {
l.layers = append(newSlice, l.layers[toIndex:]...)
}

func (l *IDList) RemoveLayer(ids ...ID) {
func (l *IDList) RemoveLayer(ids ...ID) int {
if l == nil {
return
return 0
}

removed := 0
for i := 0; i < len(l.layers); i++ {
layer := l.layers[i]
for _, id := range ids {
if layer == id {
l.RemoveLayerAt(i)
removed++
i--
break
}
}
}
return removed
}

func (l *IDList) RemoveLayerAt(index int) {
Expand Down
6 changes: 4 additions & 2 deletions server/pkg/layer/id_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,13 @@ func TestLayerIDList(t *testing.T) {

// 1, 3, 4

layers.RemoveLayer(l2)
c := layers.RemoveLayer(l2)
assert.Equal(t, 3, layers.LayerCount())
assert.Equal(t, l1, layers.LayerAt(0))
assert.Equal(t, l3, layers.LayerAt(1))
assert.Equal(t, l4, layers.LayerAt(2))
assert.False(t, layers.HasLayer(l2))
assert.Equal(t, 1, c)

// 1, 3, 4, 2

Expand All @@ -131,8 +132,9 @@ func TestLayerIDList(t *testing.T) {

// 1, 3

layers.RemoveLayer(l2, l4)
c = layers.RemoveLayer(l2, l4)
assert.Equal(t, 2, layers.LayerCount())
assert.Equal(t, l1, layers.LayerAt(0))
assert.Equal(t, l3, layers.LayerAt(1))
assert.Equal(t, 2, c)
}

0 comments on commit f2dd7be

Please sign in to comment.