[mqtt.generic] fix state description provider

[J-N-K]

Fixes #8845

Signed-off-by: Jan N. Klug [email protected]

[cpmeister]

I'm not sure about this, from the issue conversation it looks like the core is checking if the new state description is equal to the original description using .equals. IMO it shouldn't be an error for a binding to coincidentally return a description that happens to be similar to the original one. If the intent of the check is to make sure that the binding isn't returning the original description then perhaps the equality check in the core should be changed to a reference comparison between the original and returned descriptions.

[jochen314]

Even the documentation says:

If the given channel will not be managed by the provider null should be returned. You never must return the 
original state description in such case.

But the implemenation will always return null if the cannel is not managed by the provider.
This is not a problem of the provider, but of the logging in the core.

[J-N-K]

@openhab/core-maintainers Can you comment here? It's true that the original state description is returned but the channel is managed by the provider, to the restriction is not fulfilled here.

[cweitkamp]

I agree on that this implementation of the DynamicStateDeaceiptionProvider is a valid use case. And the comparison in OHC does not considered such a situation. Point is that we have to make sure in some way that no original state description will be returned by any binding if the channel is not manged by the provider because if it does it will accidently overwrite state description of other bindings.

the equality check in the core should be changed to a reference comparison between the original and returned descriptions

This is probably not possible because most of the bindings extend the BaseDynamicStateDescriptionProvider which always builds a new state description instance to provide transportation features for dynamic state options and patterns.

[cpmeister]

the equality check in the core should be changed to a reference comparison between the original and returned descriptions

This is probably not possible because most of the bindings extend the BaseDynamicStateDescriptionProvider which always builds a new state description instance to provide transportation features for dynamic state options and patterns.

Why would them always building a new dynamic state cause problems, that is exactly what you want them to do, no?
The code change in the core should be simple:

            StateDescription stateDescription = provider.getStateDescription(channel, originalStateDescription, locale);
            if (stateDescription != null) {
                if (stateDescription == originalStateDescription) {
                    logger.error(
                            "Dynamic state description matches original state description. DynamicStateDescriptionProvider implementations must never return the original state description. {} has to be fixed.",
                            provider.getClass());
                } else {
                    return stateDescription;
                }
            }

This would guarantee that the dynamic providers are not returning the originalStateDescription that is passed in as a parameter. Any returned non-null description after the reference comparison would be guaranteed to have been intentionally supplied by the provider.

[cweitkamp]

Yes, I was talking nonsense yesterday. I will do some test later this day.

[cweitkamp]

I submitted openhab/openhab-core#1817 to fix #8845. This PR will not be needed anymore.