Add support for persisting state with timestamp

[jlaur]

In core it is now possible to persist state with a given timestamp:

https://www.openhab.org/javadoc/latest/org/openhab/core/persistence/extensions/persistenceextensions#persist(org.openhab.core.items.Item,java.time.ZonedDateTime,org.openhab.core.types.State,java.lang.String)

I don't know if this is actually missing in openhab-js, or if it's only the documentation which is lacking (only .persist(serviceId) is mentioned): https://next.openhab.org/addons/automation/jsscripting/#itempersistence

Looking at https://openhab.github.io/openhab-js/items.ItemPersistence.html#persist it looks like it might be possible, so I did this attempt which was expected not to work because I didn't know how to create a State from scratch:

var spotPrices = items.Energi_Data_Service_Spot_Price.persistence.getAllStatesUntil(time.LocalDate.now().atStartOfDay().atZone(time.ZoneId.systemDefault()).plusDays(2));
for (const spotPrice of spotPrices) {
  var gridTariff = items.Energi_Data_Service_Grid_Tariff.persistence.persistedState(spotPrice.timestamp);
  var totalPrice = spotPrice.quantityState.add(gridTariff.quantityState);
  items.Energi_Data_Service_Total_Price.persistence.persist(spotPrice.timestamp, totalPrice);
}

However, it didn't fail like expected, but instead persisted the quantity value with current timestamp:

Because of openhab/openhab-core#3869 it would be really nice to be able to persist timestamped states.

Your Environment

• Version used: 4.2 snapshot 4112

[mherwege]

Did you try something like:

var spotPrices = items.Energi_Data_Service_Spot_Price.persistence.getAllStatesUntil(time.LocalDate.now().atStartOfDay().atZone(time.ZoneId.systemDefault()).plusDays(2));
for (const spotPrice of spotPrices) {
  var gridTariff = items.Energi_Data_Service_Grid_Tariff.persistence.persistedState(spotPrice.timestamp);
  var totalPrice = new PersistedState(spotPrice.quantityState.add(gridTariff.quantityState));
  items.Energi_Data_Service_Total_Price.persistence.persist(spotPrice.timestamp, totalPrice);
}

[florian-h05]

@mherwege Is right, this should work. I only have to find a way to document the overrides in the JSDoc and need to update the README

[jlaur]

@mherwege, @florian-h05 - thanks for the guidance. On first attempt I got "ReferenceError: "PersistedState" is not defined: ReferenceError: "PersistedState" is not defined", so will need to play with it some more. @florian-h05, do you want to keep this issue for reference when improving the documentation, or should I close it (at least when figuring out how to do this).

[florian-h05]

Sorry I didn’t read the code well enough, Marks code won‘t work.

@jlaur Your code didn’t fail like expected because you tried to persist a Quantity, which is casted to a QuantityType (which implements State) by GraalJS. However would it fail in most other cases because you don’t create a Java State inside JS - for better scripting support it would be great to have a persist method that accepts a stateAsString similar to https://www.openhab.org/javadoc/latest/org/openhab/core/automation/module/script/defaultscope/scriptbusevent#postUpdate(org.openhab.core.items.Item,java.lang.String).

However do I wonder why it persisted with wrong timestamp in your case.

[jlaur]

@florian-h05 - I tried a few things, but nothing so far worked. Inspired by what you wrote, same but more explicit:

var { QuantityType } = require("@runtime");
// (...)
var state = new QuantityType(totalPrice.toString());
items.Energi_Data_Service_Total_Price.persistence.persist(spotPrice.timestamp, state);

As expected, this also persists with "now" as timestamp. Now I'm wondering if the provided JS Joda ZonedDateTime will be automatically converted to Java ZDT here?

openhab-js/src/items/item-persistence.js

Lines 152 to 154 in a53216a

	persist (timestamp, state, serviceId) {
	PersistenceExtensions.persist(this.rawItem, ...arguments);
	}

[jlaur]

It seems to make no difference, same result with:

const ZonedDateTime = Java.type('java.time.ZonedDateTime');

var state = new QuantityType(totalPrice.toString());
var timestamp = ZonedDateTime.parse(spotPrice.timestamp.toString());
items.Energi_Data_Service_Total_Price.persistence.persist(timestamp, state);

[florian-h05]

@jlaur I would suspect that there is a bug in core or in the persistence service itself, the JS-Joda ZonedDateTime is properly converted to the Java equivalent.

I have verified this with this simple test (run at 12:18):

items.getItem('test_uom').persistence.persist(time.toZDT().plusMinutes(5), Quantity('1 m'))

Now attach a debugger to openHAB and put a breakpoint into PersistenceExtensions#persist(Item item, ZonedDateTime timestamp, State state, @Nullable String serviceId and you'll get the correct timestamp passed:

[mherwege]

What persistence service is being used for this?

[florian-h05]

I have just tried this on my dev instance without having an modifiable persistence service, if I am back home I can try with InfluxDB.

[mherwege]

@jlaur What persistence service do you use for this?

The actual storing is done in the persistence service, and the persistence extension passes it on the the persistence service.
I do see there is a bug in the persistence extension, but I am not sure it causes this issue.
In the persistence extensions, the following line has one argument too many: https://github.com/openhab/openhab-core/blob/5f1b708f27a07c9112bb8c23bbb8e7cb4370fcd6/bundles/org.openhab.core.persistence/src/main/java/org/openhab/core/persistence/extensions/PersistenceExtensions.java#L143
I will fix that part, but the only thing it should do is save it under another alias, not save them all at the same timestamp. That would be down to the individual persistence service. I would expect it to be stored in the persistence service under the name of the persistence service (the wrong alias), rather than under the item name.

