Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.18 ports5 #7209

Merged
merged 16 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ object ProcessCompilationError {
extends PartSubGraphCompilationError
with InASingleNode

final case class RequireValueFromEmptyFixedList(paramName: ParameterName, nodeIds: Set[String])
final case class EmptyFixedListForRequiredField(paramName: ParameterName, nodeIds: Set[String])
extends PartSubGraphCompilationError

final case class InitialValueNotPresentInPossibleValues(paramName: ParameterName, nodeIds: Set[String])
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package pl.touk.nussknacker.engine.api.json

import io.circe.Json
import pl.touk.nussknacker.engine.util.Implicits._

import scala.jdk.CollectionConverters._

object FromJsonDecoder {

def jsonToAny(json: Json): Any = json.fold(
jsonNull = null,
jsonBoolean = identity[Boolean],
jsonNumber = jsonNumber =>
// we pick the narrowest type as possible to reduce the amount of memory and computations overheads
jsonNumber.toInt orElse
jsonNumber.toLong orElse
// We prefer java big decimal over float/double
jsonNumber.toBigDecimal.map(_.bigDecimal)
getOrElse (throw new IllegalArgumentException(s"Not supported json number: $jsonNumber")),
jsonString = identity[String],
jsonArray = _.map(jsonToAny).asJava,
jsonObject = _.toMap.mapValuesNow(jsonToAny).asJava
)

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package pl.touk.nussknacker.engine.api.typed

import cats.implicits.toTraverseOps
import io.circe.{ACursor, Decoder, DecodingFailure, Json}
import pl.touk.nussknacker.engine.api.json.FromJsonDecoder
import pl.touk.nussknacker.engine.api.typed.typing._

import java.math.BigInteger
Expand Down Expand Up @@ -58,19 +59,15 @@ object ValueDecoder {
case record: TypedObjectTypingResult =>
for {
fieldsJson <- obj.as[Map[String, Json]]
decodedFields <- record.fields.toList.traverse { case (fieldName, fieldType) =>
fieldsJson.get(fieldName) match {
case Some(fieldJson) => decodeValue(fieldType, fieldJson.hcursor).map(fieldName -> _)
case None =>
Left(
DecodingFailure(
s"Record field '$fieldName' isn't present in encoded Record fields: $fieldsJson",
List()
)
)
decodedFields <-
fieldsJson.toList.traverse { case (fieldName, fieldJson) =>
val fieldType = record.fields.getOrElse(fieldName, Unknown)
decodeValue(fieldType, fieldJson.hcursor).map(fieldName -> _)
}
}
} yield decodedFields.toMap.asJava
case Unknown =>
/// For Unknown we fallback to generic json to any conversion. It won't work for some types such as LocalDate but for others should work correctly
obj.as[Json].map(FromJsonDecoder.jsonToAny)
case typ => Left(DecodingFailure(s"Decoding of type [$typ] is not supported.", List()))
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package pl.touk.nussknacker.engine.api.json

import io.circe.Json
import org.scalatest.OptionValues
import org.scalatest.funsuite.AnyFunSuiteLike
import org.scalatest.matchers.should.Matchers

class FromJsonDecoderTest extends AnyFunSuiteLike with Matchers with OptionValues {

test("json number decoding pick the narrowest type") {
FromJsonDecoder.jsonToAny(Json.fromInt(1)) shouldBe 1
FromJsonDecoder.jsonToAny(Json.fromInt(Integer.MAX_VALUE)) shouldBe Integer.MAX_VALUE
FromJsonDecoder.jsonToAny(Json.fromLong(Long.MaxValue)) shouldBe Long.MaxValue
FromJsonDecoder.jsonToAny(
Json.fromBigDecimal(java.math.BigDecimal.valueOf(Double.MaxValue))
) shouldBe java.math.BigDecimal.valueOf(Double.MaxValue)
val moreThanLongMaxValue = BigDecimal(Long.MaxValue) * 10
FromJsonDecoder.jsonToAny(Json.fromBigDecimal(moreThanLongMaxValue)) shouldBe moreThanLongMaxValue.bigDecimal
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,20 @@ class TypingResultDecoderSpec
Map("a" -> TypedObjectWithValue(Typed.typedClass[Int], 1))
),
List(Map("a" -> 1).asJava).asJava
)
),
typedListWithElementValues(
Typed.record(
List(
"a" -> Typed.typedClass[Int],
"b" -> Typed.typedClass[Int]
)
),
List(Map("a" -> 1).asJava, Map("b" -> 2).asJava).asJava
),
typedListWithElementValues(
Unknown,
List(Map("a" -> 1).asJava, 2).asJava
),
).foreach { typing =>
val encoded = TypeEncoders.typingResultEncoder(typing)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class ValueDecoderSpec extends AnyFunSuite with EitherValuesDetailedMessage with
)
}

test("decodeValue should fail when a required Record field is missing") {
test("decodeValue should ignore missing Record field") {
val typedRecord = Typed.record(
Map(
"name" -> Typed.fromInstance("Alice"),
Expand All @@ -45,12 +45,10 @@ class ValueDecoderSpec extends AnyFunSuite with EitherValuesDetailedMessage with
"name" -> "Alice".asJson
)

ValueDecoder.decodeValue(typedRecord, json.hcursor).leftValue.message should include(
"Record field 'age' isn't present in encoded Record fields"
)
ValueDecoder.decodeValue(typedRecord, json.hcursor).rightValue shouldBe Map("name" -> "Alice").asJava
}

test("decodeValue should not include extra fields that aren't typed") {
test("decodeValue should decode extra fields using generic json decoding strategy") {
val typedRecord = Typed.record(
Map(
"name" -> Typed.fromInstance("Alice"),
Expand All @@ -66,8 +64,9 @@ class ValueDecoderSpec extends AnyFunSuite with EitherValuesDetailedMessage with

ValueDecoder.decodeValue(typedRecord, json.hcursor) shouldEqual Right(
Map(
"name" -> "Alice",
"age" -> 30
"name" -> "Alice",
"age" -> 30,
"occupation" -> "nurse"
).asJava
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ package pl.touk.nussknacker.openapi.extractor

import java.util.Collections
import io.circe.Json
import pl.touk.nussknacker.engine.json.swagger.extractor.FromJsonDecoder
import pl.touk.nussknacker.engine.json.swagger.decode.FromJsonSchemaBasedDecoder
import pl.touk.nussknacker.engine.json.swagger.{SwaggerArray, SwaggerTyped}

object HandleResponse {

def apply(res: Option[Json], responseType: SwaggerTyped): AnyRef = res match {
case Some(json) =>
FromJsonDecoder.decode(json, responseType)
FromJsonSchemaBasedDecoder.decode(json, responseType)
case None =>
responseType match {
case _: SwaggerArray => Collections.EMPTY_LIST
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion designer/client/cypress/e2e/fragment.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe("Fragment", () => {
cy.get("[data-testid='settings:4']").contains("Typing...").should("not.exist");
cy.get("[data-testid='settings:4']").find("[id='ace-editor']").type("{enter}");
cy.get("[data-testid='settings:4']")
.contains(/Add list item/i)
.contains(/Suggested values/i)
.siblings()
.eq(0)
.find("[data-testid='form-helper-text']")
Expand Down
8 changes: 8 additions & 0 deletions designer/client/cypress/e2e/labels.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ describe("Scenario labels", () => {

cy.get("[data-testid=scenario-label-0]").should("be.visible").contains("tagX");

cy.get("@labelInput").should("be.visible").click().type("tagX");

cy.wait("@labelvalidation");

cy.get("@labelInput").should("be.visible").contains("This label already exists. Please enter a unique value.");

cy.get("@labelInput").find("input").clear();

cy.get("@labelInput").should("be.visible").click().type("tag2");

cy.wait("@labelvalidation");
Expand Down
2 changes: 1 addition & 1 deletion designer/client/cypress/e2e/process.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ describe("Process", () => {
cy.layoutScenario();

cy.contains("button", "ad hoc").should("be.enabled").click();
cy.get("[data-testid=window]").should("be.visible").find("input").type("10"); //There should be only one input field
cy.get("[data-testid=window]").should("be.visible").find("#ace-editor").type("10");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Update similar test cases to use #ace-editor selector

Based on the search results, there are several test cases using generic input selectors that should be updated for consistency with the new Ace editor implementation:

  • designer/client/cypress/e2e/process.cy.ts: Multiple instances using generic find("input") for value inputs
  • designer/client/cypress/e2e/fragment.cy.ts: Already uses #ace-editor in some places but has inconsistent usage
  • designer/client/cypress/e2e/compareVersions.cy.ts: Uses #ace-editor consistently
  • designer/client/cypress/e2e/sqlEditor.cy.ts: Uses #ace-editor consistently

The change in the review is part of a larger pattern where value inputs are being migrated to use Ace editor. The codebase shows mixed usage, suggesting an ongoing migration.

🔗 Analysis chain

LGTM! Consider reviewing similar test cases.

The change to use the specific #ace-editor selector instead of a generic input selector makes the test more robust and less prone to flakiness.

Let's verify if similar test cases need to be updated for consistency:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other test cases that might be using generic input selectors
# and could benefit from using the ace-editor selector

# Look for similar patterns in test files
rg -A 5 'find\("input"\)' 'designer/client/cypress'

# Look for other ace editor usages
rg -A 5 'ace-editor' 'designer/client/cypress'

Length of output: 16671

cy.get("[data-testid=window]")
.contains(/^test$/i)
.should("be.enabled")
Expand Down
27 changes: 16 additions & 11 deletions designer/client/src/components/AddProcessDialog.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { WindowButtonProps, WindowContentProps } from "@touk/window-manager";
import React, { useCallback, useEffect, useMemo, useState } from "react";
import React, { useCallback, useMemo, useState } from "react";
import { useTranslation } from "react-i18next";
import { visualizationUrl } from "../common/VisualizationUrl";
import { useProcessNameValidators } from "../containers/hooks/useProcessNameValidators";
import HttpService, { ProcessingMode, ScenarioParametersCombination } from "../http/HttpService";
import HttpService, { ProcessingMode } from "../http/HttpService";
import { WindowContent } from "../windowManager";
import { AddProcessForm, FormValue, TouchedValue } from "./AddProcessForm";
import { extendErrors, mandatoryValueValidator } from "./graph/node-modal/editors/Validators";
Expand All @@ -12,6 +12,7 @@ import { NodeValidationError } from "../types";
import { flow, isEmpty, transform } from "lodash";
import { useProcessFormDataOptions } from "./useProcessFormDataOptions";
import { LoadingButtonTypes } from "../windowManager/LoadingButton";
import { useGetAllCombinations } from "./useGetAllCombinations";

interface AddProcessDialogProps extends WindowContentProps {
isFragment?: boolean;
Expand All @@ -22,16 +23,26 @@ export function AddProcessDialog(props: AddProcessDialogProps): JSX.Element {
const { t } = useTranslation();
const { isFragment = false, errors = [], ...passProps } = props;
const nameValidators = useProcessNameValidators();
const [value, setState] = useState<FormValue>({ processName: "", processCategory: "", processingMode: "", processEngine: "" });
const [value, setState] = useState<FormValue>({
processName: "",
processCategory: "",
processingMode: "" as ProcessingMode,
processEngine: "",
});
const [touched, setTouched] = useState<TouchedValue>({
processName: false,
processCategory: false,
processingMode: false,
processEngine: false,
});
const [processNameFromBackend, setProcessNameFromBackendError] = useState<NodeValidationError[]>([]);
const [engineSetupErrors, setEngineSetupErrors] = useState<Record<string, string[]>>({});
const [allCombinations, setAllCombinations] = useState<ScenarioParametersCombination[]>([]);

const { engineSetupErrors, allCombinations } = useGetAllCombinations({
processCategory: value.processCategory,
processingMode: value.processingMode,
processEngine: value.processEngine,
});

const engineErrors: NodeValidationError[] = (engineSetupErrors[value.processEngine] ?? []).map((error) => ({
fieldName: "processEngine",
errorType: "SaveNotAllowed",
Expand Down Expand Up @@ -122,12 +133,6 @@ export function AddProcessDialog(props: AddProcessDialogProps): JSX.Element {
setTouched(touched);
};

useEffect(() => {
HttpService.fetchScenarioParametersCombinations().then((response) => {
setAllCombinations(response.data.combinations);
setEngineSetupErrors(response.data.engineSetupErrors);
});
}, []);
return (
<WindowContent buttons={buttons} {...passProps}>
<AddProcessForm
Expand Down
2 changes: 1 addition & 1 deletion designer/client/src/components/AddProcessForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { InfoOutlined } from "@mui/icons-material";
import Input from "./graph/node-modal/editors/field/Input";
import { formLabelWidth } from "../containers/theme/styles";

export type FormValue = { processName: string; processCategory: string; processingMode: string; processEngine: string };
export type FormValue = { processName: string; processCategory: string; processingMode: ProcessingMode; processEngine: string };

export type TouchedValue = Record<keyof FormValue, boolean>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ const parametersPath = (node) => {
export function adjustParameters(node: NodeType, parameterDefinitions: UIParameter[]): AdjustReturn {
const path = parametersPath(node);

if (!path) {
if (!path || !parameterDefinitions) {
return { adjustedNode: node, unusedParameters: [] };
}

const currentNode = cloneDeep(node);
const currentParameters = get(currentNode, path);
//TODO: currently dynamic branch parameters are *not* supported...
const adjustedParameters = parameterDefinitions
?.filter((def) => !def.branchParam)
.filter((def) => !def.branchParam)
.map((def) => {
const currentParam = currentParameters.find((p) => p.name == def.name);
const parameterFromDefinition = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,18 @@ export interface AceWrapperProps extends Pick<IAceEditorProps, "value" | "onChan
customAceEditorCompleter?;
showLineNumbers?: boolean;
commands?: AceKeyCommand[];
enableLiveAutocompletion?: boolean;
}

export const DEFAULT_OPTIONS: IAceOptions = {
indentedSoftWrap: false, //removes weird spaces for multiline strings when wrapEnabled=true
enableLiveAutocompletion: true,
enableSnippets: false,
fontSize: 14,
highlightActiveLine: false,
highlightGutterLine: true,
};

const DEFAULF_EDITOR_PROPS: IEditorProps = {
const DEFAULT_EDITOR_PROPS: IEditorProps = {
$blockScrolling: true,
};

Expand Down Expand Up @@ -133,7 +133,15 @@ function editorLangToMode(language: ExpressionLang | string, editorMode?: Editor
}

export default forwardRef(function AceWrapper(
{ inputProps, customAceEditorCompleter, showLineNumbers, wrapEnabled = true, commands = [], ...props }: AceWrapperProps,
{
inputProps,
customAceEditorCompleter,
showLineNumbers,
wrapEnabled = true,
commands = [],
enableLiveAutocompletion = true,
...props
}: AceWrapperProps,
ref: ForwardedRef<ReactAce>,
): JSX.Element {
const { language, readOnly, rows = 1, editorMode } = inputProps;
Expand Down Expand Up @@ -183,9 +191,10 @@ export default forwardRef(function AceWrapper(
className={readOnly ? " read-only" : ""}
wrapEnabled={!!wrapEnabled}
showGutter={!!showLineNumbers}
editorProps={DEFAULF_EDITOR_PROPS}
editorProps={DEFAULT_EDITOR_PROPS}
setOptions={{
...DEFAULT_OPTIONS,
enableLiveAutocompletion,
showLineNumbers,
}}
enableBasicAutocompletion={customAceEditorCompleter && [customAceEditorCompleter]}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ export type CustomCompleterAceEditorProps = {
showValidation?: boolean;
isMarked?: boolean;
className?: string;
enableLiveAutocompletion?: boolean;
};

export function CustomCompleterAceEditor(props: CustomCompleterAceEditorProps): JSX.Element {
const { className, isMarked, showValidation, fieldErrors, validationLabelInfo, completer, isLoading } = props;
const { className, isMarked, showValidation, fieldErrors, validationLabelInfo, completer, isLoading, enableLiveAutocompletion } = props;
const { value, onValueChange, ref, ...inputProps } = props.inputProps;

const [editorFocused, setEditorFocused] = useState(false);
Expand Down Expand Up @@ -65,6 +66,7 @@ export function CustomCompleterAceEditor(props: CustomCompleterAceEditorProps):
...inputProps,
}}
customAceEditorCompleter={completer}
enableLiveAutocompletion={enableLiveAutocompletion}
/>
</Box>
<Fade
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const MarkdownFormControl = ({ value, onChange, className, children, read
{children}
<CustomCompleterAceEditor
{...props}
enableLiveAutocompletion={false}
inputProps={{
language: ExpressionLang.MD,
className,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export const AnyValueWithSuggestionVariant = ({ item, path, onChange, variableTy
typ={item.typ}
name={item.name}
initialValue={item.initialValue}
userDefinedListInputLabel={t("fragment.userDefinedList.label.suggestedValues", "Suggested values:")}
/>
<InitialValue
path={path}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const FixedListVariant = ({ item, path, onChange, readOnly, variableTypes
typ={item.typ}
name={item.name}
initialValue={item.initialValue}
userDefinedListInputLabel={t("fragment.userDefinedList.label.possibleValues", "Possible values:")}
/>
<InitialValue
path={path}
Expand Down
Loading
Loading