ESH Changes behavior of MAP Transform from return "" to return original value

Setup, Configuration and Use 1.x Compatibility
,
1

Continuing the discussion from [Fixed] Mios binding 1.8 not working in openHAB2:

I don’t believe the behavior of MAP needed to change (from returning an empty String, to returning the original/input String).

We should discuss why it did, and come to a consensus about whether to revert that part of the original commit, or leave it as-is in ESH (and document the change in behavior)

1 Like
2

@Kai, @guessed : I jump in the conversation, take a look at code and will keep you updated on this soon.

3

@glhopital,
Have you had an opportunity to go over the change or would it be better to file a Bug/Issue to track the discussion on the incompatibility that was introduced?

4

@guessed : yes for sure this is a change in the behaviour of the Map. Iirw, I did it provide a better function by default (e.g.when used in sitemaps). At this point I’m not really clear on what should be the reaction of a faultly transformation null or original value…

5

I can certainly see examples of wanting to change the behavior of existing transforms (REGEX confuses a lot of people, for example) but without a mechanism to provide options (for compatibility etc) I don’t think we should change the behavior.

In the case of MAP, it’s easy to see that either null/empty or original string are reasonable return options when there’s no match, but I’d always vote for compatibility in these cases… Esp when there’s no obvious error in the usage.

If we want different behavior, then I’d vote that we create MAP2 (etc) with that new behavior. This would avoid users seeing behavioral changes when they’re upgrading one release to the next.

I think the change should be rolled back.

1 Like
6

I think the change should be rolled back.

Ok for me. @glhopital what do you think?

7

Fine to me - so I’ll rollback this modification soon. PR created here

8

@guessed, @Kai : from what I see in the MapTransformationServiceTest here , the current behaviour of the Map service is as expected by the test designed by Kai (lines 72/76).
Do we all agree that I change that to return empty string when not found ?

9

Haha, you should look further back in the history and you will find that the tests were implemented by yourself :slight_smile:

10

Well done boss ! I’m catched, let me roll this back :slightly_smiling:

11

Thanks Gaël!

Here’s my simple OH 1.x MAP Test Rule, and it’s output under 1.8.1 showing the Empty string result:

rule "Test MAP behavior"
when
    Time cron "0/10 * * * * ?"
then
    var closed = transform("MAP", "en.map", "CLOSED")
    var other = transform("MAP", "en.map", "OTHER")
    logInfo("test-MAP", String::format("CLOSED='%1$s' OTHER='%2$s'", closed, other))
end

08:12:40.004 DEBUG o.o.m.r.i.e.ExecuteRuleJob[:53]- Executing scheduled rule 'Test MAP behavior'
08:12:40.008 WARN  o.o.c.t.i.s.MapTransformationService[:67]- Could not find a mapping for 'OTHER' in the file 'en.map'.
08:12:40.012 INFO  o.o.model.script.test-MAP[:53]- CLOSED='closed' OTHER=''
12

Should be merged soon !
In fact…it is merged now.

1 Like