XDS sender should persist last successful request date

[mozzy11]

No description provided.

[ibacher]

Couple of comments here:

• If we're going to create a whole table for this, it should be associated to the specific task, so that we can use this same structure for multiple things. However, I think this should be backed by a GP or something similar.
• OpenMRS best practice is for tables to always use the module id of the module defining them, so xds_sender_request_date rather than request_date. This way, the module will not interfere with other modules (that follow this rule) and also, it makes it clearer what the table is for.

[mozzy11]

i ddint want to inntially use a table.
Thought about just storing the timeStamp in a text file some where, though it also came with just a few other considerations

About usig a GP , do you mean a GP for allowing allowing these objects to be persisted ??

like

if (GP.xds_persist_request_timestamp){
// persist the request date 
}

About associating this to the specific task
I also though about Associating this to some kind of Object but i failed to get a resonable one

What Task exactly did you mean ??

[ibacher]

The PullNotificationsTask instance associated with the run. The idea is to have a pattern we can use for multiple similar tasks (for future cases where we may not be just exchanging labs, but possibly other data from the HIE).

In terms of GP, basically what we're storing is a single datetime, which can be represented as a string, so why not just store that string as, e.g., xds.sender.<PullNotificationsTaskUuid>.lastSuccessfulRun? It's still functionally backed by the DB, but it doesn't require creating the table, etc.

[mozzy11]

Thats true , i though about doing that at some point , and thought i would misusing the GP.
But let me just simply use a GP

[mozzy11]

so when you say xds.sender.<PullNotificationsTaskUuid>.lastSuccessfulRun,
do you mean creating a new GP on every succesful run ??
I though i only need to use a single GP xds.sender.PullNotificationTask.lastSuccessfulRun and just keep updating the same

[ibacher]

That's fine too, but, no I was thinking of the UUID for the specific task (each scheduled task is stored in the database as an object with a UUID, since each task instance can have a separate configuration even if they use the same code).

[ibacher]

A few things here:

• The 5-days ago calculation is very arbitrary. Why that value? The process runs, by default, once an hour, so 5 days ago is likely to actually be requesting more data than the "give me 100 results" version.
• It's substantially simpler (and more correct) to do this with the java.time classes:

String lastRequestDateTime = /* get last date-time timestamp */

if (lastRequestDateTime == null || lastRequestDateTime.isEmpty()) {
   lastRequestDateTime = DateTimeFormatter.ISO_INSTANT.format(Instant.now().minus(2, ChronoUnit.HOURS));
}

Then on the storage side, we can just persist the timestamp, e.g.,

if (success) {
   saveLastRun(DateTimeFormatter.ISO_INSTANT.format(Instant.now()))
}

[ibacher]

A technically better solution, though, would be not to use any guessed period and just send a single, final request with the current 100 results, and then start using the since from there.

[ibacher]

Always use primitives unless you need null;

Suggested change

	Boolean success = true;
	boolean success = true;

[ibacher]

This should actually be a minor version bump, i.e., 2.5.0-SNAPSHOT.