openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.86k stars 3.58k forks source link

[persistence-addons] Implement QueryablePersistenceService where appropriate #12663

Open TBail opened 2 years ago

TBail commented 2 years ago

While working on some persistence rules i tried to use the API to save some data to a persistence service. Unfortunately this idi not work.

To find out what is wrong with my code i tried the same with the api explorer in OH3 latest nightly. I used the

PUT /persistence/items/{itemname}

with appropriate parameters, but unfortunaltely i got an error with both services installed. i used (influxdb and mapdb). The error is

{
  "error": {
    "message": "Persistence service not modifiable: mapdb",
    "http-code": 400
  }
}

To ensure the the parameters like service id and time etc are working in general i tried ti retrieve persistence data. This worked very well.

Based on this i assume that writing persistence data with thw rest api does not work.

cweitkamp commented 2 years ago

Thanks for reporting this missing feature. To store values in persistence via REST interface the services have to implement the ModifiablePersistenceService interface - which extends the QueryablePersistenceService interface. After a quick look into Add-Ons repository none of the available persistence services implements the mentioned feature.

I will try to have a look later into mapdb and jdbc how it can be archived.

spacemanspiff2007 commented 2 years ago

Thank you @cweitkamp for stepping in! That's really nice of you.

cweitkamp commented 2 years ago

Does it really make sense to implement this interface for MapDB as it only can store the last value? Please continue discussion in https://github.com/openhab/openhab-addons/pull/11923.

lujop commented 2 years ago

@cweitkamp It's possible to implement only the store part without removing one and throwing a not-supported exception?

InfuxDB 2.0 it's not very supportive in deletes, and implementing full FilterCriteria won't be possible and require some work. Because delete predicates are totally different than query predicates and the API doesn't support to construct them, and there are operators like inequality that are not supported (see https://docs.influxdata.com/influxdb/cloud/reference/syntax/delete-predicate) However the store part it's trivial and ready to submit a PR.

cweitkamp commented 2 years ago

Yes, I think so. Just throw an UnsupportedOperationException in the remove method. That's it.

lujop commented 2 years ago

I've added #12013 to track InfluxDB implementation

cweitkamp commented 2 years ago

Thanks. I have updated my list in openhab/openhab-addons#12663.

TBail commented 2 years ago

I have found out the the persistence service is taking the timestamp from the database system and not from the date/time the item is changed/updated. In my case i have differences between the timestamps sometimes of mor than half an hour.

Maybe it is an approach to set the timestamp in general from openhab and not from the database system.

openhab-bot commented 2 years ago

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/persistence-irritation/134307/7

J-N-K commented 2 years ago

@TBail Using different time sources sounds like a bad idea. However, it is up to the persistence service implementation to determine the correct way to store values. I had a look at JDBC and it would be easy to use the ZonedDateTime.now() as timestamp when no timestamp is given (instead of passing only the value to the DB server).