From 5cf72f988414ffc29faa5bfa0e94d40abb60c063 Mon Sep 17 00:00:00 2001 From: Szymon Bogusz Date: Wed, 22 Jan 2025 12:24:36 +0100 Subject: [PATCH 1/6] 31+ days in windowLength are not lost, small fe fix --- .../editors/expression/Duration/DurationEditor.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/designer/client/src/components/graph/node-modal/editors/expression/Duration/DurationEditor.tsx b/designer/client/src/components/graph/node-modal/editors/expression/Duration/DurationEditor.tsx index 40c4b755c1d..7ad91b4f794 100644 --- a/designer/client/src/components/graph/node-modal/editors/expression/Duration/DurationEditor.tsx +++ b/designer/client/src/components/graph/node-modal/editors/expression/Duration/DurationEditor.tsx @@ -9,6 +9,7 @@ import { isEmpty } from "lodash"; import { ExtendedEditor } from "../Editor"; export type Duration = { + months: number; days: number; hours: number; minutes: number; @@ -29,6 +30,7 @@ type Props = { const SPEL_DURATION_SWITCHABLE_TO_REGEX = /^T\(java\.time\.Duration\)\.parse\('(-)?P([0-9]{1,}D)?(T((-)?[0-9]{1,}H)?((-)?[0-9]{1,}M)?((-)?[0-9]{1,}S)?)?'\)$/; const NONE_DURATION = { + months: () => null, days: () => null, hours: () => null, minutes: () => null, @@ -63,7 +65,8 @@ export const DurationEditor: ExtendedEditor = (props: Props) => { const duration = decodeExecResult == null || typeof decodeExecResult !== "string" ? NONE_DURATION : moment.duration(decodeExecResult); return { - days: duration.days(), + months: 0, + days: duration.days() + duration.months() * 31, hours: duration.hours(), minutes: duration.minutes(), seconds: duration.seconds(), From 540c8cf49bd9b01cba5d68b7729d74772354b2a8 Mon Sep 17 00:00:00 2001 From: Szymon Bogusz Date: Fri, 24 Jan 2025 15:04:01 +0100 Subject: [PATCH 2/6] Added default values to time window components, so there will be less red alerts Also changed default in session window for `endSessionCondition` from true to false --- .../transformer/aggregate/sampleTransformers.scala | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/engine/flink/components/base-unbounded/src/main/scala/pl/touk/nussknacker/engine/flink/util/transformer/aggregate/sampleTransformers.scala b/engine/flink/components/base-unbounded/src/main/scala/pl/touk/nussknacker/engine/flink/util/transformer/aggregate/sampleTransformers.scala index d38ce11e8a8..aad6264abc7 100644 --- a/engine/flink/components/base-unbounded/src/main/scala/pl/touk/nussknacker/engine/flink/util/transformer/aggregate/sampleTransformers.scala +++ b/engine/flink/components/base-unbounded/src/main/scala/pl/touk/nussknacker/engine/flink/util/transformer/aggregate/sampleTransformers.scala @@ -54,7 +54,7 @@ object sampleTransformers { defaultMode = DualEditorMode.SIMPLE ) aggregator: Aggregator, @ParamName("aggregateBy") aggregateBy: LazyParameter[AnyRef], - @ParamName("windowLength") length: java.time.Duration, + @ParamName("windowLength") @DefaultValue("T(java.time.Duration).parse('PT1H')") length: java.time.Duration, @ParamName("emitWhenEventLeft") @DefaultValue("false") emitWhenEventLeft: Boolean, @OutputVariableName variableName: String )(implicit nodeId: NodeId): ContextTransformation = { @@ -112,7 +112,7 @@ object sampleTransformers { defaultMode = DualEditorMode.SIMPLE ) aggregator: Aggregator, @ParamName("aggregateBy") aggregateBy: LazyParameter[AnyRef], - @ParamName("windowLength") length: java.time.Duration, + @ParamName("windowLength") @DefaultValue("T(java.time.Duration).parse('PT1H')") length: java.time.Duration, @ParamName("emitWhen") trigger: TumblingWindowTrigger, @OutputVariableName variableName: String )(implicit nodeId: NodeId): ContextTransformation = { @@ -174,8 +174,10 @@ object sampleTransformers { defaultMode = DualEditorMode.SIMPLE ) aggregator: Aggregator, @ParamName("aggregateBy") aggregateBy: LazyParameter[AnyRef], - @ParamName("endSessionCondition") endSessionCondition: LazyParameter[java.lang.Boolean], - @ParamName("sessionTimeout") sessionTimeout: java.time.Duration, + @ParamName("endSessionCondition") @DefaultValue("false") endSessionCondition: LazyParameter[java.lang.Boolean], + @ParamName("sessionTimeout") @DefaultValue( + "T(java.time.Duration).parse('PT1H')" + ) sessionTimeout: java.time.Duration, @ParamName("emitWhen") trigger: SessionWindowTrigger, @OutputVariableName variableName: String )(implicit nodeId: NodeId): ContextTransformation = { From a02789db4827d3d87945a95874d28da8b6e566d7 Mon Sep 17 00:00:00 2001 From: Szymon Bogusz Date: Tue, 28 Jan 2025 15:48:23 +0100 Subject: [PATCH 3/6] Changelog update --- docs/Changelog.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/Changelog.md b/docs/Changelog.md index dfe6d400410..b3f50974193 100644 --- a/docs/Changelog.md +++ b/docs/Changelog.md @@ -69,6 +69,11 @@ cannot overlap, if they do, an exception is thrown. * [#7504](https://github.com/TouK/nussknacker/pull/7504) Return scenario validation error when an incompatible change was introduced in a fragment or component parameter definition. * [#7468](https://github.com/TouK/nussknacker/pull/7468) Configurable namespace separator (was fixed to `_`), added namespace tag to Lite engine metrics and fixed namespacing of Kafka consumer groups. +* [#7508](https://github.com/TouK/nussknacker/pull/7508) Fixes for window components: + * Can now pass more than 31 days as `windowLength` and it won't be reduced to remainder of 31 + * Introduced some default values: + * For all - default `windowLength` is 1 hour + * For `aggregate-session` - default `endSessionCondition` is now false ## 1.18 From 6ed5d0483ec12d62b3fb6199b5910cbbaed9ac3e Mon Sep 17 00:00:00 2001 From: Szymon Bogusz Date: Wed, 29 Jan 2025 14:19:34 +0100 Subject: [PATCH 4/6] Use of duration.asDays instead of passing months --- .../editors/expression/Duration/DurationEditor.tsx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/designer/client/src/components/graph/node-modal/editors/expression/Duration/DurationEditor.tsx b/designer/client/src/components/graph/node-modal/editors/expression/Duration/DurationEditor.tsx index 7ad91b4f794..26360913622 100644 --- a/designer/client/src/components/graph/node-modal/editors/expression/Duration/DurationEditor.tsx +++ b/designer/client/src/components/graph/node-modal/editors/expression/Duration/DurationEditor.tsx @@ -9,7 +9,6 @@ import { isEmpty } from "lodash"; import { ExtendedEditor } from "../Editor"; export type Duration = { - months: number; days: number; hours: number; minutes: number; @@ -30,8 +29,7 @@ type Props = { const SPEL_DURATION_SWITCHABLE_TO_REGEX = /^T\(java\.time\.Duration\)\.parse\('(-)?P([0-9]{1,}D)?(T((-)?[0-9]{1,}H)?((-)?[0-9]{1,}M)?((-)?[0-9]{1,}S)?)?'\)$/; const NONE_DURATION = { - months: () => null, - days: () => null, + asDays: () => null, hours: () => null, minutes: () => null, seconds: () => null, @@ -65,8 +63,7 @@ export const DurationEditor: ExtendedEditor = (props: Props) => { const duration = decodeExecResult == null || typeof decodeExecResult !== "string" ? NONE_DURATION : moment.duration(decodeExecResult); return { - months: 0, - days: duration.days() + duration.months() * 31, + days: duration.asDays() < 1 ? null : duration.asDays().toFixed(), hours: duration.hours(), minutes: duration.minutes(), seconds: duration.seconds(), From 1c49332b03683871d644427d77b313b66a17cb8b Mon Sep 17 00:00:00 2001 From: Szymon Bogusz Date: Thu, 30 Jan 2025 10:39:25 +0100 Subject: [PATCH 5/6] Some fixes and added test --- .../expression/Duration/DurationEditor.tsx | 19 +++++++------ .../test/Editors/DurationEditor-test.tsx | 27 +++++++++++++++++-- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/designer/client/src/components/graph/node-modal/editors/expression/Duration/DurationEditor.tsx b/designer/client/src/components/graph/node-modal/editors/expression/Duration/DurationEditor.tsx index 26360913622..5bb019391bb 100644 --- a/designer/client/src/components/graph/node-modal/editors/expression/Duration/DurationEditor.tsx +++ b/designer/client/src/components/graph/node-modal/editors/expression/Duration/DurationEditor.tsx @@ -60,14 +60,7 @@ export const DurationEditor: ExtendedEditor = (props: Props) => { (expression: string): Duration => { const decodeExecResult = durationFormatter.decode(expression); - const duration = - decodeExecResult == null || typeof decodeExecResult !== "string" ? NONE_DURATION : moment.duration(decodeExecResult); - return { - days: duration.asDays() < 1 ? null : duration.asDays().toFixed(), - hours: duration.hours(), - minutes: duration.minutes(), - seconds: duration.seconds(), - }; + return duration(decodeExecResult); }, [durationFormatter], ); @@ -97,3 +90,13 @@ DurationEditor.notSwitchableToHint = () => "editors.duration.noSwitchableToHint", "Expression must match pattern T(java.time.Duration).parse('P(n)DT(n)H(n)M') to switch to basic mode", ); + +export function duration(decodeExecResult) { + const duration = decodeExecResult == null || typeof decodeExecResult !== "string" ? NONE_DURATION : moment.duration(decodeExecResult); + return { + days: Math.floor(duration.asDays()), + hours: duration.hours(), + minutes: duration.minutes(), + seconds: duration.seconds(), + }; +} diff --git a/designer/client/test/Editors/DurationEditor-test.tsx b/designer/client/test/Editors/DurationEditor-test.tsx index ab6910237b7..6bfeed86538 100644 --- a/designer/client/test/Editors/DurationEditor-test.tsx +++ b/designer/client/test/Editors/DurationEditor-test.tsx @@ -2,11 +2,12 @@ import * as React from "react"; import { render, screen } from "@testing-library/react"; import { DualEditorMode, EditorType } from "../../src/components/graph/node-modal/editors/expression/Editor"; -import { DurationEditor } from "../../src/components/graph/node-modal/editors/expression/Duration/DurationEditor"; +import { DurationEditor, duration } from "../../src/components/graph/node-modal/editors/expression/Duration/DurationEditor"; import { TimeRange } from "../../src/components/graph/node-modal/editors/expression/Duration/TimeRangeComponent"; import { mockFormatter, mockFieldErrors, mockValueChange } from "./helpers"; import { NuThemeProvider } from "../../src/containers/theme/nuThemeProvider"; -import { nodeInputWithError } from "../../src/components/graph/node-modal/NodeDetailsContent/NodeTableStyled"; +import { FormatterType, typeFormatters } from "../../src/components/graph/node-modal/editors/expression/Formatter"; +import type { Duration } from "../../src/components/graph/node-modal/editors/expression/Duration/DurationEditor"; describe(DurationEditor.name, () => { it("should display validation error when the field is required", () => { @@ -32,3 +33,25 @@ describe(DurationEditor.name, () => { expect(screen.getByText("validation error")).toBeInTheDocument(); }); }); + +describe(`${duration.name} function`, () => { + it("should parse duration without loosing days if more than 31", () => { + const emptyDuration: Duration = { days: 0, hours: 0, minutes: 0, seconds: 0 }; + const formatter = typeFormatters[FormatterType.Duration]; + const oneMinute = "PT1M"; + const oneHour = "PT1H"; + const oneDay = "P1D"; + const fortyDays = "P40D"; + const mix = "P1DT1H1M"; + + const times = [oneMinute, oneHour, oneDay, fortyDays, mix]; + const results = [ + { ...emptyDuration, minutes: 1 }, + { ...emptyDuration, hours: 1 }, + { ...emptyDuration, days: 1 }, + { ...emptyDuration, days: 40 }, + { days: 1, hours: 1, minutes: 1, seconds: 0 }, + ]; + expect(times.map(formatter.decode).map(duration)).toStrictEqual(results); + }); +}); From e9a221db2e415421e142ac316a4a56f5e166b654 Mon Sep 17 00:00:00 2001 From: Szymon Bogusz Date: Thu, 30 Jan 2025 11:53:17 +0100 Subject: [PATCH 6/6] Added 2 corner cases to test --- designer/client/test/Editors/DurationEditor-test.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/designer/client/test/Editors/DurationEditor-test.tsx b/designer/client/test/Editors/DurationEditor-test.tsx index 6bfeed86538..089b2d070de 100644 --- a/designer/client/test/Editors/DurationEditor-test.tsx +++ b/designer/client/test/Editors/DurationEditor-test.tsx @@ -40,14 +40,18 @@ describe(`${duration.name} function`, () => { const formatter = typeFormatters[FormatterType.Duration]; const oneMinute = "PT1M"; const oneHour = "PT1H"; + const almostOneDay = "PT23H"; + const oneDayOneHour = "PT25H"; const oneDay = "P1D"; const fortyDays = "P40D"; const mix = "P1DT1H1M"; - const times = [oneMinute, oneHour, oneDay, fortyDays, mix]; + const times = [oneMinute, oneHour, almostOneDay, oneDayOneHour, oneDay, fortyDays, mix]; const results = [ { ...emptyDuration, minutes: 1 }, { ...emptyDuration, hours: 1 }, + { ...emptyDuration, hours: 23 }, + { ...emptyDuration, days: 1, hours: 1 }, { ...emptyDuration, days: 1 }, { ...emptyDuration, days: 40 }, { days: 1, hours: 1, minutes: 1, seconds: 0 },