message cache allows index locking, rework messageHandler to use bulk fetching, sendMessage flow with no sleep; move some core getMessages/SendMessage handlers from FlwtchWorker to MainActivity #407

Merged
dan merged 4 commits from androMessage into trunk 2022-03-24 19:41:17 +00:00
Owner
  • message cache allows index locking
  • rework messageHandler to use bulk fetching, lock while fetching by index
  • sendMessage flow with no sleep
  • move some core getMessages/SendMessage handlers from FlwtchWorker to MainActivity
- message cache allows index locking - rework messageHandler to use bulk fetching, lock while fetching by index - sendMessage flow with no sleep - move some core getMessages/SendMessage handlers from FlwtchWorker to MainActivity
Member
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch-ui/674
sarah reviewed 2022-03-23 23:34:41 +00:00
@ -66,3 +66,1 @@
MessageInfo? lookup(MessageCache cache);
Future<dynamic> fetch(Cwtch cwtch, String profileOnion, int conversationIdentifier);
void add(MessageCache cache, MessageInfo messageInfo, String contenthash);
//Future<MessageInfo?> lookup(MessageCache cache);
Owner

delete?

delete?
dan marked this conversation as resolved
@ -67,2 +66,2 @@
Future<dynamic> fetch(Cwtch cwtch, String profileOnion, int conversationIdentifier);
void add(MessageCache cache, MessageInfo messageInfo, String contenthash);
//Future<MessageInfo?> lookup(MessageCache cache);
//Future<MessageInfo?> fetch(Cwtch cwtch, String profileOnion, int conversationIdentifier, MessageCache cache);
Owner

delete?

delete?
dan marked this conversation as resolved
@ -69,0 +67,4 @@
//Future<MessageInfo?> fetch(Cwtch cwtch, String profileOnion, int conversationIdentifier, MessageCache cache);
Future<MessageInfo?> get(Cwtch cwtch, String profileOnion, int conversationIdentifier, MessageCache cache);
//void add(MessageCache cache, MessageInfo messageInfo);
Owner

delete?

delete?
dan marked this conversation as resolved
@ -80,2 +83,2 @@
Future<dynamic> fetch(Cwtch cwtch, String profileOnion, int conversationIdentifier) {
return cwtch.GetMessage(profileOnion, conversationIdentifier, index);
Future<MessageInfo?> get( Cwtch cwtch, String profileOnion, int conversationIdentifier, MessageCache cache) async {
var chunk = 40;
Owner

explain

explain
dan marked this conversation as resolved
@ -81,1 +83,3 @@
return cwtch.GetMessage(profileOnion, conversationIdentifier, index);
Future<MessageInfo?> get( Cwtch cwtch, String profileOnion, int conversationIdentifier, MessageCache cache) async {
var chunk = 40;
if (chunk > cache.storageMessageCount - index) {
Owner

comment

comment
dan marked this conversation as resolved
@ -82,0 +91,4 @@
}
cache.lockIndexs(index, index+chunk);
var msgs = await cwtch.GetMessages(profileOnion, conversationIdentifier, index, chunk);
int i = 0; // declared here for use in finally to unlock
Owner

what

what
dan marked this conversation as resolved
@ -82,0 +103,4 @@
} catch (e, stacktrace) {
EnvironmentConfig.debugLog("Error: Getting indexed messages $index to ${index+chunk} failed parsing: " + e.toString() + " " + stacktrace.toString());
} finally {
// todo unlock remaining and mark malformed
Owner

seems important

seems important
dan marked this conversation as resolved
@ -41,3 +114,3 @@
}
void add(MessageInfo messageInfo, int index, String? contenthash) {
void lockIndexs(int start, int end) {
Owner

comment to explain purpose

comment to explain purpose
dan marked this conversation as resolved
@ -48,0 +132,4 @@
} else {
this.cacheByIndex.insert(index, LocalIndexMessage(messageInfo.metadata.messageID));
}
if (messageInfo.metadata.contenthash != "") {
Owner

can this ever happen?

can this ever happen?
Author
Owner

it used to be possible, i've been trying to catch all those cases. delete?

it used to be possible, i've been trying to catch all those cases. delete?
dan marked this conversation as resolved
@ -225,2 +225,4 @@
)))));
// TODO calculate newMark ID in CIS so we dont have to get here
var mark = Provider.of<ContactInfoState>(context).newMarker;
//var mi = await Provider.of<ContactInfoState>(context).messageCache.getByIndex(mark - 1);
Owner

remove or add mroe context

remove or add mroe context
Author
Owner

ah thanks, i forgot to unbreak this...

ah thanks, i forgot to unbreak this...
dan marked this conversation as resolved
@ -226,1 +226,4 @@
// TODO calculate newMark ID in CIS so we dont have to get here
var mark = Provider.of<ContactInfoState>(context).newMarker;
//var mi = await Provider.of<ContactInfoState>(context).messageCache.getByIndex(mark - 1);
var markMatch = false; //mi?.metadata.messageID == Provider.of<MessageMetadata>(context).messageID;
Owner

delete or add more context

delete or add more context
dan marked this conversation as resolved
dan force-pushed androMessage from 5443367bf4 to ecc9a3a48c 2022-03-24 01:06:15 +00:00 Compare
Member
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch-ui/676
sarah approved these changes 2022-03-24 18:20:13 +00:00
@ -82,0 +80,4 @@
// observationally flutter future builder seemed to be reaching for 20-40 message on pane load, so we start trying to load up to that many messages in one request
var chunk = 40;
// check that we aren't asking for messages beyond stored messages
if (chunk > cache.storageMessageCount - index) {
Owner

Feels weird that this code checks length for a static number, but references dynamic index on the right hand side.

feels like this should conceptually be:

	if (index + chunk > cache.storageMessageCount)  {
    	chunk = cache.storageMessageCount - index
    }
Feels weird that this code checks length for a static number, but references dynamic index on the right hand side. feels like this should conceptually be: if (index + chunk > cache.storageMessageCount) { chunk = cache.storageMessageCount - index }
dan marked this conversation as resolved
@ -82,0 +84,4 @@
chunk = cache.storageMessageCount - index;
}
if (index < cache.cacheByIndex.length) {
Owner

what is this check for?

what is this check for?
Author
Owner

if it's in cache, get it

otherwise fetch follow logic follows, prolly should move this to the top

if it's in cache, get it otherwise fetch follow logic follows, prolly should move this to the top
dan marked this conversation as resolved
@ -82,0 +87,4 @@
if (index < cache.cacheByIndex.length) {
return cache.getByIndex(index);
}
cache.lockIndexs(index, index+chunk);
Owner

lockIndexes

lockIndex*e*s
dan marked this conversation as resolved
dan added 1 commit 2022-03-24 19:04:14 +00:00
continuous-integration/drone/pr Build is passing Details
9812111041
comments, organizing logic
Member
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch-ui/677
dan merged commit b8c1c7682b into trunk 2022-03-24 19:41:17 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cwtch.im/cwtch-ui#407
No description provided.