-
Notifications
You must be signed in to change notification settings - Fork 73
GO-4284 Remove Relation links #2145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Coverage provided by https://github.com/seriousben/go-patch-cover-action |
@@ -296,7 +293,12 @@ func handleObjectRelation(st *state.State, oldIDtoNew map[string]string, v domai | |||
} | |||
return | |||
} | |||
objectsIDs := v.StringList() | |||
|
|||
objectsIDs, ok := v.TryStringList() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only objects relations have list in details?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, in general SringList could carry relation of every format.
However, our clients do not allow to make relations of format != object
and with StringList type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's really good, you paid attention to this code, because I don't really know, how to check format here. We could propagate store index and it would be useful only for bundled relations and relations that already present in the store. Other relations are not indexed, because they are not imported yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All we rely on here right now is cid.Parse check, but of course that is not the best check 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, we can get bundled relations format without store the same way we do it in system objects revisor. As for imported relations, we should extract their format from according snapshot and provide this information to this function. But I think, it is a separate task
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have filed a separate task on it: https://linear.app/anytype/issue/GO-5214/resolve-relation-format-from-snapshots-on-import
return s.Details().Merge(s.LocalDetails()) | ||
} | ||
|
||
func (s *State) AllRelationKeys() []domain.RelationKey { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every call it allocate 3 slices, I suggest you to use Iterators to make this code to have zero allocations. AllRelationKeys iterator will call Details.Iterate and LocalDetails.Iterate.
https://pkg.go.dev/slices#Collect is useful to collect result of iteration to slice
Also you can add new function ContainsInSeq to our library. This function will search for element in iter.Seq and have no allocations. So there will no need to convert iterator to slice and then use slices.Contains.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have also introduced State.iteratreKeys() iter.Seq[domain.Key]
method. It can be made global, but I am not sure about security
# Conflicts: # clientlibrary/service/service.pb.go # core/block/editor/archive.go # core/block/editor/smartblock/detailsinject.go # core/block/editor/smartblock/smartblock.go # core/block/editor/state/state_test.go # core/block/source/bundledobjecttype.go # core/indexer/fulltext.go # core/publish/service_test.go # pb/commands.pb.go # pb/service/mock_service/mock_ClientCommandsServer.go # pb/service/service.pb.go # pkg/lib/pb/model/models.pb.go
|
https://linear.app/anytype/issue/GO-4284/get-rid-of-relation-links