From 062fdaffb7702e7e1e6f65a0ebf46c4679534fe3 Mon Sep 17 00:00:00 2001 From: Dawid Poliszak Date: Wed, 6 Nov 2024 12:41:45 +0100 Subject: [PATCH 1/5] update changelog frontend changes (#7120) --- docs/Changelog.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/Changelog.md b/docs/Changelog.md index bb5ab7799da..310e13fe918 100644 --- a/docs/Changelog.md +++ b/docs/Changelog.md @@ -80,6 +80,12 @@ * isBigDecimal/toBigDecimal/toBigDecimalOrNull * isList/toList/toListOrNull * isMap/toMap/toMapOrNull - the list of key-value pairs or unknown map can be converted to a map. +* [#7106](https://github.com/TouK/nussknacker/pull/7106) Fix an issue where pressing the β€œEsc” key did not remove focus from input fields in dialogs, which prevented the dialog window from closing +* [#7002](https://github.com/TouK/nussknacker/pull/7002) Resolve an issue with union nodes output expression when nodes were copied and pasted +* [#6994](https://github.com/TouK/nussknacker/pull/6994) Fix styling issues for form checkboxes in Firefox +* [#6721](https://github.com/TouK/nussknacker/pull/6721) Provide a popover to display additional information about count +* [#7099](https://github.com/TouK/nussknacker/pull/7099) Provide an option to embedded video to the markdown +* [#7102](https://github.com/TouK/nussknacker/pull/7102) Introduce a new UI to defining aggregations within nodes ## 1.17 From 1f1b395756dbbc508e4fa69c3337d8f2296628a6 Mon Sep 17 00:00:00 2001 From: Arek Burdach Date: Wed, 6 Nov 2024 22:39:32 +0100 Subject: [PATCH 2/5] [NU-1848] Work around for broken compatibility with Flink < 1.19: environment variable allowing to disable Flink TypeInfos (#7128) * [NU-1848] Work around for broken compatibility with Flink < 1.19: environment variable allowing to disable Flink TypeInfos * Ability to disable type info registration programmatically --- .../FlinkTypeInfoRegistrar.scala | 38 ++++++++++++++++--- .../TypeInformationDetection.scala | 2 +- .../FlinkTypeInfoRegistrarTest.scala | 2 +- .../process/ExecutionConfigPreparer.scala | 2 +- 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/engine/flink/components-api/src/main/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/FlinkTypeInfoRegistrar.scala b/engine/flink/components-api/src/main/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/FlinkTypeInfoRegistrar.scala index 2a9408688a0..a7b024d47df 100644 --- a/engine/flink/components-api/src/main/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/FlinkTypeInfoRegistrar.scala +++ b/engine/flink/components-api/src/main/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/FlinkTypeInfoRegistrar.scala @@ -6,29 +6,55 @@ import org.apache.flink.api.java.typeutils.TypeExtractor import java.lang.reflect.Type import java.time.{LocalDate, LocalDateTime, LocalTime} import java.util +import java.util.concurrent.atomic.AtomicBoolean +// This class contains registers TypeInfoFactory for commonly used classes in Nussknacker. +// It is a singleton as Flink's only contains a global registry for such purpose object FlinkTypeInfoRegistrar { - private case class RegistrationEntry[T, K <: TypeInfoFactory[T]](klass: Class[T], factoryClass: Class[K]) + private val typeInfoRegistrationEnabled = new AtomicBoolean(true) - private val typesToRegister = List( + private val DisableFlinkTypeInfoRegistrationEnvVarName = "NU_DISABLE_FLINK_TYPE_INFO_REGISTRATION" + + private case class RegistrationEntry[T](klass: Class[T], factoryClass: Class[_ <: TypeInfoFactory[T]]) + + private val typeInfoToRegister = List( RegistrationEntry(classOf[LocalDate], classOf[LocalDateTypeInfoFactory]), RegistrationEntry(classOf[LocalTime], classOf[LocalTimeTypeInfoFactory]), RegistrationEntry(classOf[LocalDateTime], classOf[LocalDateTimeTypeInfoFactory]), ) - def ensureBaseTypesAreRegistered(): Unit = - typesToRegister.foreach { base => - register(base) + def ensureTypeInfosAreRegistered(): Unit = { + // TypeInfo registration is available in Flink >= 1.19. For backward compatibility purpose we allow + // to disable this by either environment variable or programmatically + if (typeInfoRegistrationEnabled.get() && !typeInfoRegistrationDisabledByEnvVariable) { + typeInfoToRegister.foreach { entry => + register(entry) + } } + } + + private def typeInfoRegistrationDisabledByEnvVariable = { + Option(System.getenv(DisableFlinkTypeInfoRegistrationEnvVarName)).exists(_.toBoolean) + } - private def register(entry: RegistrationEntry[_, _ <: TypeInfoFactory[_]]): Unit = { + private def register(entry: RegistrationEntry[_]): Unit = { val opt = Option(TypeExtractor.getTypeInfoFactory(entry.klass)) if (opt.isEmpty) { TypeExtractor.registerFactory(entry.klass, entry.factoryClass) } } + // These methods are mainly for purpose of tests in nussknacker-flink-compatibility project + // It should be used in caution as it changes the global state + def enableFlinkTypeInfoRegistration(): Unit = { + typeInfoRegistrationEnabled.set(true) + } + + def disableFlinkTypeInfoRegistration(): Unit = { + typeInfoRegistrationEnabled.set(false) + } + class LocalDateTypeInfoFactory extends TypeInfoFactory[LocalDate] { override def createTypeInfo( diff --git a/engine/flink/components-api/src/main/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/TypeInformationDetection.scala b/engine/flink/components-api/src/main/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/TypeInformationDetection.scala index 35a14eab36e..50ad362f158 100644 --- a/engine/flink/components-api/src/main/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/TypeInformationDetection.scala +++ b/engine/flink/components-api/src/main/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/TypeInformationDetection.scala @@ -52,7 +52,7 @@ object TypeInformationDetection { // We use SPI to provide implementation of TypeInformationDetection because we don't want to make // implementation classes available in flink-components-api module. val instance: TypeInformationDetection = { - FlinkTypeInfoRegistrar.ensureBaseTypesAreRegistered() + FlinkTypeInfoRegistrar.ensureTypeInfosAreRegistered() val classloader = Thread.currentThread().getContextClassLoader ServiceLoader diff --git a/engine/flink/components-api/src/test/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/FlinkTypeInfoRegistrarTest.scala b/engine/flink/components-api/src/test/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/FlinkTypeInfoRegistrarTest.scala index 71bc968124a..6514ba0c21d 100644 --- a/engine/flink/components-api/src/test/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/FlinkTypeInfoRegistrarTest.scala +++ b/engine/flink/components-api/src/test/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/FlinkTypeInfoRegistrarTest.scala @@ -42,7 +42,7 @@ class FlinkTypeInfoRegistrarTest extends AnyFunSuite with Matchers { } test("Looking for TypeInformation for a NU types with registrar should return a specific TypeInformation") { - FlinkTypeInfoRegistrar.ensureBaseTypesAreRegistered() + FlinkTypeInfoRegistrar.ensureTypeInfosAreRegistered() nuTypesMapping.foreach { case (klass, expected) => val typeInfo = TypeInformation.of(klass) diff --git a/engine/flink/executor/src/main/scala/pl/touk/nussknacker/engine/process/ExecutionConfigPreparer.scala b/engine/flink/executor/src/main/scala/pl/touk/nussknacker/engine/process/ExecutionConfigPreparer.scala index e605dfc5a4d..d614af52f67 100644 --- a/engine/flink/executor/src/main/scala/pl/touk/nussknacker/engine/process/ExecutionConfigPreparer.scala +++ b/engine/flink/executor/src/main/scala/pl/touk/nussknacker/engine/process/ExecutionConfigPreparer.scala @@ -101,7 +101,7 @@ object ExecutionConfigPreparer extends LazyLogging { override def prepareExecutionConfig( config: ExecutionConfig )(jobData: JobData, deploymentData: DeploymentData): Unit = { - FlinkTypeInfoRegistrar.ensureBaseTypesAreRegistered() + FlinkTypeInfoRegistrar.ensureTypeInfosAreRegistered() Serializers.registerSerializers(modelData, config) if (enableObjectReuse) { config.enableObjectReuse() From 332bf1aca5725b1a8fdce7f6c26df1c7096961f3 Mon Sep 17 00:00:00 2001 From: Arek Burdach Date: Fri, 8 Nov 2024 12:06:40 +0100 Subject: [PATCH 3/5] 1.18 release highlights (#7127) * [NU-1846] Release 1.18 highlights * review fixes --- docs/Changelog.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/Changelog.md b/docs/Changelog.md index 310e13fe918..80992227237 100644 --- a/docs/Changelog.md +++ b/docs/Changelog.md @@ -4,7 +4,19 @@ #### Highlights -(Not available yet) +##### End-user + +* New Activities panel, replacing Versions, Comments and Attachments panels. Now you can browse all scenario activities on one chronological list. +* Added scenario labels. You can now organize your scenarios and find different groups of scenarios more easily. +* SpEL: added navigation through fields inside variables typed as Unknown. You can now access the data inside a variable, even if Nussknacker doesn't know its exact type during scenario authoring. +* SpEL: added conversion methods to cast or convert between data types (e.g. `String` to `Integer`). +* SpEL: various enhancements, like `#CONV.toJson` and `#CONV.toJsonString` methods, new `#BASE64` helper, possibility to treat arrays as lists, and more. +* Various UX improvements, including new UI for aggregation definitions and better validation handling in ad-hoc tests. + +##### Administrator + +* Flink upgrade to 1.19.1. Note: it is possible to use Nussknacker with older versions of Flink, but it requires some extra steps. See [Migration guide](MigrationGuide.md) for details. +* Performance optimisations of the serialisation of events passing through Flink's `DataStream`s. ### 1.18.0 (Not released yet) From 98d45ad71f772bfb0506c7d5c4b657b0d15f05cf Mon Sep 17 00:00:00 2001 From: Dawid Poliszak Date: Fri, 8 Nov 2024 14:48:01 +0100 Subject: [PATCH 4/5] Nu 1865 activities changes (#7133) * NU-1865 fix issue with a search of removed attachment --- designer/client/cypress/e2e/activities.cy.ts | 15 +++++------ ...og.tsx => ModifyActivityCommentDialog.tsx} | 8 +++--- .../ActivityPanelRowItem/ActivityItem.tsx | 12 +++------ .../ActivityItemComment.tsx | 10 ++++++- .../ActivityItemCommentModify.tsx | 27 ++++++++++++++++--- .../ActivityItemHeader.tsx | 8 +++++- .../helpers/extendActivitiesWithUIData.ts | 27 +++++++++++++------ .../components/toolbars/activities/types.ts | 7 ++++- .../activities/useActivitiesSearch.test.ts | 20 +++++++++++++- .../src/windowManager/ContentGetter.tsx | 6 ++--- .../client/src/windowManager/WindowKind.tsx | 2 +- 11 files changed, 100 insertions(+), 42 deletions(-) rename designer/client/src/components/modals/{ModifyExistingCommentDialog.tsx => ModifyActivityCommentDialog.tsx} (91%) diff --git a/designer/client/cypress/e2e/activities.cy.ts b/designer/client/cypress/e2e/activities.cy.ts index add5f128625..06b565a1a74 100644 --- a/designer/client/cypress/e2e/activities.cy.ts +++ b/designer/client/cypress/e2e/activities.cy.ts @@ -100,15 +100,12 @@ describe("Activities", () => { makeScreenshot(); // modify comment - cy.intercept("/api/processes/*/activity/comment/*").as("modifyComment"); - cy.get("[data-testid=activity-row-3]").as("modifyCommentRow").trigger("mouseover"); - cy.get("@modifyCommentRow").find("[data-testid=edit-comment-icon]").click(); + cy.intercept("/api/processes/*/activity/comment/*").as("editComment"); + cy.get("[data-testid=activity-row-3]").as("editCommentRow").trigger("mouseover"); + cy.get("@editCommentRow").find("[data-testid=edit-comment-icon]").click(); cy.get("[data-testid=window]").find("textarea").eq(0).type(" new comment"); - cy.get("[data-testid=window]") - .find("button") - .contains(/^Modify/i) - .click(); - cy.wait("@modifyComment"); - cy.get("@modifyCommentRow").contains("test comment new comment").should("be.visible"); + cy.get("[data-testid=window]").find("button").contains(/^Edit/i).click(); + cy.wait("@editComment"); + cy.get("@editCommentRow").contains("test comment new comment").should("be.visible"); }); }); diff --git a/designer/client/src/components/modals/ModifyExistingCommentDialog.tsx b/designer/client/src/components/modals/ModifyActivityCommentDialog.tsx similarity index 91% rename from designer/client/src/components/modals/ModifyExistingCommentDialog.tsx rename to designer/client/src/components/modals/ModifyActivityCommentDialog.tsx index fdde7573ca0..181961c82a8 100644 --- a/designer/client/src/components/modals/ModifyExistingCommentDialog.tsx +++ b/designer/client/src/components/modals/ModifyActivityCommentDialog.tsx @@ -12,7 +12,7 @@ import { getProcessName } from "../../reducers/selectors/graph"; import { getScenarioActivities } from "../../actions/nk/scenarioActivities"; import { ModifyActivityCommentMeta } from "../toolbars/activities/types"; -const ModifyExistingCommentDialog = (props: WindowContentProps) => { +const ModifyActivityCommentDialog = (props: WindowContentProps) => { const meta = props.data.meta; const [comment, setState] = useState(meta.existingComment); const { t } = useTranslation(); @@ -35,9 +35,9 @@ const ModifyExistingCommentDialog = (props: WindowContentProps [ { title: t("dialog.button.cancel", "Cancel"), action: () => props.close(), classname: LoadingButtonTypes.secondaryButton }, - { title: t("dialog.button.modify", "Modify"), action: () => confirmAction() }, + { title: meta.confirmButtonText, action: () => confirmAction() }, ], - [confirmAction, props, t], + [confirmAction, meta.confirmButtonText, props, t], ); return ( @@ -65,4 +65,4 @@ const ModifyExistingCommentDialog = (props: WindowContentProps )} - {activity?.attachment?.file.status === "DELETED" && ( - - {t("activityItem.attachmentRemoved", "File β€˜{{filename}}’ removed", { - filename: activity.attachment.filename, - })} - - )} - {activity.additionalFields.map((additionalField, index) => { - const additionalFieldText = `${additionalField.name}: ${additionalField.value}`; + const additionalFieldText = additionalField.name + ? `${additionalField.name}: ${additionalField.value}` + : additionalField.value; return ( diff --git a/designer/client/src/components/toolbars/activities/ActivityPanelRowItem/ActivityItemComment.tsx b/designer/client/src/components/toolbars/activities/ActivityPanelRowItem/ActivityItemComment.tsx index 3d65c1006e9..3ba7a445fa9 100644 --- a/designer/client/src/components/toolbars/activities/ActivityPanelRowItem/ActivityItemComment.tsx +++ b/designer/client/src/components/toolbars/activities/ActivityPanelRowItem/ActivityItemComment.tsx @@ -81,6 +81,8 @@ const CommentActivity = ({ commentContent={activityComment.content} data-testid={`edit-comment-icon`} key={activityAction.id} + title={t("panels.actions.editComment.title", "Edit comment")} + confirmButtonText={t("panels.actions.editComment.confirmButton", "Edit")} {...getEventTrackingProps({ selector: EventTrackingSelector.ScenarioActivitiesEditComment })} /> ); @@ -108,7 +110,13 @@ export const ActivityItemComment = ({ comment, searchQuery, activityActions, sce }; return ( - + { +export const ActivityItemCommentModify = ({ + commentContent, + scenarioActivityId, + activityType, + activityAction, + title, + confirmButtonText, + ...props +}: Props) => { const featuresSettings = useSelector(getFeatureSettings); const { open } = useWindows(); @@ -19,19 +29,28 @@ export const ActivityItemCommentModify = ({ commentContent, scenarioActivityId, const permittedModifyCommentTypes: ActivityType[] = ["SCENARIO_DEPLOYED", "SCENARIO_CANCELED", "SCENARIO_PAUSED"]; open({ - title: "Modify comment", + title, isModal: true, shouldCloseOnEsc: true, - kind: WindowKind.modifyComment, + kind: WindowKind.modifyActivityComment, meta: { existingComment: commentContent.value, scenarioActivityId, placeholder: permittedModifyCommentTypes.includes(activityType) ? featuresSettings?.deploymentCommentSettings?.exampleComment : undefined, + confirmButtonText, }, }); - }, [activityType, commentContent.value, featuresSettings?.deploymentCommentSettings?.exampleComment, open, scenarioActivityId]); + }, [ + activityType, + commentContent.value, + confirmButtonText, + featuresSettings?.deploymentCommentSettings?.exampleComment, + open, + scenarioActivityId, + title, + ]); return ( ({ width: "16px", height: "16px", color: theme.palette.primary.main, + svg: { + width: "16px", + height: "16px", + }, })); const StyledHeaderActionRoot = styled("div")(({ theme }) => ({ @@ -37,7 +41,7 @@ const StyledActivityItemHeader = styled("div")<{ isHighlighted: boolean; isRunni ({ theme, isHighlighted, isRunning, isActiveFound }) => ({ display: "flex", alignItems: "center", - padding: theme.spacing(0.5, 0, 0.5, 0.75), + padding: theme.spacing(0.5, 0.5, 0.5, 0.75), borderRadius: theme.spacing(0.5), ...getHeaderColors(theme, isHighlighted, isRunning, isActiveFound), }), @@ -149,6 +153,8 @@ const HeaderActivity = ({ scenarioActivityId={scenarioActivityId} activityType={activityType} activityAction={activityAction} + title={t("panels.actions.addComment.title", "Add comment")} + confirmButtonText={t("panels.actions.addComment.confirmButton", "Add")} {...getEventTrackingProps({ selector: EventTrackingSelector.ScenarioActivitiesAddCommentToActivity })} /> ); diff --git a/designer/client/src/components/toolbars/activities/helpers/extendActivitiesWithUIData.ts b/designer/client/src/components/toolbars/activities/helpers/extendActivitiesWithUIData.ts index f9d231d7b8e..7ab58e85846 100644 --- a/designer/client/src/components/toolbars/activities/helpers/extendActivitiesWithUIData.ts +++ b/designer/client/src/components/toolbars/activities/helpers/extendActivitiesWithUIData.ts @@ -3,6 +3,22 @@ import { v4 as uuid4 } from "uuid"; import { Activity, ButtonActivity, DateActivity, UIActivity } from "../ActivitiesPanel"; import { formatDate } from "./date"; +const createUiActivity = (activity: Activity) => { + const uiActivity: UIActivity = { + ...activity, + isActiveFound: false, + isFound: false, + uiGeneratedId: uuid4(), + uiType: "item", + isHidden: false, + }; + + if (uiActivity?.attachment?.file?.status === "DELETED") { + uiActivity.additionalFields.push({ name: "", value: `File '${uiActivity.attachment.filename}' removed` }); + } + + return uiActivity; +}; const getLatestDateItem = (uiActivities: UIActivity[]) => { let previousDateItem: DateActivity | undefined; @@ -123,14 +139,9 @@ export const extendActivitiesWithUIData = (activitiesDataWithMetadata: Activity[ const dateLabel = recursiveDateLabelDesignation(activity, index); const toggleItemsButton = recursiveToggleItemsButtonDesignation(activity, index); dateLabel && uiActivities.push(dateLabel); - uiActivities.push({ - ...activity, - isActiveFound: false, - isFound: false, - uiGeneratedId: uuid4(), - uiType: "item", - isHidden: false, - }); + + uiActivities.push(createUiActivity(activity)); + if (toggleItemsButton) { initiallyHideItems(toggleItemsButton.sameItemOccurrence); uiActivities.push(toggleItemsButton); diff --git a/designer/client/src/components/toolbars/activities/types.ts b/designer/client/src/components/toolbars/activities/types.ts index 5d8059a7303..2c7712826ae 100644 --- a/designer/client/src/components/toolbars/activities/types.ts +++ b/designer/client/src/components/toolbars/activities/types.ts @@ -80,4 +80,9 @@ export interface ActivityMetadataResponse { actions: ActionMetadata[]; } -export type ModifyActivityCommentMeta = { existingComment?: string; scenarioActivityId: string; placeholder?: string }; +export type ModifyActivityCommentMeta = { + existingComment?: string; + scenarioActivityId: string; + placeholder?: string; + confirmButtonText: string; +}; diff --git a/designer/client/src/components/toolbars/activities/useActivitiesSearch.test.ts b/designer/client/src/components/toolbars/activities/useActivitiesSearch.test.ts index 6565ecab364..8e402127ed4 100644 --- a/designer/client/src/components/toolbars/activities/useActivitiesSearch.test.ts +++ b/designer/client/src/components/toolbars/activities/useActivitiesSearch.test.ts @@ -94,17 +94,35 @@ const sampleActivitiesResponse: ActivitiesResponse["activities"] = [ ], type: "SCENARIO_NAME_CHANGED", }, + { + id: "524adff3-fb7d-42d1-bf92-f0903d548d00", + user: "writer", + date: "2022-11-07T11:55:53.127796Z", + scenarioVersionId: 7, + attachment: { + file: { + status: "DELETED", + }, + filename: "Screenshot 2022-10-21 at 08.07.38.png", + lastModifiedBy: "writer", + lastModifiedAt: "2022-11-07T11:55:53.127796Z", + }, + additionalFields: [], + overrideDisplayableName: "File removed", + type: "ATTACHMENT_ADDED", + }, ]; const mockedActivities = extendActivitiesWithUIData(mergeActivityDataWithMetadata(sampleActivitiesResponse, sampleMetadataResponse)); describe(useActivitiesSearch.name, () => { it.each<[string, string[]]>([ - ["atta", [mockedActivities[4].uiGeneratedId]], + ["atta", [mockedActivities[4].uiGeneratedId, mockedActivities[9].uiGeneratedId]], ["3 saved", [mockedActivities[3].uiGeneratedId]], ["2024-09-27", [mockedActivities[1].uiGeneratedId]], ["tests save", [mockedActivities[3].uiGeneratedId]], ["newName: old marketing campaign", [mockedActivities[7].uiGeneratedId]], + [".png", [mockedActivities[9].uiGeneratedId]], ])("should find elements when query is '%s'", (searchQuery, expected) => { const handleScrollToItemMock = jest.fn(); const handleUpdateScenarioActivitiesMock = jest.fn(); diff --git a/designer/client/src/windowManager/ContentGetter.tsx b/designer/client/src/windowManager/ContentGetter.tsx index 8a1496a164f..9c7d2528af2 100644 --- a/designer/client/src/windowManager/ContentGetter.tsx +++ b/designer/client/src/windowManager/ContentGetter.tsx @@ -53,7 +53,7 @@ const AddCommentDialog = loadable(() => import("../components/modals/AddCommentD fallback: , }); -const ModifyExistingCommentDialog = loadable(() => import("../components/modals/ModifyExistingCommentDialog"), { +const ModifyActivityCommentDialog = loadable(() => import("../components/modals/ModifyActivityCommentDialog"), { fallback: , }); @@ -97,8 +97,8 @@ const contentGetter: React.FC> = (props) => { return ; case WindowKind.addComment: return ; - case WindowKind.modifyComment: - return ; + case WindowKind.modifyActivityComment: + return ; case WindowKind.addAttachment: return ; default: diff --git a/designer/client/src/windowManager/WindowKind.tsx b/designer/client/src/windowManager/WindowKind.tsx index 8cd65baf825..5e19a857daf 100644 --- a/designer/client/src/windowManager/WindowKind.tsx +++ b/designer/client/src/windowManager/WindowKind.tsx @@ -19,6 +19,6 @@ export enum WindowKind { viewDescription, editDescription, addComment, - modifyComment, + modifyActivityComment, addAttachment, } From 4a19a7718d76a5169bf30db20cd8351531883f8e Mon Sep 17 00:00:00 2001 From: Arek Burdach Date: Tue, 12 Nov 2024 14:36:14 +0100 Subject: [PATCH 5/5] =?UTF-8?q?[NU-1848]=20Restored=20predefined=20TypeInf?= =?UTF-8?q?o=20in=20TypingResultAwareTypeInform=E2=80=A6=20(#7130)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [NU-1848] Restored predefined TypeInfo in TypingResultAwareTypeInformationDetection for Flink below 1.19 * serialization fix --- .../FlinkTypeInfoRegistrar.scala | 9 ++++-- ...gResultAwareTypeInformationDetection.scala | 28 ++++++++++++++++++- ...ultAwareTypeInformationDetectionSpec.scala | 24 +++++++++++++++- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/engine/flink/components-api/src/main/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/FlinkTypeInfoRegistrar.scala b/engine/flink/components-api/src/main/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/FlinkTypeInfoRegistrar.scala index a7b024d47df..0c3b9df6c8c 100644 --- a/engine/flink/components-api/src/main/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/FlinkTypeInfoRegistrar.scala +++ b/engine/flink/components-api/src/main/scala/pl/touk/nussknacker/engine/flink/api/typeinformation/FlinkTypeInfoRegistrar.scala @@ -16,9 +16,10 @@ object FlinkTypeInfoRegistrar { private val DisableFlinkTypeInfoRegistrationEnvVarName = "NU_DISABLE_FLINK_TYPE_INFO_REGISTRATION" - private case class RegistrationEntry[T](klass: Class[T], factoryClass: Class[_ <: TypeInfoFactory[T]]) + // These members are package protected for purpose of TypingResultAwareTypeInformationDetection.FlinkBelow119AdditionalTypeInfo - see comment there + private[engine] case class RegistrationEntry[T](klass: Class[T], factoryClass: Class[_ <: TypeInfoFactory[T]]) - private val typeInfoToRegister = List( + private[engine] val typeInfoToRegister = List( RegistrationEntry(classOf[LocalDate], classOf[LocalDateTypeInfoFactory]), RegistrationEntry(classOf[LocalTime], classOf[LocalTimeTypeInfoFactory]), RegistrationEntry(classOf[LocalDateTime], classOf[LocalDateTimeTypeInfoFactory]), @@ -46,7 +47,9 @@ object FlinkTypeInfoRegistrar { } // These methods are mainly for purpose of tests in nussknacker-flink-compatibility project - // It should be used in caution as it changes the global state + // It should be used with caution as it changes the global state. They will be removed when we stop supporting Flink < 1.19 + def isFlinkTypeInfoRegistrationEnabled: Boolean = typeInfoRegistrationEnabled.get() + def enableFlinkTypeInfoRegistration(): Unit = { typeInfoRegistrationEnabled.set(true) } diff --git a/engine/flink/executor/src/main/scala/pl/touk/nussknacker/engine/process/typeinformation/TypingResultAwareTypeInformationDetection.scala b/engine/flink/executor/src/main/scala/pl/touk/nussknacker/engine/process/typeinformation/TypingResultAwareTypeInformationDetection.scala index 396b668266d..131137fd1bd 100644 --- a/engine/flink/executor/src/main/scala/pl/touk/nussknacker/engine/process/typeinformation/TypingResultAwareTypeInformationDetection.scala +++ b/engine/flink/executor/src/main/scala/pl/touk/nussknacker/engine/process/typeinformation/TypingResultAwareTypeInformationDetection.scala @@ -8,7 +8,7 @@ import pl.touk.nussknacker.engine.api.context.ValidationContext import pl.touk.nussknacker.engine.api.typed.typing._ import pl.touk.nussknacker.engine.api.{Context, ValueWithContext} import pl.touk.nussknacker.engine.flink.api.TypedMultiset -import pl.touk.nussknacker.engine.flink.api.typeinformation.TypeInformationDetection +import pl.touk.nussknacker.engine.flink.api.typeinformation.{FlinkTypeInfoRegistrar, TypeInformationDetection} import pl.touk.nussknacker.engine.flink.typeinformation.ConcreteCaseClassTypeInfo import pl.touk.nussknacker.engine.process.typeinformation.internal.ContextTypeHelpers import pl.touk.nussknacker.engine.process.typeinformation.internal.typedobject.{ @@ -17,6 +17,8 @@ import pl.touk.nussknacker.engine.process.typeinformation.internal.typedobject.{ } import pl.touk.nussknacker.engine.util.Implicits._ +import scala.jdk.CollectionConverters._ + // TODO: handle avro types - see FlinkConfluentUtils /* This class generates TypeInformation based on ValidationContext and TypingResult. @@ -41,6 +43,7 @@ class TypingResultAwareTypeInformationDetection extends TypeInformationDetection def forType[T](typingResult: TypingResult): TypeInformation[T] = { (typingResult match { + case FlinkBelow119AdditionalTypeInfo(typeInfo) => typeInfo case TypedClass(klass, elementType :: Nil) if klass == classOf[java.util.List[_]] => new ListTypeInfo[AnyRef](forType[AnyRef](elementType)) case TypedClass(klass, elementType :: Nil) if klass == classOf[Array[AnyRef]] => @@ -72,6 +75,29 @@ class TypingResultAwareTypeInformationDetection extends TypeInformationDetection }).asInstanceOf[TypeInformation[T]] } + // This extractor is to allow using of predefined type infos in Flink < 1.19. Type info registration was added in 1.19 + // It should be removed when we stop supporting Flink < 1.19 + private object FlinkBelow119AdditionalTypeInfo extends Serializable { + + def unapply(typingResult: TypingResult): Option[TypeInformation[_]] = { + if (FlinkTypeInfoRegistrar.isFlinkTypeInfoRegistrationEnabled) { + None + } else { + for { + clazz <- Option(typingResult).collect { case TypedClass(clazz, Nil) => + clazz + } + typeInfo <- FlinkTypeInfoRegistrar.typeInfoToRegister.collectFirst { + case FlinkTypeInfoRegistrar.RegistrationEntry(`clazz`, factoryClass) => + val factory = factoryClass.getDeclaredConstructor().newInstance() + factory.createTypeInfo(clazz, Map.empty[String, TypeInformation[_]].asJava) + } + } yield typeInfo + } + } + + } + private def createScalaMapTypeInformation(typingResult: TypedObjectTypingResult) = TypedScalaMapTypeInformation(typingResult.fields.mapValuesNow(forType), constructIntermediateCompatibilityResult) diff --git a/engine/flink/executor/src/test/scala/pl/touk/nussknacker/engine/process/typeinformation/TypingResultAwareTypeInformationDetectionSpec.scala b/engine/flink/executor/src/test/scala/pl/touk/nussknacker/engine/process/typeinformation/TypingResultAwareTypeInformationDetectionSpec.scala index af15bbcf529..8ed2795c1d6 100644 --- a/engine/flink/executor/src/test/scala/pl/touk/nussknacker/engine/process/typeinformation/TypingResultAwareTypeInformationDetectionSpec.scala +++ b/engine/flink/executor/src/test/scala/pl/touk/nussknacker/engine/process/typeinformation/TypingResultAwareTypeInformationDetectionSpec.scala @@ -4,7 +4,7 @@ import com.esotericsoftware.kryo.io.{Input, Output} import com.esotericsoftware.kryo.{Kryo, Serializer} import com.github.ghik.silencer.silent import org.apache.flink.api.common.ExecutionConfig -import org.apache.flink.api.common.typeinfo.TypeInformation +import org.apache.flink.api.common.typeinfo.{TypeInformation, Types} import org.apache.flink.api.common.typeutils.TypeSerializer import org.apache.flink.api.common.typeutils.base.array.StringArraySerializer import org.apache.flink.api.common.typeutils.base.{ @@ -23,10 +23,12 @@ import pl.touk.nussknacker.engine.api.context.ValidationContext import pl.touk.nussknacker.engine.api.typed.typing.Typed import pl.touk.nussknacker.engine.api.{Context, ValueWithContext} import pl.touk.nussknacker.engine.flink.api.typeinfo.caseclass.ScalaCaseClassSerializer +import pl.touk.nussknacker.engine.flink.api.typeinformation.{FlinkTypeInfoRegistrar, TypeInformationDetection} import pl.touk.nussknacker.engine.flink.serialization.FlinkTypeInformationSerializationMixin import pl.touk.nussknacker.engine.process.typeinformation.internal.typedobject._ import pl.touk.nussknacker.engine.process.typeinformation.testTypedObject.CustomTypedObject +import java.time.{LocalDate, LocalDateTime, LocalTime} import scala.jdk.CollectionConverters._ @silent("deprecated") @@ -216,6 +218,26 @@ class TypingResultAwareTypeInformationDetectionSpec oldSerializerSnapshot.resolveSchemaCompatibility(removeFieldSerializer).isCompatibleAfterMigration shouldBe true } + test("return type info for LocalDate, LocalTime and LocalDateTime even if type info registration is disabled") { + withFlinkTypeInfoRegistrationDisabled { + TypeInformationDetection.instance.forClass[LocalDate] shouldBe Types.LOCAL_DATE + TypeInformationDetection.instance.forClass[LocalTime] shouldBe Types.LOCAL_TIME + TypeInformationDetection.instance.forClass[LocalDateTime] shouldBe Types.LOCAL_DATE_TIME + } + } + + private def withFlinkTypeInfoRegistrationDisabled[T](f: => T): T = { + val stateBeforeChange = FlinkTypeInfoRegistrar.isFlinkTypeInfoRegistrationEnabled + FlinkTypeInfoRegistrar.disableFlinkTypeInfoRegistration() + try { + f + } finally { + if (stateBeforeChange) { + FlinkTypeInfoRegistrar.enableFlinkTypeInfoRegistration() + } + } + } + // We have to compare it this way because context can contains arrays private def checkContextAreSame(givenContext: Context, expectedContext: Context): Unit = { givenContext.id shouldEqual expectedContext.id