Subtraction of dates does not return days-time-duration, but a duration in hours

[rob2universe]

Describe the bug
Subtraction of dates does not return days-time-duration (P5D), but a duration in hours (PT120H).

Documentation states :

First argument	Second argument	Result
date	date	days-time-duration

https://docs.camunda.io/docs/components/modeler/feel/language-guide/feel-temporal-expressions/#subtraction
but actual result is in hours.

To Reproduce

Run this process and check result:

<?xml version="1.0" encoding="UTF-8"?>
<bpmn:definitions xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:zeebe="http://camunda.org/schema/zeebe/1.0" xmlns:modeler="http://camunda.org/schema/modeler/1.0" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" xmlns:camunda="http://camunda.org/schema/1.0/bpmn" id="Definitions_1" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Web Modeler" exporterVersion="ac3bf19" modeler:executionPlatform="Camunda Cloud" modeler:executionPlatformVersion="8.0.0" camunda:diagramRelationId="514fd40d-5540-4a9d-9838-09aa23d6358f">
  <bpmn:process id="Process_f9a71f52-2581-421f-9c61-c7a9f6fcf61f" isExecutable="true">
    <bpmn:startEvent id="StartEvent_1">
      <bpmn:outgoing>Flow_1bu3582</bpmn:outgoing>
    </bpmn:startEvent>
    <bpmn:sequenceFlow id="Flow_1bu3582" sourceRef="StartEvent_1" targetRef="Activity_1thogsb"/>
    <bpmn:userTask id="Activity_1thogsb" name="Check Duration">
      <bpmn:extensionElements>
        <zeebe:ioMapping>
          <zeebe:input source="= date(&quot;2020-04-06&quot;) - date(&quot;2020-04-01&quot;)" target="result"/>
        </zeebe:ioMapping>
      </bpmn:extensionElements>
      <bpmn:incoming>Flow_1bu3582</bpmn:incoming>
      <bpmn:outgoing>Flow_0wchbw2</bpmn:outgoing>
    </bpmn:userTask>
    <bpmn:endEvent id="Event_16zrfda">
      <bpmn:incoming>Flow_0wchbw2</bpmn:incoming>
    </bpmn:endEvent>
    <bpmn:sequenceFlow id="Flow_0wchbw2" sourceRef="Activity_1thogsb" targetRef="Event_16zrfda"/>
  </bpmn:process>
  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_f9a71f52-2581-421f-9c61-c7a9f6fcf61f">
      <bpmndi:BPMNEdge id="Flow_1bu3582_di" bpmnElement="Flow_1bu3582">
        <di:waypoint x="186" y="118"/>
        <di:waypoint x="240" y="118"/>
      </bpmndi:BPMNEdge>
      <bpmndi:BPMNEdge id="Flow_0wchbw2_di" bpmnElement="Flow_0wchbw2">
        <di:waypoint x="340" y="118"/>
        <di:waypoint x="402" y="118"/>
      </bpmndi:BPMNEdge>
      <bpmndi:BPMNShape id="_BPMNShape_StartEvent_2" bpmnElement="StartEvent_1">
        <dc:Bounds x="150" y="100" width="36" height="36"/>
      </bpmndi:BPMNShape>
      <bpmndi:BPMNShape id="Activity_0zehzx6_di" bpmnElement="Activity_1thogsb">
        <dc:Bounds x="240" y="78" width="100" height="80"/>
        <bpmndi:BPMNLabel/>
      </bpmndi:BPMNShape>
      <bpmndi:BPMNShape id="Event_16zrfda_di" bpmnElement="Event_16zrfda">
        <dc:Bounds x="402" y="100" width="36" height="36"/>
      </bpmndi:BPMNShape>
    </bpmndi:BPMNPlane>
  </bpmndi:BPMNDiagram>
</bpmn:definitions>

Expected behavior
Return P5D instead of PT120H

Environment

• Camunda 8 Saas

[rob2universe]

Workaround could be:
= "P"+ string((date("2020-04-06") - date("2020-04-01")).days) + "D"

[saig0]

@rob2universe thank you for raising this up 👍

I see your point. Technically, both formats are the same days-time duration. The difference is that the current result is not normalized. In most cases that should be no issue because both durations represent the same value. However, it could make sense to normalize the value, similar to other places.

[kaaquist]

@saig0 Is this here up for grabs ? (hacktoberfest)

[saig0]

@kaaquist yes, go for it. 🚀

As a hint: I could be handy to place the normalization in the creation of the value.

[kaaquist]

Sorry, but I'm not to sure I get the idea here.
I created the following two tests and had hoped they would fail so I had some way of testing the code after the fix.

it should "subtract date from date" in {

    eval(""" date("2020-04-06") - date("2020-04-01") """) should be(
      ValDayTimeDuration("P5D"))
    eval(""" date("2020-04-06") - date("2020-04-01") """) should be(
      ValDayTimeDuration("PT120H"))
  }

Both pass.

Looking into the code I assume that this here is where the subtraction is done: https://github.com/camunda/feel-scala/blob/main/src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala#L758

So is that the place you would like the normalization to take place? I have tried to get the Duration to output days insted of hours. But no luck. But mybe you can point me to a place in the code where you have done similar normalization?
Thanks a million.

[saig0]

@kaaquist I see. It is a bit tricky because the duration gets normalized on the creation. You could observe the behavior when transforming the value into a string.

A failing test could look like the following:

it should "subtract date from date and normalize the result" in {
    eval(""" string(date("2020-04-06") - date("2020-04-01")) """) should be(
      ValString("P5D"))
  }

It seems the issue is more about the string conversion function itself.

Currently, the days-time-duration is not normalized, only the years-months-duration. There is no similar method for Java's Duration class.

[kaaquist]

A failing test could look like the following:

it should "subtract date from date and normalize the result" in {
eval(""" string(date("2020-04-06") - date("2020-04-01")) """) should be(
ValString("P5D"))
}

It seems the issue is more about the string conversion function itself.
Currently, the days-time-duration is not normalized, only the years-months-duration. There is no similar method for Java's Duration class.

But I would expect that test to fail because of the type, ValString, when I know the subtract of dates return ValDayTimeDuration. But I can try to create an override of the toString and then pass a normalized DayTimeDuration string.

[saig0]

@kaaquist please have a look at the string() function. Currently, it calls the .toString() method on the Duration value.

[nicpuppa]

Hey @saig0, we have a customer facing this problem (support). I tried in the playground and it's happening in the latest FEEL version. Should I reopen this issue ?