Enforced Zoned Attribute Lookups #394

Merged
dan merged 2 commits from zoned_attributes into master 2021-10-08 20:54:07 +00:00
Owner
No description provided.
dan was assigned by sarah 2021-10-07 22:41:47 +00:00
sarah added 1 commit 2021-10-07 22:41:48 +00:00
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details
aec3c40180
Enforced Zoned Attribute Lookups
Member
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/404
dan requested changes 2021-10-07 23:16:27 +00:00
dan left a comment
Owner

we need to consider migrating things somewhere like manually set group and contact names, do we want to think about it here in storage? we store a storage version number, we could move to version 2 (without having to even dup a v2, just an informal 2) but it would trigger a migration script to copy those attributes).

or

we could make it libcwtch-go's job as a one time migration. thoughts? one should handle it so we don't drop user settings

we need to consider migrating things somewhere like manually set group and contact names, do we want to think about it here in storage? we store a storage version number, we could move to version 2 (without having to even dup a v2, just an informal 2) but it would trigger a migration script to copy those attributes). or we could make it libcwtch-go's job as a one time migration. thoughts? one should handle it so we don't drop user settings
@ -51,4 +64,0 @@
}
// GetScopePath take a full path and returns the scope and the scope-less path
func GetScopePath(fullPath string) (string, string) {
Owner

why did we drop a function to peel off the scope and return a zonedPath or scope, zone and path?

why did we drop a function to peel off the scope and return a zonedPath or scope, zone and path?
Author
Owner

This basically became ParseZone. Practically we never pass scope and path together in a single string in a way that needs splitting like this.

This basically became ParseZone. Practically we never pass scope and path together in a single string in a way that needs splitting like this.
@ -35,1 +32,3 @@
return PublicScope + Separator + path
// IntoScope converts a string to a Scope
func IntoScope(scope string) Scope {
return Scope(scope)
Owner

force into one of approved scopes or error? or else into "unknown-garbage" scope?

force into one of approved scopes or error? or else into "unknown-garbage" scope?
sarah marked this conversation as resolved
@ -36,0 +35,4 @@
}
// ConstructScopedZonedPath enforces a scope over a zoned path
func (scope Scope) ConstructScopedZonedPath(zonedPath ZonedPath) string {
Owner

if we're going type heavy dont we want it to return a like ScopedZonedPath?

if we're going type heavy dont we want it to return a like ScopedZonedPath?
sarah marked this conversation as resolved
@ -41,0 +48,4 @@
func (scope Scope) IsConversation() bool {
return scope == ConversationScope
}
Owner

IsLocal() bool ?

IsLocal() bool ?
sarah marked this conversation as resolved
@ -0,0 +27,4 @@
// EnforceZone takes a path and attaches a zone to it.
// Note that this returns a ZonedPath which isn't directly usable, it must be given to ConstructScopedZonedPath
// in order to be realized into an actual attribute path.
func (zone Zone) EnforceZone(path string) ZonedPath {
Owner

why constuct for scope but enforce for zone?

why constuct for scope but enforce for zone?
sarah marked this conversation as resolved
@ -168,2 +204,4 @@
SendScopedZonedGetValToContact(handle string, scope attr.Scope, zone attr.Zone, key string)
// Deprecated
SendMessageToPeer(string, string) string
Owner

this looks like it has lots of use? we want to create a issue to start on that?

this looks like it has lots of use? we want to create a issue to start on that?
Author
Owner

Created: #395

Created: https://git.openprivacy.ca/cwtch.im/cwtch/issues/395
@ -170,3 +208,4 @@
// TODO This should probably not be exposed
// Deprecated do not use this to store messages
StoreMessage(onion string, messageTxt string, sent time.Time)
Owner

What is still using this?

case event.NewMessageFromPeer: is but it can at least be dropped from the interface than as that's internal only. it looks like libcwtch-go doesn't use it. so i think it can be droped from the interface?

What is still using this? ```case event.NewMessageFromPeer:``` is but it can at least be dropped from the interface than as that's internal only. it looks like libcwtch-go doesn't use it. so i think it can be droped from the interface?
sarah marked this conversation as resolved
Author
Owner

@dan RE: migration - this change is backwards compatible with the exception of the file sharing experiment (which, because we don't have resumption completed yet means any attributes saved are effectively meaningless anyway)

I do want to eventually move attributes like name and tag over to proper zones, but I think that can be done in a separate PR.

@dan RE: migration - this change is backwards compatible with the exception of the file sharing experiment (which, because we don't have resumption completed yet means any attributes saved are effectively meaningless anyway) I do want to eventually move attributes like name and tag over to proper zones, but I think that can be done in a separate PR.
sarah added 1 commit 2021-10-08 18:09:36 +00:00
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details
81f9d0b094
Scope/Zone API Fixes per Dan's comments
sarah changed title from WIP: Enforced Zoned Attribute Lookups to Enforced Zoned Attribute Lookups 2021-10-08 18:10:20 +00:00
Member
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/406
dan merged commit 10d407c367 into master 2021-10-08 20:54:07 +00:00
Sign in to join this conversation.
No description provided.