[mherwege]

See openhab/openhab-core#4267

[jlaur]

@mherwege - I'm sorry if I'm causing unneeded confusion here, but now I'm even more confused. I'm using JDBC persistence with MySQL. I even just double-checked the code without finding any issue. Now going through the steps with debug logging enabled.

First, in the console I did:

openhab:update Energi_Data_Service_Total_Price 5

then back to the original code more or less:

var totalPrice = spotPrice.quantityState.add(gridTariff.quantityState);
console.log("Now trying to store total price " + totalPrice + " with timestamp " + spotPrice.timestamp);
items.Energi_Data_Service_Total_Price.persistence.persist(spotPrice.timestamp, totalPrice);

Result:

2024-06-05 12:58:43.219 [INFO ] [enhab.automation.script.file.test.js] - Now trying to store total price 0.4710812524999999999999999999999999200000 DKK/kWh with timestamp 2024-06-05T13:00+02:00[Europe/Copenhagen]
2024-06-05 12:58:43.221 [DEBUG] [persistence.jdbc.internal.JdbcMapper] - JDBC::storeItemValue: item=Energi_Data_Service_Total_Price (Type=NumberItem, State=5 DKK/kWh, Label=, Category=) state=5 DKK/kWh date=null
2024-06-05 12:58:43.221 [DEBUG] [persistence.jdbc.internal.dto.ItemVO] - JDBC:ItemVO tableName=Energi_Data_Service_Total_Price; newTableName=null; 
2024-06-05 12:58:43.221 [DEBUG] [istence.jdbc.internal.db.JdbcBaseDAO] - JDBC::storeItemValueProvider: item 'Energi_Data_Service_Total_Price' as Type 'NUMBERITEM' in 'Energi_Data_Service_Total_Price' with state '5 DKK/kWh'
2024-06-05 12:58:43.221 [DEBUG] [istence.jdbc.internal.db.JdbcBaseDAO] - JDBC::storeItemValueProvider: itemState: '5 DKK/kWh'
2024-06-05 12:58:43.221 [DEBUG] [persistence.jdbc.internal.dto.ItemVO] - JDBC:ItemVO setValueTypes dbType=DOUBLE; javaType=class java.lang.Double;
2024-06-05 12:58:43.221 [DEBUG] [istence.jdbc.internal.db.JdbcBaseDAO] - JDBC::storeItemValueProvider: newVal.doubleValue: '5.0'
2024-06-05 12:58:43.222 [DEBUG] [istence.jdbc.internal.db.JdbcBaseDAO] - JDBC::doStoreItemValue sql=INSERT INTO Energi_Data_Service_Total_Price (time, value) VALUES( NOW(3), ? ) ON DUPLICATE KEY UPDATE VALUE= ? value='5.0'

So I may already have made a mistake here:

items.Energi_Data_Service_Total_Price.persistence.persist(spotPrice.timestamp, totalPrice);

because it seems to actually persist the current value, not the provided one. In the logs "state=5 DKK/kWh date=null", so no new value and no timestamp.

I'll try to track when happens in core.

And now, when preparing one last thing for this comment, I might have found the issue to be caused by that bug you just fixed in core.

If this method:

https://github.com/openhab/openhab-addons/blob/43fa2c77680bf71dd554e1dcdbcae343851cd7c0/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java#L143-L147

Or this method:

https://github.com/openhab/openhab-addons/blob/43fa2c77680bf71dd554e1dcdbcae343851cd7c0/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java#L154-L158

is actually called (i.e. the alias overloads), this is very likely to be the reason, since it uses current state and null as timestamp. This is the method that is expected to be called:

https://github.com/openhab/openhab-addons/blob/43fa2c77680bf71dd554e1dcdbcae343851cd7c0/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java#L149-L152

[jlaur]

Thanks for the core fix, @mherwege. And sorry for creating the issue in this repository, @florian-h05, when actually it seems not related at all. I think only the documentation could use a small update adding the persist overload with timestamp. Feel free to close the issue.

[florian-h05]

I will update the docs and then close the issue 👍
Thanks for the issue anyway, reminded me that we need a persist method that accepts state as string, I have created a PR for that: openhab/openhab-core#4268.

[mherwege]

@jlaur So it looks like a combination of bugs in 2 places. I think you would want to pass on the date and state in this call: https://github.com/openhab/openhab-addons/blob/43fa2c77680bf71dd554e1dcdbcae343851cd7c0/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java#L157

To me it doesn't make much sense to store the current state at the current time when called with the wrong arguments. Or you store nothing and log a warning, or you ignore the alias (maybe logging a warning) and store the item at the set timestamp and date.

But if JDBC just would have ignored the alias, the bug would have gone unnoticed. So thank you for it.

[jlaur]

To me it doesn't make much sense to store the current state at the current time when called with the wrong arguments.

Agreed. This was a regression introduced in 4.1. I have provided a fix in openhab/openhab-addons#16845.

[jlaur]

@florian-h05 - oh, one last thing. 🙂 Do you have any current plans for this:

openhab-js/src/items/item-persistence.js

Line 168 in d81a61f

// TODO: Add persist for TimeSeries

?

For the use-case described in this issue, it actually would have been better to persist everything at once by preparing a time series.