Inspired by https://phab.comm.dev/D12133. Each action can result in some messages that are defined in specs.
Depends on D12775
Differential D12776
[lib] Generate P2P messages based on actions tomek on Jul 16 2024, 7:24 AM. Authored by Tags None Referenced Files
Subscribers
Details
Inspired by https://phab.comm.dev/D12133. Each action can result in some messages that are defined in specs. Depends on D12775 Tested in combination with the next diff. Checked if sending a DM operation results in generating P2P messages by adding some code calling a method from D12786.
Diff Detail
Event TimelineComment Actions Can you provide some more context about this? I don't quite understand why it makes sense to start from an action... my initial thinking was that business logic would construct a DMOperation directly, rather than constructing an action that gets converted into a DMOperation Comment Actions What do you mean by business logic? Is it the logic that lives in components, or in reducers, or maybe somewhere else? I can see at least three possible places where DMOperations can be generated
I don't have a strong opinion about which one is better for us. Centralizing this logic sounds beneficial - they can be defined in some kind of specs, but we can call the specs from many places. We also have some constraints in our solution that limit how we can handle the DMOperation
This means that without significant changes each OutboundP2PMessage needs to go through Redux. So if we decide to generate the DMOperations in components, we will need to introduce a new action that passes through Redux. This action will also need to modify the store and perform the same state modification that will happen on recipients. That's why starting with an action makes some sense - we already have the reducer logic that modifies the store based on it, and the only thing that is missing is creating the operation from it - similar to how we create DB operations. Comment Actions
I was thinking of the logic in components.
Hmm... to me, I think it would be much more simple to have the components generate a DMOp, and then to have a utility function that calls processDMOperation on it, and put the result in a Redux action. That way, we avoid having to support multiple different kinds of actions. What do you think? Comment Actions
Yeah, agree, it's simpler. It's similar to an updated solution I was thinking about. But there are some things we have to figure out:
Figured sharing these makes us sure we're on the same page. Comment Actions Would probably be a good idea to discuss this in my detail in our 1:1 tomorrow. One tradeoff neither of us has mentioned so far about using a single action for all business logic that generates DMOperations is that it might be debugging a little bit more painful.
Hmmm... but doesn't processDMOperation get called on the peer to process the DMOperation sent by somebody else? It seems like we should only call generateMessagesToPeers on the sender's side.
A little confused about this one. Can you explain when / where we need to know this "sending status"? Will the business logic in components need to call this function and then pass it to dispatchActionPromise?
I'm confused about this one too. What needs to be modified? What sort of IDs need to be stored? I thought this data was available in the MessageStore already, but I might be missing something. Comment Actions Yes, definitelly.
Yes, we should generate messages only on sender's side. I was considering introducing a flag or something, but ultimately created a different solution D12786 where I create a new method sendDMOperation in PeerToPeerProvider.
I'm not sure. E.g. in the input state container, we are checking the sending status of text messages. If our logic really needs that, we can call sendDMOperation and pass the result to dispatchActionPromise.
The missing data is the IDs of messages that are sent through tunnelbroker (OutboundP2PMessage). We're storing the messages in SQLite, but in order to retry them, we need their IDs. (there are some alternative approaches, e.g. sending messages to all the peers even if a message failed only for some - easier, but wasteful, and could be incorrect).
Comment Actions Thanks for all of the explanations above, those all make sense. @tomek and I discussed this in our 1:1 today and settled on keeping this approach, at least for now. |