Skip to content

Commit 573a816

Browse files
authored
Accept _unsafe_-prefixed forward-compat fields on scenario graph DTOs (#9309)
* Allow arbitrary _unsafeExtra fields on LayoutData without validation * Work around sbt-git's lack of git worktree support Enable useConsoleForROGit when `.git` is a file — the telltale of a worktree checkout — so sbt-git shells out to the git CLI for read-only operations that its embedded JGit cannot perform against worktrees. * Accept arbitrary _unsafe_-prefixed fields on UserDefinedAdditionalNodeFields Replaces the nested _unsafeExtra field on LayoutData with top-level pattern-matched fields on UserDefinedAdditionalNodeFields: any JSON property starting with `_unsafe_` is round-tripped via a dedicated Map[String, Json] on the case class through a hand-written Circe codec. Tapir schema keeps the named $ref for UserDefinedAdditionalNodeFields; extra _unsafe_* properties pass validation because JSON Schema allows additional properties by default. The TS NodeType mirrors this with a UnsafeOptions intersection typed as Record<`_unsafe_${string}`, unknown>. * Accept _unsafe_-prefixed fields on ProcessAdditionalFields Mirrors the node-level mechanism at the scenario level: any JSON property starting with `_unsafe_` sitting next to description / properties / metaDataType / showDescription is round-tripped via a Map[String, Json] on the case class through a hand-written Circe codec. A small `omitSchemaField` helper drops the synthetic `unsafeFields` container from the derived Tapir schema (used for both ProcessAdditionalFields and UserDefinedAdditionalNodeFields) so the generated OpenAPI stays byte-identical. TS ProcessAdditionalFields picks up the matching `& UnsafeOptions` intersection. * Extract the `_unsafe_` prefix convention into shared helpers Both ProcessAdditionalFields and UserDefinedAdditionalNodeFields inlined the same prefix constant and the same extract/merge logic across the Circe codecs; lift that into CirceUtil.UnsafePrefixedFields in common-api so the prefix lives in one place. On the TS side, export the UnsafeOptions type from node.ts and consume it from scenarioGraph.ts instead of redefining the `_unsafe_${string}` template literal locally. * Add codec tests for _unsafe_-prefixed additional fields Cover the round-trip (encoder spreads the map as top-level properties, decoder collects them back) and the symmetric check that non-prefixed extras are still dropped, for both ProcessAdditionalFields and UserDefinedAdditionalNodeFields. * Strip _unsafe_ fields from the scenario before it hits Flink _unsafe_ entries are frontend-only metadata (layout, UX preferences, render hints) that should be persisted in the saved scenario but never leak into the Flink job graph, logs or UI. Add UnsafePrefixedFields.deepStrip that recursively removes _unsafe_-prefixed keys from any JSON tree and expose CanonicalProcess.withoutUnsafeFields as a round-trip wrapper over it; both the remote and mini-cluster Flink job runners now apply it at the boundary. * Link sbt-git PR #243 (which introduced useConsoleForROGit) instead of the stale placeholder URL * Make `$toggleUserFlag` return the resolved flag value so Cypress can assert on it Previously the global helper dispatched the Redux action and returned `void`, forcing tests to separately poll the store to verify the toggle took effect. Now it returns a Promise that resolves to the post-dispatch value, and the Cypress `toggleUserFlag` command asserts the value matches when one was passed.
1 parent 53e800d commit 573a816

15 files changed

Lines changed: 312 additions & 50 deletions

File tree

build.sbt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import com.github.sbt.git.SbtGit.GitKeys.useConsoleForROGit
12
import com.typesafe.sbt.packager.SettingsHelper
23
import com.typesafe.sbt.packager.docker.DockerPlugin.autoImport.dockerUsername
34
import pl.project13.scala.sbt.JmhPlugin
@@ -54,6 +55,15 @@ crossScalaVersions := Nil
5455

5556
ThisBuild / isSnapshot := version(_ contains "-SNAPSHOT").value
5657

58+
// Workaround for sbt-git not supporting git worktrees (see https://github.com/sbt/sbt-git/pull/243).
59+
// In a worktree `.git` is a regular file (containing `gitdir: ...` pointing at the main repo's
60+
// per-worktree git dir), not a directory — and sbt-git's embedded JGit cannot parse that layout,
61+
// which breaks read-only git operations such as gitCurrentBranch / gitHeadCommit used below.
62+
// Flipping `useConsoleForROGit` to true makes sbt-git shell out to the `git` CLI for those reads,
63+
// which handles worktrees correctly. Detecting `.git` as a file keeps the default (JGit) path
64+
// for normal checkouts where `.git` is a directory.
65+
ThisBuild / useConsoleForROGit := (baseDirectory.value / ".git").isFile
66+
5767
lazy val publishSettings = Seq(
5868
publishMavenStyle := true,
5969
releasePublishArtifactsAction := PgpKeys.publishSigned.value,

common-api/src/main/scala/pl/touk/nussknacker/engine/api/CirceUtil.scala

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,26 @@ object CirceUtil {
115115
// that some value is null, but for people it generates a lot of unnecessary noise
116116
val humanReadablePrinter: Printer = Printer.spaces2.copy(dropNullValues = true)
117117

118+
// Helpers for a convention where a case class exposes `unsafeFields: Map[String, Json]` that is
119+
// encoded/decoded as top-level JSON properties prefixed with `_unsafe_` (rather than under a
120+
// nested field). Used to round-trip forward-compatible extension fields without schema changes.
121+
object UnsafePrefixedFields {
122+
val Prefix = "_unsafe_"
123+
124+
def extract(c: HCursor): Map[String, Json] =
125+
c.keys.toList.flatten
126+
.filter(_.startsWith(Prefix))
127+
.flatMap(k => c.downField(k).as[Json].toOption.map(k -> _))
128+
.toMap
129+
130+
// Only entries whose key already starts with [[Prefix]] are merged in — keeps a stray entry in
131+
// the map (e.g. `"description" -> ...`) from clobbering a real top-level field during encode.
132+
def merge(base: JsonObject, unsafeFields: Map[String, Json]): JsonObject =
133+
unsafeFields.foldLeft(base) {
134+
case (obj, (k, v)) if k.startsWith(Prefix) => obj.add(k, v)
135+
case (obj, _) => obj
136+
}
137+
138+
}
139+
118140
}

common-api/src/main/scala/pl/touk/nussknacker/engine/api/MetaData.scala

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
package pl.touk.nussknacker.engine.api
22

3-
import io.circe.{Decoder, Encoder, HCursor}
3+
import io.circe.{Decoder, Encoder, HCursor, Json, JsonObject}
44
import io.circe.generic.JsonCodec
55
import io.circe.generic.extras.ConfiguredJsonCodec
6-
import io.circe.generic.extras.semiauto.{deriveConfiguredDecoder, deriveConfiguredEncoder}
6+
import io.circe.generic.extras.semiauto.deriveConfiguredDecoder
77
import pl.touk.nussknacker.engine.api.CirceUtil._
88
import pl.touk.nussknacker.engine.api.process.ProcessName
99

@@ -86,7 +86,8 @@ case class ProcessAdditionalFields(
8686
description: Option[String],
8787
properties: Map[String, String],
8888
metaDataType: String,
89-
showDescription: Boolean = false
89+
showDescription: Boolean = false,
90+
unsafeFields: Map[String, Json] = Map.empty
9091
) {
9192

9293
def combineTypeSpecificProperties(typeSpecificData: TypeSpecificData): ProcessAdditionalFields = {
@@ -106,20 +107,33 @@ case class ProcessAdditionalFields(
106107
}
107108

108109
object ProcessAdditionalFields {
109-
110-
// TODO: is this currently needed?
111-
private case class OptionalProcessAdditionalFields(
112-
description: Option[String],
113-
properties: Option[Map[String, String]],
114-
metaDataType: String,
115-
showDescription: Boolean = false
116-
)
117-
118-
implicit val circeDecoder: Decoder[ProcessAdditionalFields] =
119-
deriveConfiguredDecoder[OptionalProcessAdditionalFields].map(opp =>
120-
ProcessAdditionalFields(opp.description, opp.properties.getOrElse(Map()), opp.metaDataType, opp.showDescription)
110+
import io.circe.syntax._
111+
112+
implicit val circeEncoder: Encoder.AsObject[ProcessAdditionalFields] = Encoder.AsObject.instance { f =>
113+
UnsafePrefixedFields.merge(
114+
JsonObject(
115+
"description" -> f.description.asJson,
116+
"properties" -> f.properties.asJson,
117+
"metaDataType" -> f.metaDataType.asJson,
118+
"showDescription" -> f.showDescription.asJson,
119+
),
120+
f.unsafeFields
121121
)
122+
}
122123

123-
implicit val circeEncoder: Encoder[ProcessAdditionalFields] = deriveConfiguredEncoder
124+
implicit val circeDecoder: Decoder[ProcessAdditionalFields] = Decoder.instance { c =>
125+
for {
126+
description <- c.get[Option[String]]("description")
127+
properties <- c.get[Option[Map[String, String]]]("properties")
128+
metaDataType <- c.get[String]("metaDataType")
129+
showDescription <- c.getOrElse[Boolean]("showDescription")(false)
130+
} yield ProcessAdditionalFields(
131+
description,
132+
properties.getOrElse(Map.empty),
133+
metaDataType,
134+
showDescription,
135+
UnsafePrefixedFields.extract(c)
136+
)
137+
}
124138

125139
}

designer/client/cypress/support/process.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { padStart } from "lodash";
22

3-
import type { Setting } from "../../src/reducers/userSettings";
3+
import type { GlobalToggleUserFlag } from "../../src/containers/SettingsInitializer";
4+
import type { UserSettings } from "../../src/reducers/userSettings";
45

56
import Chainable = Cypress.Chainable;
67

@@ -327,13 +328,19 @@ function getNode(nameOrAlias: string) {
327328
});
328329
}
329330

330-
function toggleUserFlag(flag: Setting, value?: boolean | undefined) {
331+
function toggleUserFlag(
332+
flag: keyof UserSettings | (string & NonNullable<unknown>),
333+
value?: boolean | undefined,
334+
): Chainable<boolean | "default"> {
331335
return cy
332336
.window()
333337
.its("$toggleUserFlag")
334338
.should("exist")
335-
.then((toggleUserFlag) => {
336-
toggleUserFlag(flag, value);
339+
.then((toggleUserFlag: GlobalToggleUserFlag) => toggleUserFlag(flag, value))
340+
.should((currentSubject) => {
341+
if (value !== undefined) {
342+
expect(currentSubject).to.eq(value);
343+
}
337344
});
338345
}
339346

designer/client/cypress/tsconfig.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
"target": "es5",
44
"lib": ["ES7", "dom"],
55
"types": ["node", "cypress", "@frsource/cypress-plugin-visual-regression-diff", "@4tw/cypress-drag-drop"],
6-
"resolveJsonModule": true
6+
"resolveJsonModule": true,
7+
"jsx": "react-jsx"
78
},
89
"include": ["**/*.ts"]
910
}

designer/client/src/containers/SettingsInitializer.tsx

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@ import { userSettingSet, userSettingsSetInitial, userSettingsToggle } from "../a
88
import { useUserSettings } from "../common/useUserSettings";
99
import LoaderSpinner from "../components/spinner/Spinner";
1010
import HttpService from "../http/HttpService/instance";
11-
import type { UserSettings } from "../reducers/userSettings";
11+
import type { Setting, UserSettings } from "../reducers/userSettings";
1212
import { waitForWindowValue } from "../reducers/waitForWindowValue";
1313
import { useAppDispatch } from "../store/storeHelpers";
1414

15+
export type GlobalToggleUserFlag = (flag: Setting, value?: boolean) => Promise<boolean | "default">;
16+
1517
declare global {
1618
interface Window {
17-
$toggleUserFlag?: <K extends keyof UserSettings>(flag: K, value?: UserSettings[K]) => void;
19+
$toggleUserFlag?: GlobalToggleUserFlag;
1820
$initialUserFlags?: UserSettings;
1921
}
2022
}
@@ -24,13 +26,17 @@ export function SettingsProvider({ children }: PropsWithChildren<unknown>): Reac
2426
const dispatch = useAppDispatch();
2527

2628
useEffect(() => {
27-
window.$toggleUserFlag = (flag, value) => {
28-
if (value !== undefined) {
29-
dispatch(userSettingSet(flag, value));
30-
} else {
31-
dispatch(userSettingsToggle([flag]));
32-
}
33-
};
29+
window.$toggleUserFlag = (flag, value) =>
30+
new Promise((resolve) => {
31+
if (value !== undefined) {
32+
dispatch(userSettingSet(flag, value));
33+
} else {
34+
dispatch(userSettingsToggle([flag]));
35+
}
36+
dispatch((_dispatch, getState) => {
37+
resolve(getState().userSettings.values[flag]);
38+
});
39+
});
3440
waitForWindowValue("$initialUserFlags").then((flags) => {
3541
dispatch(userSettingsSetInitial(flags));
3642
});

designer/client/src/types/node.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ type Type = "FragmentInput" | typeof StickyNoteType | "Source" | "Sink" | "Enric
77

88
export type LayoutData = { x: number; y: number };
99

10+
export type UnsafeOptions = Record<`_unsafe_${string}`, unknown>;
11+
1012
export interface BranchParams {
1113
branchId: string;
1214
parameters: Field[];
@@ -23,7 +25,7 @@ export type NodeType<F extends Field = Field> = {
2325
description?: string;
2426
layoutData?: LayoutData;
2527
creatorType?: string;
26-
};
28+
} & UnsafeOptions;
2729
parameters?: Parameter[];
2830
branchParameters?: BranchParams[];
2931
branchParametersTemplate?: BranchParametersTemplate;

designer/client/src/types/scenarioGraph.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { TestCase } from "../reducers/graph/testCase";
33
import type { ComponentGroup } from "./component";
44
import type { TypingResult, UIParameter } from "./definition";
55
import type { Edge, EdgeType } from "./edge";
6-
import type { NodeType, PropertiesType } from "./node";
6+
import type { NodeType, PropertiesType, UnsafeOptions } from "./node";
77

88
export type ScenarioGraph = {
99
nodes: NodeType[];
@@ -21,7 +21,7 @@ export type ProcessAdditionalFields = {
2121
properties: { [key in PropertiesConfigKeys]: string };
2222
metaDataType: string;
2323
showDescription?: boolean;
24-
};
24+
} & UnsafeOptions;
2525

2626
export interface UIScenarioProperty {
2727
defaultValue?: string;

designer/deployment-manager-api/src/main/scala/pl/touk/nussknacker/engine/api/deployment/DMScenarioCommand.scala

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,30 @@ case class DMValidateScenarioCommand(
3737
* We assume that validate was already called and was successful, currently savepointPath is flink specific, but we could
3838
* leverage this concept also for other engines
3939
*/
40-
case class DMRunDeploymentCommand(
40+
case class DMRunDeploymentCommand private (
4141
processVersion: ProcessVersion,
4242
deploymentData: DeploymentData,
4343
canonicalProcess: CanonicalProcess,
4444
// TODO: job update strategy should be a part of deployment service logic, instead of DeploymentManager's
4545
updateStrategy: DeploymentUpdateStrategy
4646
) extends DMScenarioCommand[Option[ExternalDeploymentId]]
4747

48+
object DMRunDeploymentCommand {
49+
50+
// `_unsafe_*` fields are frontend-only metadata (layout, UX, render hints) that must be persisted
51+
// in the saved scenario but never leak into the Flink/Lite job graph. Stripping them here — at the
52+
// single boundary where we build the command — keeps every DeploymentManager honest without
53+
// per-engine plumbing.
54+
def apply(
55+
processVersion: ProcessVersion,
56+
deploymentData: DeploymentData,
57+
canonicalProcess: CanonicalProcess,
58+
updateStrategy: DeploymentUpdateStrategy
59+
): DMRunDeploymentCommand =
60+
new DMRunDeploymentCommand(processVersion, deploymentData, canonicalProcess.withoutUnsafeFields, updateStrategy)
61+
62+
}
63+
4864
case class DMCancelDeploymentCommand(scenarioName: ProcessName, deploymentId: DeploymentId, user: User)
4965
extends DMScenarioCommand[Unit]
5066

designer/server/src/main/scala/pl/touk/nussknacker/ui/api/description/NodesApiEndpoints.scala

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,9 +1022,21 @@ object NodesApiEndpoints {
10221022
implicit lazy val typingResultInJsonSchema: Schema[TypingResultInJson] = TypingDtoSchemas.typingResult.as
10231023
}
10241024

1025-
implicit lazy val additionalInfoSchema: Schema[AdditionalInfo] = Schema.derived
1026-
implicit lazy val scenarioAdditionalFieldsSchema: Schema[ProcessAdditionalFields] = Schema.derived
1027-
implicit lazy val scenarioPropertiesSchema: Schema[ProcessProperties] = Schema.derived.hidden(true)
1025+
// `unsafeFields` is the Scala container for top-level `_unsafe_*` JSON entries; it has no
1026+
// wire representation of its own, so we strip it from the derived schema entirely (marking
1027+
// it hidden with `.modify` leaves a dangling entry in `required`).
1028+
private def omitSchemaField[T](schema: Schema[T], fieldName: String): Schema[T] =
1029+
schema.schemaType match {
1030+
case p: SProduct[T] @unchecked =>
1031+
schema.copy(schemaType = SProduct(p.fields.filterNot(_.name.name == fieldName)))
1032+
case _ => schema
1033+
}
1034+
1035+
implicit lazy val additionalInfoSchema: Schema[AdditionalInfo] = Schema.derived
1036+
implicit lazy val unsafeFieldsSchema: Schema[Map[String, io.circe.Json]] = Schema.anyObject
1037+
implicit lazy val scenarioAdditionalFieldsSchema: Schema[ProcessAdditionalFields] =
1038+
omitSchemaField(Schema.derived[ProcessAdditionalFields], "unsafeFields")
1039+
implicit lazy val scenarioPropertiesSchema: Schema[ProcessProperties] = Schema.derived.hidden(true)
10281040

10291041
implicit lazy val parameterSchema: Schema[EvaluatedParameter] = Schema.derived
10301042
implicit lazy val edgeTypeSchema: Schema[EdgeType] = Schema.derived
@@ -1055,15 +1067,18 @@ object NodesApiEndpoints {
10551067
implicit lazy val fragmentClazzRefSchema: Schema[FragmentClazzRef] = Schema.derived
10561068
implicit lazy val parameterValueCompileTimeValidationSchema: Schema[ParameterValueCompileTimeValidation] =
10571069
Schema.derived
1058-
implicit lazy val parameterValueInputSchema: Schema[ParameterValueInput] = Schema.derived
1059-
implicit lazy val fragmentParameterSchema: Schema[FragmentParameter] = Schema.derived
1060-
implicit lazy val serviceRefSchema: Schema[ServiceRef] = Schema.derived
1061-
implicit lazy val branchEndDefinitionSchema: Schema[BranchEndDefinition] = Schema.derived
1062-
implicit lazy val userDefinedAdditionalNodeFieldsSchema: Schema[UserDefinedAdditionalNodeFields] = Schema.derived
1063-
implicit lazy val layoutDataSchema: Schema[LayoutData] = Schema.derived
1064-
implicit lazy val branchParametersSchema: Schema[BranchParameters] = Schema.derived
1065-
implicit lazy val fieldSchema: Schema[Field] = Schema.derived
1066-
implicit lazy val fragmentOutputVarDefinitionSchema: Schema[FragmentOutputVarDefinition] = Schema.derived
1070+
implicit lazy val parameterValueInputSchema: Schema[ParameterValueInput] = Schema.derived
1071+
implicit lazy val fragmentParameterSchema: Schema[FragmentParameter] = Schema.derived
1072+
implicit lazy val serviceRefSchema: Schema[ServiceRef] = Schema.derived
1073+
implicit lazy val branchEndDefinitionSchema: Schema[BranchEndDefinition] = Schema.derived
1074+
implicit lazy val layoutDataSchema: Schema[LayoutData] = Schema.derived
1075+
1076+
implicit lazy val userDefinedAdditionalNodeFieldsSchema: Schema[UserDefinedAdditionalNodeFields] =
1077+
omitSchemaField(Schema.derived[UserDefinedAdditionalNodeFields], "unsafeFields")
1078+
1079+
implicit lazy val branchParametersSchema: Schema[BranchParameters] = Schema.derived
1080+
implicit lazy val fieldSchema: Schema[Field] = Schema.derived
1081+
implicit lazy val fragmentOutputVarDefinitionSchema: Schema[FragmentOutputVarDefinition] = Schema.derived
10671082

10681083
// Tapir currently supports only json schema v4 which has no way to declare discriminator
10691084
// We declare that each type of NodeData belongs to an enum with only one value as a workaround for this problem

0 commit comments

Comments
 (0)