Merge pull request #2012 from anyproto/go-4840-set-null-detail-on-obj…

…ect-relation-add

GO-4840 Set null on ObjectRelationAdd

core/block/editor.go

@@ -469,15 +469,6 @@ func (s *Service) GetRelations(ctx session.Context, objectId string) (relations
 	return
 }
 
-func (s *Service) AddExtraRelations(ctx session.Context, objectId string, relationIds []string) (err error) {
-	if len(relationIds) == 0 {
-		return nil
-	}
-	return cache.Do(s, objectId, func(b smartblock.SmartBlock) error { // TODO RQ: check if empty
-		return b.AddRelationLinks(ctx, slice.StringsInto[domain.RelationKey](relationIds)...)
-	})
-}
-
 func (s *Service) SetObjectTypes(ctx session.Context, objectId string, objectTypeUniqueKeys []string) (err error) {
 	return cache.Do(s, objectId, func(b basic.CommonOperations) error {
 		objectTypeKeys := make([]domain.TypeKey, 0, len(objectTypeUniqueKeys))

core/block/editor/basic/details.go

@@ -73,7 +73,7 @@ func (bs *basic) UpdateDetailsAndLastUsed(update func(current *domain.Details) (
 		return err
 	}
 
-	diff := domain.StructDiff(oldDetails, newDetails)
+	diff, _ := domain.StructDiff(oldDetails, newDetails)
 	if diff.Len() == 0 {
 		return nil
 	}

core/block/editor/smartblock/smartblock.go

@@ -154,7 +154,6 @@ type SmartBlock interface {
 	History() undo.History
 	Relations(s *state.State) relationutils.Relations
 	HasRelation(s *state.State, relationKey string) bool
-	AddRelationLinks(ctx session.Context, relationKeys ...domain.RelationKey) (err error)
 	AddRelationLinksToState(s *state.State, relationKeys ...domain.RelationKey) (err error)
 	RemoveExtraRelations(ctx session.Context, relationKeys []domain.RelationKey) (err error)
 	SetVerticalAlign(ctx session.Context, align model.BlockVerticalAlign, ids ...string) error
@@ -565,13 +564,13 @@ func (sb *smartBlock) onMetaChange(details *domain.Details) {
 	id := details.GetString(bundle.RelationKeyId)
 	var msgs []*pb.EventMessage
 	if v, exists := sb.lastDepDetails[id]; exists {
-		diff := domain.StructDiff(v, details)
+		diff, keysToUnset := domain.StructDiff(v, details)
 		if id == sb.Id() {
 			// if we've got update for ourselves, we are only interested in local-only details, because the rest details changes will be appended when applying records in the current sb
 			diff = diff.CopyOnlyKeys(bundle.LocalRelationsKeys...)
 		}
 
-		msgs = append(msgs, state.StructDiffIntoEvents(sb.SpaceID(), id, diff)...)
+		msgs = append(msgs, state.StructDiffIntoEvents(sb.SpaceID(), id, diff, keysToUnset)...)
 	} else {
 		msgs = append(msgs, event.NewMessage(sb.SpaceID(), &pb.EventMessageValueOfObjectDetailsSet{
 			ObjectDetailsSet: &pb.EventObjectDetailsSet{
@@ -904,14 +903,6 @@ func (sb *smartBlock) History() undo.History {
 	return sb.undo
 }
 
-func (sb *smartBlock) AddRelationLinks(ctx session.Context, relationKeys ...domain.RelationKey) (err error) {
-	s := sb.NewStateCtx(ctx)
-	if err = sb.AddRelationLinksToState(s, relationKeys...); err != nil {
-		return
-	}
-	return sb.Apply(s)
-}
-
 func (sb *smartBlock) AddRelationLinksToState(s *state.State, relationKeys ...domain.RelationKey) (err error) {
 	if len(relationKeys) == 0 {
 		return

core/block/editor/smartblock/smarttest/smarttest.go

@@ -294,7 +294,7 @@ func (st *SmartTest) UpdateDetailsAndLastUsed(update func(current *domain.Detail
 		return err
 	}
 
-	diff := domain.StructDiff(oldDetails, newDetails)
+	diff, _ := domain.StructDiff(oldDetails, newDetails)
 	if diff == nil {
 		return nil
 	}

core/block/editor/state/event.go

@@ -2,6 +2,7 @@ package state
 
 import (
 	"fmt"
+	"slices"
 	"sort"
 
 	"github.com/anyproto/anytype-heart/core/block/simple"
@@ -15,8 +16,8 @@ import (
 	"github.com/anyproto/anytype-heart/core/block/simple/table"
 	"github.com/anyproto/anytype-heart/core/block/simple/text"
 	"github.com/anyproto/anytype-heart/core/block/simple/widget"
-	"github.com/anyproto/anytype-heart/core/event"
 	"github.com/anyproto/anytype-heart/core/domain"
+	"github.com/anyproto/anytype-heart/core/event"
 	"github.com/anyproto/anytype-heart/pb"
 	"github.com/anyproto/anytype-heart/pkg/lib/pb/model"
 	"github.com/anyproto/anytype-heart/util/slice"
@@ -314,28 +315,27 @@ func WrapEventMessages(virtual bool, msgs []*pb.EventMessage) []simple.EventMess
 	return wmsgs
 }
 
-func StructDiffIntoEvents(spaceId string, contextId string, diff *domain.Details) (msgs []*pb.EventMessage) {
-	return StructDiffIntoEventsWithSubIds(spaceId, contextId, diff, nil, nil)
+// StructDiffIntoEvents converts diff details and relation keys to unset into events
+func StructDiffIntoEvents(spaceId string, contextId string, diff *domain.Details, keysToUnset []domain.RelationKey) (msgs []*pb.EventMessage) {
+	return StructDiffIntoEventsWithSubIds(spaceId, contextId, diff, nil, keysToUnset, nil)
 }
 
-// StructDiffIntoEvents converts map into events. nil map value converts to Remove event
-func StructDiffIntoEventsWithSubIds(spaceId string, contextId string, diff *domain.Details, keys []domain.RelationKey, subIds []string) (msgs []*pb.EventMessage) {
+func StructDiffIntoEventsWithSubIds(
+	spaceId, contextId string,
+	diff *domain.Details,
+	filterKeys, keysToUnset []domain.RelationKey,
+	subIds []string,
+) (msgs []*pb.EventMessage) {
 	if diff.Len() == 0 {
 		return nil
 	}
 	var (
-		removed []string
 		details []*pb.EventObjectDetailsAmendKeyValue
 	)
 
 	for k, v := range diff.Iterate() {
 		key := string(k)
-		if len(keys) > 0 && slice.FindPos(keys, k) == -1 {
-			continue
-		}
-		// TODO This is not correct! Rewrite this code to use separate diff structures
-		if v.IsNull() {
-			removed = append(removed, key)
+		if len(filterKeys) > 0 && slice.FindPos(filterKeys, k) == -1 {
 			continue
 		}
 		details = append(details, &pb.EventObjectDetailsAmendKeyValue{Key: key, Value: v.ToProto()})
@@ -350,11 +350,15 @@ func StructDiffIntoEventsWithSubIds(spaceId string, contextId string, diff *doma
 			},
 		}))
 	}
-	if len(removed) > 0 {
+
+	filteredKeys := slices.DeleteFunc(keysToUnset, func(key domain.RelationKey) bool {
+		return !slices.Contains(filterKeys, key)
+	})
+	if len(filteredKeys) > 0 {
 		msgs = append(msgs, event.NewMessage(spaceId, &pb.EventMessageValueOfObjectDetailsUnset{
 			ObjectDetailsUnset: &pb.EventObjectDetailsUnset{
 				Id:     contextId,
-				Keys:   removed,
+				Keys:   slice.IntoStrings(keysToUnset),
 				SubIds: subIds,
 			},
 		}))

core/block/editor/state/state.go

@@ -620,9 +620,9 @@ func (s *State) apply(spaceId string, fast, one, withLayouts bool) (msgs []simpl
 	}
 	if s.parent != nil && s.details != nil {
 		prev := s.parent.Details()
-		if diff := domain.StructDiff(prev, s.details); diff != nil {
+		if diff, keysToUnset := domain.StructDiff(prev, s.details); diff != nil {
 			action.Details = &undo.Details{Before: prev.Copy(), After: s.details.Copy()}
-			msgs = append(msgs, WrapEventMessages(false, StructDiffIntoEvents(s.SpaceID(), s.RootId(), diff))...)
+			msgs = append(msgs, WrapEventMessages(false, StructDiffIntoEvents(s.SpaceID(), s.RootId(), diff, keysToUnset))...)
 			s.parent.details = s.details
 		} else if !s.details.Equal(s.parent.details) {
 			s.parent.details = s.details
@@ -651,8 +651,8 @@ func (s *State) apply(spaceId string, fast, one, withLayouts bool) (msgs []simpl
 
 	if s.parent != nil && s.localDetails != nil {
 		prev := s.parent.LocalDetails()
-		if diff := domain.StructDiff(prev, s.localDetails); diff != nil {
-			msgs = append(msgs, WrapEventMessages(true, StructDiffIntoEvents(spaceId, s.RootId(), diff))...)
+		if diff, keysToUnset := domain.StructDiff(prev, s.localDetails); diff != nil {
+			msgs = append(msgs, WrapEventMessages(true, StructDiffIntoEvents(spaceId, s.RootId(), diff, keysToUnset))...)
 			s.parent.localDetails = s.localDetails
 		} else if !s.localDetails.Equal(s.parent.localDetails) {
 			s.parent.localDetails = s.localDetails

core/domain/details.go

@@ -126,20 +126,18 @@ func setValueFromAnyEnc(d *Details, key RelationKey, val *anyenc.Value) error {
 // StructDiff returns pb struct which contains:
 // - st2 fields that not exist in st1
 // - st2 fields that not equal to ones exist in st1
-// - nil map value for st1 fields not exist in st2
+// - absentKeys are st1 fields that do not exist in st2
 // In case st1 and st2 are equal returns nil
-func StructDiff(st1, st2 *Details) *Details {
-	var diff *Details
+func StructDiff(st1, st2 *Details) (diff *Details, absentKeys []RelationKey) {
 	if st1 == nil {
-		return st2
+		return st2, nil
 	}
 	if st2 == nil {
 		diff = NewDetails()
 		for k, _ := range st1.Iterate() {
-			// TODO This is not correct, Null value could be a valid value. Just rewrite this diff and generate events logic
-			diff.Set(k, Null())
+			absentKeys = append(absentKeys, k)
 		}
-		return diff
+		return nil, absentKeys
 	}
 
 	for k2, v2 := range st2.Iterate() {
@@ -154,14 +152,11 @@ func StructDiff(st1, st2 *Details) *Details {
 
 	for k, _ := range st1.Iterate() {
 		if !st2.Has(k) {
-			if diff == nil {
-				diff = NewDetails()
-			}
-			diff.Set(k, Null())
+			absentKeys = append(absentKeys, k)
 		}
 	}
 
-	return diff
+	return diff, absentKeys
 }
 
 func DetailsListToProtos(dets []*Details) []*types.Struct {

core/domain/details_test.go

@@ -17,13 +17,14 @@ func TestStructDiff(t *testing.T) {
 		st2 *Details
 	}
 	tests := []struct {
-		name string
-		args args
-		want *Details
+		name     string
+		args     args
+		wantDiff *Details
+		wantKeys []RelationKey
 	}{
 		{"both nil",
 			args{nil, nil},
-			nil,
+			nil, nil,
 		},
 		{"equal",
 			args{
@@ -34,7 +35,7 @@ func TestStructDiff(t *testing.T) {
 					"k1": String("v1"),
 				}),
 			},
-			nil,
+			nil, nil,
 		},
 		{"nil st1", args{
 			nil,
@@ -43,7 +44,7 @@ func TestStructDiff(t *testing.T) {
 			}),
 		}, NewDetailsFromMap(map[RelationKey]Value{
 			"k1": String("v1"),
-		}),
+		}), nil,
 		},
 		{"nil map st1", args{
 			NewDetails(),
@@ -52,7 +53,7 @@ func TestStructDiff(t *testing.T) {
 			}),
 		}, NewDetailsFromMap(map[RelationKey]Value{
 			"k1": String("v1"),
-		}),
+		}), nil,
 		},
 		{"empty map st1", args{
 			NewDetailsFromMap(map[RelationKey]Value{}),
@@ -61,33 +62,29 @@ func TestStructDiff(t *testing.T) {
 			}),
 		}, NewDetailsFromMap(map[RelationKey]Value{
 			"k1": String("v1"),
-		}),
+		}), nil,
 		},
 		{"nil st2", args{
 			NewDetailsFromMap(map[RelationKey]Value{
 				"k1": String("v1"),
 			}),
 			nil,
-		}, NewDetailsFromMap(map[RelationKey]Value{
-			"k1": Null(),
-		}),
+		}, nil, []RelationKey{"k1"},
 		},
 		{"nil map st2", args{
 			NewDetailsFromMap(map[RelationKey]Value{
 				"k1": String("v1"),
 			}),
 			NewDetails(),
 		},
-			NewDetailsFromMap(map[RelationKey]Value{
-				"k1": Null(),
-			})},
+			nil, []RelationKey{"k1"},
+		},
 		{"empty map st2", args{
 			NewDetailsFromMap(map[RelationKey]Value{
 				"k1": String("v1"),
 			}),
-			NewDetailsFromMap(map[RelationKey]Value{})}, NewDetailsFromMap(map[RelationKey]Value{
-			"k1": Null(),
-		}),
+			NewDetailsFromMap(map[RelationKey]Value{})},
+			nil, []RelationKey{"k1"},
 		},
 		{"complex", args{
 			NewDetailsFromMap(map[RelationKey]Value{
@@ -100,17 +97,16 @@ func TestStructDiff(t *testing.T) {
 				"k3": String("v3_"),
 			}),
 		}, NewDetailsFromMap(map[RelationKey]Value{
-			"k2": Null(),
 			"k3": String("v3_"),
-		}),
+		}), []RelationKey{"k2"},
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := StructDiff(tt.args.st1, tt.args.st2); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("StructDiff() = %v, want %v", got, tt.want)
-			}
+			diff, keys := StructDiff(tt.args.st1, tt.args.st2)
+			assert.True(t, reflect.DeepEqual(diff, tt.wantDiff))
+			assert.True(t, reflect.DeepEqual(keys, tt.wantKeys))
 		})
 	}
 }

core/history/history_test.go

@@ -697,7 +697,7 @@ func TestHistory_DiffVersions(t *testing.T) {
 
 		// then
 		assert.Nil(t, err)
-		assert.Len(t, changes, 4)
+		assert.Len(t, changes, 3)
 	})
 	t.Run("object diff -local relations changes", func(t *testing.T) {
 		// given

core/object.go

@@ -10,6 +10,7 @@ import (
 	"github.com/samber/lo"
 
 	"github.com/anyproto/anytype-heart/core/block"
+	"github.com/anyproto/anytype-heart/core/block/detailservice"
 	importer "github.com/anyproto/anytype-heart/core/block/import"
 	"github.com/anyproto/anytype-heart/core/block/import/common"
 	"github.com/anyproto/anytype-heart/core/block/object/objectgraph"
@@ -407,29 +408,32 @@ func objectResponse(
 	return response
 }
 
-func (mw *Middleware) ObjectRelationAdd(cctx context.Context, req *pb.RpcObjectRelationAddRequest) *pb.RpcObjectRelationAddResponse {
-	ctx := mw.newContext(cctx)
-	response := func(code pb.RpcObjectRelationAddResponseErrorCode, err error) *pb.RpcObjectRelationAddResponse {
-		m := &pb.RpcObjectRelationAddResponse{Error: &pb.RpcObjectRelationAddResponseError{Code: code}}
-		if err != nil {
-			m.Error.Description = getErrorDescription(err)
-		} else {
-			m.Event = mw.getResponseEvent(ctx)
-		}
-		return m
-	}
+func (mw *Middleware) ObjectRelationAdd(_ context.Context, req *pb.RpcObjectRelationAddRequest) *pb.RpcObjectRelationAddResponse {
 	if len(req.RelationKeys) == 0 {
-		return response(pb.RpcObjectRelationAddResponseError_BAD_INPUT, fmt.Errorf("relation is nil"))
+		return &pb.RpcObjectRelationAddResponse{Error: &pb.RpcObjectRelationAddResponseError{
+			Code:        pb.RpcObjectRelationAddResponseError_BAD_INPUT,
+			Description: fmt.Errorf("relation keys list is empty").Error(),
+		}}
 	}
 
-	err := mw.doBlockService(func(bs *block.Service) (err error) {
-		return bs.AddExtraRelations(ctx, req.ContextId, req.RelationKeys)
+	detailsService := mustService[detailservice.Service](mw)
+	err := detailsService.ModifyDetails(req.ContextId, func(current *domain.Details) (*domain.Details, error) {
+		for _, key := range req.RelationKeys {
+			if current.Has(domain.RelationKey(key)) {
+				continue
+			}
+			current.Set(domain.RelationKey(key), domain.Null())
+		}
+		return current, nil
 	})
 	if err != nil {
-		return response(pb.RpcObjectRelationAddResponseError_BAD_INPUT, err)
+		return &pb.RpcObjectRelationAddResponse{Error: &pb.RpcObjectRelationAddResponseError{
+			Code:        pb.RpcObjectRelationAddResponseError_BAD_INPUT,
+			Description: getErrorDescription(err),
+		}}
 	}
 
-	return response(pb.RpcObjectRelationAddResponseError_NULL, nil)
+	return &pb.RpcObjectRelationAddResponse{Error: &pb.RpcObjectRelationAddResponseError{}}
 }
 
 func (mw *Middleware) ObjectRelationDelete(cctx context.Context, req *pb.RpcObjectRelationDeleteRequest) *pb.RpcObjectRelationDeleteResponse {