opensmarthouse / opensmarthouse-core

Eclipse Public License 2.0
7 stars 0 forks source link

Fine grained persistence configuration shortages #32

Open splatch opened 3 years ago

splatch commented 3 years ago

Persistence service configuration within openHAB is provided through "persist" files. These files are bound to persistence service identifiers based on their names. So influxdb.persist gets paired with InfluxDBPersistenceService and so forth (which declares its id to be influxdb). Since we provide a way to run system without EMF/ecore/xtext stuff we effectively have lost above way of configuring database services. Now, each of our assemblies, will rely on defaults inherited from persistence service. This is a change introduced within OH 3 which we acquired while doing initial clean ups.

I started to get this puzzle sorted cause I need to disable persistence on some items so their state is not propagated to database on every update. This turned to be a quite difficult task, because persistence configuration does not explicitly support such case. Obviously it is possible to set defaults to none and configure items selectively, yet this could be burdensome. All examples from documentation focus on how to set persistence policy but not how to unset a policy inherited ie. from a group. I am not sure why, but PersistenceItemConfiguration which allows to group several strategies at once does have a support for PersistenceFilter, yet it is not implemented. Both EMF and PersistenceManager do not make any use of it. The PersistenceFilter is probably a good candidate to terminate or use in some cases (ie. to use categories, tags at item level to disable/enable certain config).

One observation I have right now is that persistence configuration is very, very closely related with the way how it was written in xtext. It is almost impossible to understand configuration structure without looking at the sample persist files. Naming of some elements is also hard to follow. For example: PersistenceItemConfiguration wrapped by PersistenceServiceConfiguration has a field items of type List<PersistenceConfig>. One of PersistenceConfig implementations is actually aPersistenceItemConfig and is intended to control storage for an item based on its name. This means that a single PersistenceItemConfiguration wraps configuration of multiple configurations which can utilize a PersistenceItemConfig to setup individual items. I believe that PersistenceItemConfiguration could become a PersistenceItem*s*Configuration to clearly distinguish its role. A side note, all above types miss equals and hashCode making it quite difficult to test them. Also some of toString operations will fail if a nullable field is set to null.

Another point on PersistenceItemConfiguration is following. This type is supposed to hold strategies and let various combinations of PersistenceConfigs refer them. However, we must note that internal structures there do not rely on any direct object references. In fact the only strategy which has a settable name by user is a "cron". Strategies coming from PersistenceStrategy.Globals are self contained and do not have any configuration options. Going forward fact PersistenceItemConfiguration.strategies field could be turned into List<PersistenceCronStrategy> with no harm, as it is the only strategy which is accepted and used in this area by PersistenceManager. All it causes is a periodic write to persistence service if assigned strategy name matches. Strategies currently are compared by their names making it pretty hard to provide other strategies beyond ones defined by system itself. One additional note on design. I believe that PersistenceConfig could till some degree become an Predicate<Item> on its own. We have so far 3 predicates there 1) PersistenceAllConfig: *: item -> true 2) PersistenceItemConfig: name: item -> name.equals(item.getName()) 3) PersistenceGroupConfig: group: item -> group.equals(item.getName()) && item instanceof GroupItem && lookup(group).getAllMembers().contains(item)

Having all above notes I believe I finally understand how the config works, despite of lack of full clearance why it was made in such a way. I don't know yet how this could or should be fixed.