status-im / status-protocol-go

Status Protocol implementation in Go
Mozilla Public License 2.0
0 stars 1 forks source link

Add Init() method to Messenger #53

Closed adambabik closed 5 years ago

adambabik commented 5 years ago

As a developer, I would like to have Messenger.Init() method available so that I can initialize various objects and Whisper filters.

Problem

Currently, to load filters, a caller needs to execute Messenger.LoadFilters(). This is not needed anymore because both contacts and chats are moved to status-protocol-go.

Instead, there should be a Messenger.Init() which will load chats and contacts and initialize filters and other objects.

Acceptance Criteria

cc @cammellos

cammellos commented 5 years ago

It looks mostly good to me, not sure we will be able to get rid of LoadFilters RemoveFilters as that entails more work (they are to be called each time a new member is added to a group chat/a new chat is opened/ a contact is added), so I would start by just providing Init, get it working/integrate and take it from there, as we would still need to replace LoadFilters with something equivalent (for example returning the filters on saving a chat, but I think it's premature, as if we port more code we can stop surfacing filters all together). Also I will remove v1 as it's not strictly necessary for it.

adambabik commented 5 years ago

@cammellos I don't quite understand how contacts result in filters. We have three types of contacts: added by us, which added us, blocked. Questions Need confirmation for the following statements :D

  1. All contacts added by us should be already covered by existing AND active chats.
  2. Contacts which added us might not be covered by existing chats so these filters should be created based on a public key from Contact.Address.
  3. Contacts blocked should result in removing filters. Is it possible there is an active chat for a blocked contact and this removal should be done in Init()?

I also need possible values for Contact.SystemTags as currently it's of type []string.

cammellos commented 5 years ago

Not sure exactly what the questions are :) but all 3 statements are correct:

All contacts added by us should be already covered by existing AND active chats.

Correct

Contacts which added us might not be covered by existing chats so these filters should be created based on a public key from Contact.Address

Correct, not the address, but the public key, but yes

Contacts blocked should result in removing filters.

This is also correct, but not strictly necessary (we don't clear the filters now), but we'd need to have access to SystemTags which we don't, but that's not a big deal for now.

The flow I am sort of worried about is:

1) We receive a contact request 2) Is passed to status-react 3) Status-react process the contact request 4a) Status react loads the filters (current flow) as it stands now, the quickest change would be: 4b) Status react/client saves the contact, status-protocol-go loads the filter (this would have to be implemented)

I think neither of this flows are going to stay, as I see it, the correct flow should be eventually:

1) We receive a contact request 2) We process the contact request in status-protocol-go, save the contact, load the filters 3) status-react/client is notified of a new contact being added

but that's a bit more work, and probably best done separately, so 4b is not that appealing as we still would have to change it eventually

adambabik commented 5 years ago

This is also correct, but not strictly necessary (we don't clear the filters now), but we'd need to have access to SystemTags which we don't, but that's not a big deal for now.

Ok, so I skip removing filters for now. When provided with SystemTags (which are in Contact struct, I just have no idea of possible values) we can implement removing filters.

so 4b is not that appealing as we still would have to change it eventually

You mean "appalling"? :) We want to go with (4b) as it is the closest to the final flow you described and get rid of managing filters by status-react, right?

cammellos commented 5 years ago

You mean "appalling"? :) We want to go with (4b) as it is the closest to the final flow you described and get rid of managing filters by status-react, right?

Not sure I would go for 4b, as it won't simplify much code in status-react, and we would have to write quite a bit of code in status-protocol-go that we would change again once we do things properly, so I would skip 4b, leave things as they are (status-react calls Init on startup, but then loadfilters on saving contacts/chats/group-chats), and eventually go directly to the proper implementation, like so:

1) handle contact requests in status-protocol-go -> remove loadfilters on contact 2) handleJoin in status-protocol-go->remove loadfilters on creating a chat, 3) handleGroupChatinstatus-go-> remove loadFilters on new members of the group chat)\

at which point loadfilters can be removed (we still need to surface those filters to status-react until mailserver logic is ported as well, but we'd be half of the way)

adambabik commented 5 years ago

Closes #55