From cb9ae468e40315fffc8288fbdc7d4088293786e3 Mon Sep 17 00:00:00 2001 From: Robert Isele Date: Wed, 5 Apr 2023 14:03:15 +0200 Subject: [PATCH 1/9] - Serialize boolean parameter values as JSON booleans. - Add uriAttribute and readonly to searchItems API --- .../scala/org/silkframework/util/StringUtils.scala | 13 +++++++++---- .../serialization/json/JsonHelpers.scala | 2 ++ .../serialization/json/PluginSerializers.scala | 8 +++++++- .../workspaceApi/search/SearchApiModel.scala | 13 +++++++++++++ workspace/src/app/utils/transformers.ts | 5 ----- 5 files changed, 31 insertions(+), 10 deletions(-) diff --git a/silk-core/src/main/scala/org/silkframework/util/StringUtils.scala b/silk-core/src/main/scala/org/silkframework/util/StringUtils.scala index 0452a7eb32..4636b0efb0 100644 --- a/silk-core/src/main/scala/org/silkframework/util/StringUtils.scala +++ b/silk-core/src/main/scala/org/silkframework/util/StringUtils.scala @@ -16,7 +16,6 @@ package org.silkframework.util import javax.xml.datatype.{DatatypeFactory, XMLGregorianCalendar} import scala.language.implicitConversions -import scala.util.Try import scala.util.matching.Regex object StringUtils { @@ -76,9 +75,15 @@ object StringUtils { object BooleanLiteral { def apply(x: Boolean): String = x.toString - def unapply(x: String): Option[Boolean] = { - Option(x) flatMap { s => - Try(s.toBoolean).toOption + def unapply(s: String): Option[Boolean] = { + if (s != null) { + s.toLowerCase match { + case "true" => Some(true) + case "false" => Some(false) + case _ => None + } + } else { + None } } } diff --git a/silk-plugins/silk-serialization-json/src/main/scala/org/silkframework/serialization/json/JsonHelpers.scala b/silk-plugins/silk-serialization-json/src/main/scala/org/silkframework/serialization/json/JsonHelpers.scala index 0bb15e75e2..3c9a5817b3 100644 --- a/silk-plugins/silk-serialization-json/src/main/scala/org/silkframework/serialization/json/JsonHelpers.scala +++ b/silk-plugins/silk-serialization-json/src/main/scala/org/silkframework/serialization/json/JsonHelpers.scala @@ -87,6 +87,8 @@ object JsonHelpers { optionalValue(json, attributeName) match { case Some(jsBoolean: JsBoolean) => Some(jsBoolean.value) + case Some(jsString: JsString) => + Some(jsString.value.toBoolean) case Some(_) => throw JsonParseException("Value for attribute '" + attributeName + "' is not a boolean!") case None => diff --git a/silk-plugins/silk-serialization-json/src/main/scala/org/silkframework/serialization/json/PluginSerializers.scala b/silk-plugins/silk-serialization-json/src/main/scala/org/silkframework/serialization/json/PluginSerializers.scala index 6c4e943f2c..7b479ea9df 100644 --- a/silk-plugins/silk-serialization-json/src/main/scala/org/silkframework/serialization/json/PluginSerializers.scala +++ b/silk-plugins/silk-serialization-json/src/main/scala/org/silkframework/serialization/json/PluginSerializers.scala @@ -3,6 +3,7 @@ package org.silkframework.serialization.json import org.silkframework.runtime.plugin._ import org.silkframework.runtime.serialization.{ReadContext, Serialization, WriteContext} import org.silkframework.serialization.json.JsonSerializers.{PARAMETERS, TEMPLATES, TYPE} +import org.silkframework.util.StringUtils.BooleanLiteral import play.api.libs.json._ import scala.reflect.ClassTag @@ -60,7 +61,12 @@ object PluginSerializers { JsObject( params.values.collect { case (key, ParameterStringValue(strValue)) => - (key, JsString(strValue)) + strValue match { + case BooleanLiteral(booleanValue) => + (key, JsBoolean(booleanValue)) + case _ => + (key, JsString(strValue)) + } case (key, template: ParameterTemplateValue) => (key, JsString(template.evaluate())) case (key, ParameterObjectValue(objValue)) => diff --git a/silk-workbench/silk-workbench-workspace/app/controllers/workspaceApi/search/SearchApiModel.scala b/silk-workbench/silk-workbench-workspace/app/controllers/workspaceApi/search/SearchApiModel.scala index 8248b1981c..80fd5471bf 100644 --- a/silk-workbench/silk-workbench-workspace/app/controllers/workspaceApi/search/SearchApiModel.scala +++ b/silk-workbench/silk-workbench-workspace/app/controllers/workspaceApi/search/SearchApiModel.scala @@ -3,6 +3,7 @@ package controllers.workspaceApi.search import controllers.util.TextSearchUtils import io.swagger.v3.oas.annotations.media.{ArraySchema, Schema} import org.silkframework.config.{CustomTask, TaskSpec} +import org.silkframework.dataset.DatasetSpec.GenericDatasetSpec import org.silkframework.dataset.{Dataset, DatasetSpec} import org.silkframework.rule.{LinkSpec, TransformSpec} import org.silkframework.runtime.activity.UserContext @@ -35,6 +36,8 @@ object SearchApiModel { final val PLUGIN_LABEL = "pluginLabel" final val TAGS = "tags" final val PARAMETERS = "parameters" + final val READ_ONLY = "readOnly" + final val URI_PROPERTY = "uriProperty" // type values final val PROJECT_TYPE = "project" /* JSON serialization */ @@ -496,6 +499,15 @@ object SearchApiModel { } else { Seq.empty } + val datasetAttributes = { + task.data match { + case ds: GenericDatasetSpec => + Seq(READ_ONLY -> JsBoolean(ds.readOnly)) ++ + ds.uriAttribute.map(uri => URI_PROPERTY -> JsString(uri)) + case _ => + Seq.empty + } + } JsObject( Seq( PROJECT_ID -> JsString(typedTask.project), @@ -510,6 +522,7 @@ object SearchApiModel { ) ++ task.metaData.description.map(d => DESCRIPTION -> JsString(d)) ++ parameters + ++ datasetAttributes ) } } diff --git a/workspace/src/app/utils/transformers.ts b/workspace/src/app/utils/transformers.ts index d7a2ef100f..c5ae89f19a 100644 --- a/workspace/src/app/utils/transformers.ts +++ b/workspace/src/app/utils/transformers.ts @@ -17,11 +17,6 @@ export const stringValueAsJs = (valueType: string, value: OptionallyLabelledPara const stringValue = value != null ? optionallyLabelledParameterToValue(value) ?? "" : ""; let v: any = stringValue; - if (valueType === INPUT_TYPES.BOOLEAN) { - // cast to boolean from string - v = stringValue.toLowerCase() === "true"; - } - if (valueType === INPUT_TYPES.INTEGER) { if (v !== "" && stringValue) { v = parseInt(stringValue); From 1b37fbb98b2ff338595a33b01b17e4e614f2589f Mon Sep 17 00:00:00 2001 From: Robert Isele Date: Tue, 11 Apr 2023 11:07:11 +0200 Subject: [PATCH 2/9] Ignore test until fixed --- .../CreateArtefactModal.test.tsx | 61 ++++++++++--------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/workspace/test/integration/components/CreateArtefactModal/CreateArtefactModal.test.tsx b/workspace/test/integration/components/CreateArtefactModal/CreateArtefactModal.test.tsx index 3671852a7d..15d859bd9c 100644 --- a/workspace/test/integration/components/CreateArtefactModal/CreateArtefactModal.test.tsx +++ b/workspace/test/integration/components/CreateArtefactModal/CreateArtefactModal.test.tsx @@ -269,36 +269,37 @@ describe("Task creation widget", () => { await expectValidationErrors(wrapper, 6); }); - it("should send the correct request when clicking 'Create' on a valid form", async () => { - const { wrapper, history } = await pluginCreationDialogWrapper(); - changeValue(findSingleElement(wrapper, "#intParam"), "100"); - changeValue(findSingleElement(wrapper, "#label"), "Some label"); - changeValue(findSingleElement(wrapper, "#description"), "Some description"); - changeValue(findSingleElement(wrapper, byName("objectParameter.subStringParam")), "Something"); - clickCreate(wrapper); - await expectValidationErrors(wrapper, 0); - const tasksUri = legacyApiUrl("workspace/projects/projectId/tasks"); - const request = mockAxios.getReqByUrl(tasksUri); - expect(request).toBeTruthy(); - const metaData = request.data.metadata; - const data = request.data.data; - expect(metaData.label).toBe("Some label"); - expect(metaData.description).toBe("Some description"); - expect(data.taskType).toBe(TaskTypes.CUSTOM_TASK); - expect(data.type).toBe("pluginA"); - expect(data.parameters.intParam).toEqual("100"); - expect(data.parameters.booleanParam).toEqual("false"); - expect(data.parameters.objectParameter.subStringParam).toEqual("Something"); - expect(data.parameters.stringParam).toEqual("default string"); - // Test redirection to task details page - const newTaskId = "newTaskId"; - mockAxiosResponse(tasksUri, { data: { id: newTaskId } }); - await waitFor(() => { - expect(history.location.pathname).toEqual( - expect.stringMatching(new RegExp(`projects/${PROJECT_ID}/task/${newTaskId}$`)) - ); - }); - }); + // TODO enable after fix + // it("should send the correct request when clicking 'Create' on a valid form", async () => { + // const { wrapper, history } = await pluginCreationDialogWrapper(); + // changeValue(findSingleElement(wrapper, "#intParam"), "100"); + // changeValue(findSingleElement(wrapper, "#label"), "Some label"); + // changeValue(findSingleElement(wrapper, "#description"), "Some description"); + // changeValue(findSingleElement(wrapper, byName("objectParameter.subStringParam")), "Something"); + // clickCreate(wrapper); + // await expectValidationErrors(wrapper, 0); + // const tasksUri = legacyApiUrl("workspace/projects/projectId/tasks"); + // const request = mockAxios.getReqByUrl(tasksUri); + // expect(request).toBeTruthy(); + // const metaData = request.data.metadata; + // const data = request.data.data; + // expect(metaData.label).toBe("Some label"); + // expect(metaData.description).toBe("Some description"); + // expect(data.taskType).toBe(TaskTypes.CUSTOM_TASK); + // expect(data.type).toBe("pluginA"); + // expect(data.parameters.intParam).toEqual("100"); + // expect(data.parameters.booleanParam).toEqual("false"); + // expect(data.parameters.objectParameter.subStringParam).toEqual("Something"); + // expect(data.parameters.stringParam).toEqual("default string"); + // // Test redirection to task details page + // const newTaskId = "newTaskId"; + // mockAxiosResponse(tasksUri, { data: { id: newTaskId } }); + // await waitFor(() => { + // expect(history.location.pathname).toEqual( + // expect.stringMatching(new RegExp(`projects/${PROJECT_ID}/task/${newTaskId}$`)) + // ); + // }); + // }); it("should show an error message if task creation failed in the backend", async () => { const { wrapper } = await pluginCreationDialogWrapper(); From 7cb5f1a9f45329b467e7081321aab12d9384a5d2 Mon Sep 17 00:00:00 2001 From: Robert Isele Date: Tue, 11 Apr 2023 15:01:23 +0200 Subject: [PATCH 3/9] Fixed SearchApiIntegrationTest --- .../test/controllers/workspace/SearchApiIntegrationTest.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/silk-workbench/silk-workbench-workspace/test/controllers/workspace/SearchApiIntegrationTest.scala b/silk-workbench/silk-workbench-workspace/test/controllers/workspace/SearchApiIntegrationTest.scala index aa2179621e..900178ac8a 100644 --- a/silk-workbench/silk-workbench-workspace/test/controllers/workspace/SearchApiIntegrationTest.scala +++ b/silk-workbench/silk-workbench-workspace/test/controllers/workspace/SearchApiIntegrationTest.scala @@ -78,7 +78,7 @@ class SearchApiIntegrationTest extends FlatSpec typeIds mustBe Seq("workflow", "dataset", "transform", "linking", "task") } private def resultAsMap(searchResultObject: JsObject): Map[String, String] = searchResultObject.value - .filter(v => v._1 != "itemLinks" && v._1 != "tags") // Filter out item links and tags, since they are no string + .filter(v => v._2.isInstanceOf[JsString]) // Filter out non-string values .mapValues(_.as[String]).toMap it should "return all tasks (pages) for a unrestricted search" in { From 19238bef9b88ac026b722dac7adf5a1d31940871 Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Thu, 27 Apr 2023 14:58:32 +0200 Subject: [PATCH 4/9] Deal with boolean parameter values in the task config --- workspace/src/app/store/ducks/common/typings.ts | 2 +- workspace/src/app/store/ducks/shared/typings.ts | 4 ++-- .../app/views/shared/TaskConfig/TaskConfigPreview.tsx | 10 +++++++--- .../CreateArtefactModal/ArtefactForms/TaskForm.tsx | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/workspace/src/app/store/ducks/common/typings.ts b/workspace/src/app/store/ducks/common/typings.ts index 07d960468f..0c00fd86b6 100644 --- a/workspace/src/app/store/ducks/common/typings.ts +++ b/workspace/src/app/store/ducks/common/typings.ts @@ -121,7 +121,7 @@ export interface IProjectTaskUpdatePayload { taskPluginDetails: IPluginDetails; metaData: IMetadata; currentParameterValues: { - [key: string]: string | object; + [key: string]: string | boolean | object; }; currentTemplateValues: TemplateValueType; dataParameters?: { diff --git a/workspace/src/app/store/ducks/shared/typings.ts b/workspace/src/app/store/ducks/shared/typings.ts index c3b2db80fc..b1b9c67191 100644 --- a/workspace/src/app/store/ducks/shared/typings.ts +++ b/workspace/src/app/store/ducks/shared/typings.ts @@ -53,8 +53,8 @@ export type RuleOperatorType = "AggregationOperator" | "TransformOperator" | "Co export type PluginType = TaskType | RuleOperatorType; export interface IArbitraryPluginParameters { - // If requested with withLabels option, then the values will be reified like this: {label: string, value: string | object} - [key: string]: string | object; + // If requested with withLabels option, then the values will be reified like this: {label: string, value: string | boolean | object} + [key: string]: string | boolean | object; } /** The data of a project task from the generic /tasks endpoint. */ diff --git a/workspace/src/app/views/shared/TaskConfig/TaskConfigPreview.tsx b/workspace/src/app/views/shared/TaskConfig/TaskConfigPreview.tsx index cb0e915022..5804c49945 100644 --- a/workspace/src/app/views/shared/TaskConfig/TaskConfigPreview.tsx +++ b/workspace/src/app/views/shared/TaskConfig/TaskConfigPreview.tsx @@ -88,11 +88,15 @@ export function TaskConfigPreview({ taskData, taskDescription }: IProps) { /** Returns the string value if this is an atomic value, else it returns the parameter value object. */ const paramDisplayValue = (parameterValue: any): string | any => { if (typeof parameterValue === "string") { - return t("widget.ConfigWidget.values." + parameterValue, parameterValue); + return parameterValue; + } else if (typeof parameterValue === "boolean") { + return `${parameterValue}`; } else if (typeof parameterValue.label === "string") { - return t("widget.ConfigWidget.values." + parameterValue.label, parameterValue.label); + return parameterValue.label; } else if (typeof parameterValue.value === "string") { - return t("widget.ConfigWidget.values." + parameterValue.value, parameterValue.value); + return parameterValue.value; + } else if (typeof parameterValue.value === "boolean") { + return `${parameterValue.value}`; } else if (parameterValue.value) { // withLabels "object" value return parameterValue.value; diff --git a/workspace/src/app/views/shared/modals/CreateArtefactModal/ArtefactForms/TaskForm.tsx b/workspace/src/app/views/shared/modals/CreateArtefactModal/ArtefactForms/TaskForm.tsx index a00cb22cc1..43574bdbee 100644 --- a/workspace/src/app/views/shared/modals/CreateArtefactModal/ArtefactForms/TaskForm.tsx +++ b/workspace/src/app/views/shared/modals/CreateArtefactModal/ArtefactForms/TaskForm.tsx @@ -39,7 +39,7 @@ export interface IProps { export interface UpdateTaskProps { // The existing parameter values parameterValues: { - [key: string]: string | object; + [key: string]: string | boolean | object; }; variableTemplateValues: Record; dataParameters?: { From e86c44739339633bc7f1ff7250607ba10457f2b2 Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Thu, 27 Apr 2023 15:36:47 +0200 Subject: [PATCH 5/9] Fix stringValueAsJs --- workspace/src/app/utils/transformers.ts | 12 +++- .../CreateArtefactModal.test.tsx | 65 +++++++++---------- 2 files changed, 42 insertions(+), 35 deletions(-) diff --git a/workspace/src/app/utils/transformers.ts b/workspace/src/app/utils/transformers.ts index c5ae89f19a..35c94f0e97 100644 --- a/workspace/src/app/utils/transformers.ts +++ b/workspace/src/app/utils/transformers.ts @@ -13,13 +13,21 @@ export const defaultValueAsJs = (property: IArtefactItemProperty, withLabel: boo return withLabel ? { value, label: optionallyLabelledParameterToLabel(property.value) } : value; }; /** Converts a string value to its typed equivalent based on the given value type. */ -export const stringValueAsJs = (valueType: string, value: OptionallyLabelledParameter | null): any => { +export const stringValueAsJs = ( + valueType: string, + value: OptionallyLabelledParameter | null +): any => { const stringValue = value != null ? optionallyLabelledParameterToValue(value) ?? "" : ""; let v: any = stringValue; + if (valueType === INPUT_TYPES.BOOLEAN && typeof v === "string") { + // cast to boolean from string + v = (stringValue as string).toLowerCase() === "true"; + } + if (valueType === INPUT_TYPES.INTEGER) { if (v !== "" && stringValue) { - v = parseInt(stringValue); + v = parseInt(stringValue as string); } else { v = null; } diff --git a/workspace/test/integration/components/CreateArtefactModal/CreateArtefactModal.test.tsx b/workspace/test/integration/components/CreateArtefactModal/CreateArtefactModal.test.tsx index 15d859bd9c..19ba5953f8 100644 --- a/workspace/test/integration/components/CreateArtefactModal/CreateArtefactModal.test.tsx +++ b/workspace/test/integration/components/CreateArtefactModal/CreateArtefactModal.test.tsx @@ -269,37 +269,36 @@ describe("Task creation widget", () => { await expectValidationErrors(wrapper, 6); }); - // TODO enable after fix - // it("should send the correct request when clicking 'Create' on a valid form", async () => { - // const { wrapper, history } = await pluginCreationDialogWrapper(); - // changeValue(findSingleElement(wrapper, "#intParam"), "100"); - // changeValue(findSingleElement(wrapper, "#label"), "Some label"); - // changeValue(findSingleElement(wrapper, "#description"), "Some description"); - // changeValue(findSingleElement(wrapper, byName("objectParameter.subStringParam")), "Something"); - // clickCreate(wrapper); - // await expectValidationErrors(wrapper, 0); - // const tasksUri = legacyApiUrl("workspace/projects/projectId/tasks"); - // const request = mockAxios.getReqByUrl(tasksUri); - // expect(request).toBeTruthy(); - // const metaData = request.data.metadata; - // const data = request.data.data; - // expect(metaData.label).toBe("Some label"); - // expect(metaData.description).toBe("Some description"); - // expect(data.taskType).toBe(TaskTypes.CUSTOM_TASK); - // expect(data.type).toBe("pluginA"); - // expect(data.parameters.intParam).toEqual("100"); - // expect(data.parameters.booleanParam).toEqual("false"); - // expect(data.parameters.objectParameter.subStringParam).toEqual("Something"); - // expect(data.parameters.stringParam).toEqual("default string"); - // // Test redirection to task details page - // const newTaskId = "newTaskId"; - // mockAxiosResponse(tasksUri, { data: { id: newTaskId } }); - // await waitFor(() => { - // expect(history.location.pathname).toEqual( - // expect.stringMatching(new RegExp(`projects/${PROJECT_ID}/task/${newTaskId}$`)) - // ); - // }); - // }); + it("should send the correct request when clicking 'Create' on a valid form", async () => { + const { wrapper, history } = await pluginCreationDialogWrapper(); + changeValue(findSingleElement(wrapper, "#intParam"), "100"); + changeValue(findSingleElement(wrapper, "#label"), "Some label"); + changeValue(findSingleElement(wrapper, "#description"), "Some description"); + changeValue(findSingleElement(wrapper, byName("objectParameter.subStringParam")), "Something"); + clickCreate(wrapper); + await expectValidationErrors(wrapper, 0); + const tasksUri = legacyApiUrl("workspace/projects/projectId/tasks"); + const request = mockAxios.getReqByUrl(tasksUri); + expect(request).toBeTruthy(); + const metaData = request.data.metadata; + const data = request.data.data; + expect(metaData.label).toBe("Some label"); + expect(metaData.description).toBe("Some description"); + expect(data.taskType).toBe(TaskTypes.CUSTOM_TASK); + expect(data.type).toBe("pluginA"); + expect(data.parameters.intParam).toEqual("100"); + expect(data.parameters.booleanParam).toEqual("false"); + expect(data.parameters.objectParameter.subStringParam).toEqual("Something"); + expect(data.parameters.stringParam).toEqual("default string"); + // Test redirection to task details page + const newTaskId = "newTaskId"; + mockAxiosResponse(tasksUri, { data: { id: newTaskId } }); + await waitFor(() => { + expect(history.location.pathname).toEqual( + expect.stringMatching(new RegExp(`projects/${PROJECT_ID}/task/${newTaskId}$`)) + ); + }); + }); it("should show an error message if task creation failed in the backend", async () => { const { wrapper } = await pluginCreationDialogWrapper(); @@ -373,8 +372,8 @@ describe("Task creation widget", () => { // }) }); - const value = (value: string, label?: string) => { - const result: { value: string; label?: string } = { value }; + const value = (value: string | boolean, label?: string) => { + const result: { value: string | boolean; label?: string } = { value }; if (label) { result.label = label; } From 7cfa4f4d7094f4a1a72215889d80c77d03d27b67 Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Thu, 27 Apr 2023 15:42:09 +0200 Subject: [PATCH 6/9] Test that search API returns JSON booleans for boolean parameters --- .../test/controllers/workspace/SearchApiIntegrationTest.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/silk-workbench/silk-workbench-workspace/test/controllers/workspace/SearchApiIntegrationTest.scala b/silk-workbench/silk-workbench-workspace/test/controllers/workspace/SearchApiIntegrationTest.scala index 900178ac8a..825426295d 100644 --- a/silk-workbench/silk-workbench-workspace/test/controllers/workspace/SearchApiIntegrationTest.scala +++ b/silk-workbench/silk-workbench-workspace/test/controllers/workspace/SearchApiIntegrationTest.scala @@ -333,6 +333,7 @@ class SearchApiIntegrationTest extends FlatSpec val parameters = (resultWithParameters.head \ PARAMETERS).asOpt[JsObject] parameters mustBe defined (parameters.get \ "file").as[String] mustBe "xyz.json" + (parameters.get \ "streaming").as[Boolean] mustBe true } private def resourceNames(defaultResults: IndexedSeq[collection.Map[String, JsValue]]) = { From 409b30e05511fa5d3e5da02d2d9d66296b79d7d6 Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Thu, 27 Apr 2023 15:58:00 +0200 Subject: [PATCH 7/9] Set initial values for uriProperty and readOnly parameters correctly on update --- .../modals/CreateArtefactModal/ArtefactForms/TaskForm.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/workspace/src/app/views/shared/modals/CreateArtefactModal/ArtefactForms/TaskForm.tsx b/workspace/src/app/views/shared/modals/CreateArtefactModal/ArtefactForms/TaskForm.tsx index 43574bdbee..ccbad53ff7 100644 --- a/workspace/src/app/views/shared/modals/CreateArtefactModal/ArtefactForms/TaskForm.tsx +++ b/workspace/src/app/views/shared/modals/CreateArtefactModal/ArtefactForms/TaskForm.tsx @@ -224,7 +224,9 @@ export function TaskForm({ form, projectId, artefact, updateTask, taskId, detect } if (artefact.taskType === "Dataset") { register({ name: URI_PROPERTY_PARAMETER_ID }); + setValue(URI_PROPERTY_PARAMETER_ID, updateTask?.dataParameters?.uriProperty ?? ""); register({ name: READ_ONLY_PARAMETER }); + setValue(READ_ONLY_PARAMETER, updateTask?.dataParameters?.readOnly ?? false); } registerParameters("", visibleParams, updateTask ? updateTask.parameterValues : {}, requiredRootParameters); setFormValueKeys(returnKeys); From edba6ac1d062a46ab6bcb504a246aab3898e41c1 Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Wed, 3 May 2023 17:07:50 +0200 Subject: [PATCH 8/9] Fix EmptyResourceManager issues in JSON serialization --- .../silkframework/serialization/json/JsonSerializers.scala | 7 ++++++- .../app/controllers/transform/TransformTaskApi.scala | 6 ++++-- .../test/controllers/transform/AutoCompletionApiTest.scala | 2 +- .../controllers/transform/TargetVocabularyApiTest.scala | 2 +- .../app/controllers/workspace/JsonSerializer.scala | 2 +- 5 files changed, 13 insertions(+), 6 deletions(-) diff --git a/silk-plugins/silk-serialization-json/src/main/scala/org/silkframework/serialization/json/JsonSerializers.scala b/silk-plugins/silk-serialization-json/src/main/scala/org/silkframework/serialization/json/JsonSerializers.scala index 9cb92135ac..6d697c1a7b 100644 --- a/silk-plugins/silk-serialization-json/src/main/scala/org/silkframework/serialization/json/JsonSerializers.scala +++ b/silk-plugins/silk-serialization-json/src/main/scala/org/silkframework/serialization/json/JsonSerializers.scala @@ -1369,7 +1369,12 @@ object JsonSerializers { } } - def toJson[T](value: T)(implicit format: JsonFormat[T], writeContext: WriteContext[JsValue] = WriteContext.empty[JsValue]): JsValue = { + def toJson[T](value: T)(implicit format: JsonFormat[T], writeContext: WriteContext[JsValue]): JsValue = { + format.write(value) + } + + def toJsonEmptyContext[T](value: T)(implicit format: JsonFormat[T]): JsValue = { + implicit val emptyWriteContext: WriteContext[JsValue] = WriteContext.empty[JsValue] format.write(value) } diff --git a/silk-workbench/silk-workbench-rules/app/controllers/transform/TransformTaskApi.scala b/silk-workbench/silk-workbench-rules/app/controllers/transform/TransformTaskApi.scala index 5e70d6be71..f77443cb72 100644 --- a/silk-workbench/silk-workbench-rules/app/controllers/transform/TransformTaskApi.scala +++ b/silk-workbench/silk-workbench-rules/app/controllers/transform/TransformTaskApi.scala @@ -20,7 +20,7 @@ import org.silkframework.rule.execution.ExecuteTransform import org.silkframework.rule.util.UriPatternParser.UriPatternParserException import org.silkframework.runtime.activity.{Activity, UserContext} import org.silkframework.runtime.resource.ResourceManager -import org.silkframework.runtime.serialization.ReadContext +import org.silkframework.runtime.serialization.{ReadContext, WriteContext} import org.silkframework.runtime.validation.{BadUserInputException, NotFoundException, ValidationError, ValidationException} import org.silkframework.serialization.json.JsonParseException import org.silkframework.serialization.json.JsonSerializers._ @@ -425,6 +425,7 @@ class TransformTaskApi @Inject() () extends InjectedController with UserContextA task.synchronized { processRule(task, ruleId) { currentRule => handleValidationExceptions { + implicit val writeContext: WriteContext[JsValue] = WriteContext.fromProject[JsValue](project).copy(prefixes = Prefixes.empty) implicit val updatedRequest: Request[AnyContent] = updateJsonRequest(request, currentRule) deserializeCompileTime[TransformRule]() { updatedRule => updateRule(currentRule.update(updatedRule)) @@ -869,7 +870,8 @@ class TransformTaskApi @Inject() () extends InjectedController with UserContextA TransformSpec.identifierGenerator(transformTask.data) } - private def updateJsonRequest(request: Request[AnyContent], rule: RuleTraverser): Request[AnyContent] = { + private def updateJsonRequest(request: Request[AnyContent], rule: RuleTraverser) + (implicit writeContext: WriteContext[JsValue]): Request[AnyContent] = { request.body.asJson match { case Some(requestJson) => val ruleJson = toJson(rule.operator.asInstanceOf[TransformRule]).as[JsObject] diff --git a/silk-workbench/silk-workbench-rules/test/controllers/transform/AutoCompletionApiTest.scala b/silk-workbench/silk-workbench-rules/test/controllers/transform/AutoCompletionApiTest.scala index ff30e8cdc1..cb944feb84 100644 --- a/silk-workbench/silk-workbench-rules/test/controllers/transform/AutoCompletionApiTest.scala +++ b/silk-workbench/silk-workbench-rules/test/controllers/transform/AutoCompletionApiTest.scala @@ -113,7 +113,7 @@ class AutoCompletionApiTest extends TransformTaskApiTestBase { ) val transformTask = PlainTask(task, transformSpec) val request = client.url(s"$baseUrl/transform/tasks/$project/$task") - val response = request.put(toJson[TransformTask](transformTask)) + val response = request.put(toJsonEmptyContext[TransformTask](transformTask)) checkResponse(response) waitForCaches(task) diff --git a/silk-workbench/silk-workbench-rules/test/controllers/transform/TargetVocabularyApiTest.scala b/silk-workbench/silk-workbench-rules/test/controllers/transform/TargetVocabularyApiTest.scala index ebf22250d1..438c9f6121 100644 --- a/silk-workbench/silk-workbench-rules/test/controllers/transform/TargetVocabularyApiTest.scala +++ b/silk-workbench/silk-workbench-rules/test/controllers/transform/TargetVocabularyApiTest.scala @@ -109,7 +109,7 @@ class TargetVocabularyApiTest extends TransformTaskApiTestBase { ) val transformTask = PlainTask(task, transformSpec) val request = client.url(s"$baseUrl/transform/tasks/$project/$task") - val response = request.put(toJson[TransformTask](transformTask)) + val response = request.put(toJsonEmptyContext[TransformTask](transformTask)) checkResponse(response) waitForCaches(task) diff --git a/silk-workbench/silk-workbench-workspace/app/controllers/workspace/JsonSerializer.scala b/silk-workbench/silk-workbench-workspace/app/controllers/workspace/JsonSerializer.scala index 0587994855..da41e2b184 100644 --- a/silk-workbench/silk-workbench-workspace/app/controllers/workspace/JsonSerializer.scala +++ b/silk-workbench/silk-workbench-workspace/app/controllers/workspace/JsonSerializer.scala @@ -36,7 +36,7 @@ object JsonSerializer { (implicit userContext: UserContext): JsObject = { Json.obj( "name" -> JsString(project.id), - "metaData" -> JsonSerializers.toJson(project.config.metaData), + "metaData" -> JsonSerializers.toJsonEmptyContext(project.config.metaData), "tasks" -> Json.obj( "dataset" -> tasksJson[GenericDatasetSpec](project), "transform" -> tasksJson[TransformSpec](project), From 1fa545f7f7da1c1287239199035317daf12635b1 Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Wed, 3 May 2023 17:43:35 +0200 Subject: [PATCH 9/9] Fix handling of boolean rule operator parameters in rule editors --- .../RuleEditor/model/RuleEditorModel.tsx | 9 +++- .../model/RuleEditorModel.typings.ts | 10 ++-- .../RuleEditor/view/ruleNode/NodeContent.tsx | 4 +- .../view/ruleNode/RuleParameterInput.tsx | 54 +++++++++++++------ .../linking/LinkingRuleEditor.utils.ts | 23 ++++---- .../taskViews/shared/rules/rule.typings.ts | 4 +- .../taskViews/shared/rules/rule.utils.tsx | 9 ++-- 7 files changed, 73 insertions(+), 40 deletions(-) diff --git a/workspace/src/app/views/shared/RuleEditor/model/RuleEditorModel.tsx b/workspace/src/app/views/shared/RuleEditor/model/RuleEditorModel.tsx index a534d6b6a3..98b19abb42 100644 --- a/workspace/src/app/views/shared/RuleEditor/model/RuleEditorModel.tsx +++ b/workspace/src/app/views/shared/RuleEditor/model/RuleEditorModel.tsx @@ -1539,7 +1539,14 @@ export const RuleEditorModel = ({ children }: RuleEditorModelProps) => { parameterDiff && parameterDiff.has(parameterId) ? parameterDiff.get(parameterId) : parameterValue; - return [parameterId, typeof value === "string" ? value : value ? value.value : undefined]; + return [ + parameterId, + typeof value === "string" || typeof value === "boolean" + ? value + : value + ? value.value + : undefined, + ]; }) ) : originalNode.parameters, diff --git a/workspace/src/app/views/shared/RuleEditor/model/RuleEditorModel.typings.ts b/workspace/src/app/views/shared/RuleEditor/model/RuleEditorModel.typings.ts index 0b0c5b54d1..e1bfebe1f1 100644 --- a/workspace/src/app/views/shared/RuleEditor/model/RuleEditorModel.typings.ts +++ b/workspace/src/app/views/shared/RuleEditor/model/RuleEditorModel.typings.ts @@ -13,12 +13,14 @@ export interface RuleEditorNode extends Node; } -export type RuleEditorNodeParameterValue = IOperatorNodeParameterValueWithLabel | string | undefined; -export const ruleEditorNodeParameterValue = (value: RuleEditorNodeParameterValue): string | undefined => { - return typeof value === "string" ? value : value?.value; +export type RuleEditorNodeParameterValue = IOperatorNodeParameterValueWithLabel | string | boolean | undefined; +export const ruleEditorNodeParameterValue = (value: RuleEditorNodeParameterValue): string | boolean | undefined => { + return typeof value === "string" || typeof value === "boolean" ? value : value?.value; }; export const ruleEditorNodeParameterLabel = (value: RuleEditorNodeParameterValue): string | undefined => { - return typeof value === "string" ? value : value?.label ?? value?.value; + return typeof value === "string" || typeof value === "boolean" + ? `${value}` + : value?.label ?? (value?.value != null ? `${value.value}` : undefined); }; export type StickyNodePropType = { content?: string; style?: CSSProperties }; diff --git a/workspace/src/app/views/shared/RuleEditor/view/ruleNode/NodeContent.tsx b/workspace/src/app/views/shared/RuleEditor/view/ruleNode/NodeContent.tsx index 5248067a70..11b69fe650 100644 --- a/workspace/src/app/views/shared/RuleEditor/view/ruleNode/NodeContent.tsx +++ b/workspace/src/app/views/shared/RuleEditor/view/ruleNode/NodeContent.tsx @@ -81,9 +81,9 @@ export const NodeContent = ({ const dependentValue = (paramId: string): string | undefined => { const value = operatorContext.currentValue(nodeId, paramId); if ((value as IOperatorNodeParameterValueWithLabel).value != null) { - return (value as IOperatorNodeParameterValueWithLabel).value; + return `${(value as IOperatorNodeParameterValueWithLabel).value}`; } else { - return value as string | undefined; + return value != null ? `${value}` : undefined; } }; return rerender ? null : ( diff --git a/workspace/src/app/views/shared/RuleEditor/view/ruleNode/RuleParameterInput.tsx b/workspace/src/app/views/shared/RuleEditor/view/ruleNode/RuleParameterInput.tsx index 9a7192394c..b0ff5b9ed0 100644 --- a/workspace/src/app/views/shared/RuleEditor/view/ruleNode/RuleParameterInput.tsx +++ b/workspace/src/app/views/shared/RuleEditor/view/ruleNode/RuleParameterInput.tsx @@ -50,14 +50,30 @@ export const RuleParameterInput = ({ const uniqueId = `${nodeId}_${ruleParameter.parameterId}`; const defaultValueWithLabel = ruleParameter.currentValue() ?? ruleParameter.initialValue; const defaultValue = ruleEditorNodeParameterValue(defaultValueWithLabel); - const inputAttributes = { + const _inputAttributes = { id: uniqueId, name: uniqueId, - defaultValue: defaultValue, onChange, intent: hasValidationError ? Intent.DANGER : undefined, readOnly: ruleEditorContext.readOnlyMode || modelContext.readOnly, }; + const stringDefaultValue = + typeof defaultValue === "string" ? defaultValue : defaultValue != null ? `${defaultValue}` : undefined; + const stringValueInputAttributes = { + ..._inputAttributes, + defaultValue: stringDefaultValue, + }; + const booleanDefaultValue = + typeof defaultValue === "boolean" + ? defaultValue + : typeof defaultValue === "string" + ? defaultValue.toLowerCase() === "true" + : undefined; + const booleanValueInputAttributes = { + ..._inputAttributes, + defaultValue: booleanDefaultValue ? "true" : "false", + defaultChecked: booleanDefaultValue, + }; const handleFileSearch = async (input: string) => { try { @@ -77,11 +93,13 @@ export const RuleParameterInput = ({ if (large) { // FIXME: CodeEditor looks buggy in the modal // return ; - return ; + return ( + + ); } else { return ( inputAttributes.onChange(`${value}`)} - defaultChecked={inputAttributes.defaultValue?.toLowerCase() === "true"} - disabled={inputAttributes.readOnly} + {...booleanValueInputAttributes} + onChange={(value: boolean) => booleanValueInputAttributes.onChange(`${value}`)} + disabled={booleanValueInputAttributes.readOnly} /> ); case "code": // FIXME: Add readOnly mode - return ; + return ; case "password": return ( ) => { onChange(e.target.value); @@ -125,11 +142,11 @@ export const RuleParameterInput = ({ itemValueString: resourceNameFn, noResultText: t("common.messages.noResults", "No results."), inputProps: { - readOnly: inputAttributes.readOnly, + readOnly: stringValueInputAttributes.readOnly, }, }} required={ruleParameter.parameterSpecification.required} - {...inputAttributes} + {...stringValueInputAttributes} insideModal={insideModal} /> ); @@ -143,11 +160,11 @@ export const RuleParameterInput = ({ projectId={ruleEditorContext.projectId} paramId={ruleParameter.parameterId} pluginId={pluginId} - onChange={inputAttributes.onChange} + onChange={stringValueInputAttributes.onChange} initialValue={ - defaultValue + stringDefaultValue ? { - value: defaultValue, + value: stringDefaultValue, label: (defaultValueWithLabel as IOperatorNodeParameterValueWithLabel)?.label, } : undefined @@ -157,13 +174,16 @@ export const RuleParameterInput = ({ formParamId={uniqueId} dependentValue={dependentValue} required={ruleParameter.parameterSpecification.required} - readOnly={inputAttributes.readOnly} + readOnly={stringValueInputAttributes.readOnly} hasBackDrop={!insideModal} /> ); } else { return ( - + ); } } diff --git a/workspace/src/app/views/taskViews/linking/LinkingRuleEditor.utils.ts b/workspace/src/app/views/taskViews/linking/LinkingRuleEditor.utils.ts index 2f263108cb..5226256c6a 100644 --- a/workspace/src/app/views/taskViews/linking/LinkingRuleEditor.utils.ts +++ b/workspace/src/app/views/taskViews/linking/LinkingRuleEditor.utils.ts @@ -70,8 +70,11 @@ const extractSimilarityOperatorNode = ( } return t; }; - const reverseParameterValue = () => - operator.parameters[REVERSE_PARAMETER_ID]?.["value"] ?? operator.parameters[REVERSE_PARAMETER_ID]; + const reverseParameterValue = (): string | undefined => { + const value = + operator.parameters[REVERSE_PARAMETER_ID]?.["value"] ?? operator.parameters[REVERSE_PARAMETER_ID]; + return typeof value == "string" ? value : typeof value == "boolean" ? `${value}` : undefined; + }; const inputsCanBeSwitched = isComparison && reverseParameterValue() != null; const switchInputs = inputsCanBeSwitched && reverseParameterValue() === "true"; if (switchInputs) { @@ -185,14 +188,16 @@ const convertRuleOperatorNodeToSimilarityOperator = ( id: ruleOperatorNode.nodeId, indexing: true, // FIXME: Should this be part of the config in the UI? parameters: Object.fromEntries( - Object.entries(ruleOperatorNode.parameters).map(([parameterKey, parameterValue]) => [ - parameterKey, - parameterValue ?? "", - ]) + Object.entries(ruleOperatorNode.parameters) + // Do not send values that were not set, these must have a default value defined. + .filter(([_key, value]) => value != null) + .map(([parameterKey, parameterValue]) => [parameterKey, parameterValue!]) ), type: "Comparison", - threshold: parseFloat(ruleEditorNodeParameterValue(ruleOperatorNode.parameters["threshold"])!!), - weight: parseInt(ruleEditorNodeParameterValue(ruleOperatorNode.parameters["weight"])!!), + threshold: parseFloat( + ruleEditorNodeParameterValue(ruleOperatorNode.parameters["threshold"])!! as string + ), + weight: parseInt(ruleEditorNodeParameterValue(ruleOperatorNode.parameters["weight"])!! as string), }; if (ruleOperatorNode.inputsCanBeSwitched && ruleOperatorNode.inputs[0] != null) { // Set reverse parameter correctly. Either the first input has a target path or the second input has a source path. @@ -232,7 +237,7 @@ const convertRuleOperatorNodeToSimilarityOperator = ( ]) ), type: "Aggregation", - weight: parseInt(ruleEditorNodeParameterValue(ruleOperatorNode.parameters["weight"])!!), + weight: parseInt(ruleEditorNodeParameterValue(ruleOperatorNode.parameters["weight"])!! as string), }; return aggregation; } diff --git a/workspace/src/app/views/taskViews/shared/rules/rule.typings.ts b/workspace/src/app/views/taskViews/shared/rules/rule.typings.ts index ba2d450e47..dda2c7e944 100644 --- a/workspace/src/app/views/taskViews/shared/rules/rule.typings.ts +++ b/workspace/src/app/views/taskViews/shared/rules/rule.typings.ts @@ -30,13 +30,13 @@ export interface ITransformOperator extends IValueInput { /** Labelled value. In order to show human-readable versions of values, e.g. in auto-completion. */ export interface IOperatorNodeParameterValueWithLabel { - value: string; + value: string | boolean; label?: string; } /** Parameters of a rule operator. */ export interface IOperatorNodeParameters { - [key: string]: string | IOperatorNodeParameterValueWithLabel; + [key: string]: string | boolean | IOperatorNodeParameterValueWithLabel; } /** Rule layout information. */ diff --git a/workspace/src/app/views/taskViews/shared/rules/rule.utils.tsx b/workspace/src/app/views/taskViews/shared/rules/rule.utils.tsx index 006ea7086e..6637e14279 100644 --- a/workspace/src/app/views/taskViews/shared/rules/rule.utils.tsx +++ b/workspace/src/app/views/taskViews/shared/rules/rule.utils.tsx @@ -332,10 +332,9 @@ const convertRuleOperatorNodeToValueInput = ( ) ), parameters: Object.fromEntries( - Object.entries(ruleOperatorNode.parameters).map(([parameterKey, parameterValue]) => [ - parameterKey, - parameterValue ?? "", - ]) + Object.entries(ruleOperatorNode.parameters) + .filter(([_key, value]) => value != null) + .map(([parameterKey, parameterValue]) => [parameterKey, parameterValue!]) ), type: "transformInput", }; @@ -343,7 +342,7 @@ const convertRuleOperatorNodeToValueInput = ( } else if (ruleOperatorNode.pluginType === "PathInputOperator") { const pathInput: IPathInput = { id: ruleOperatorNode.nodeId, - path: ruleEditorNodeParameterValue(ruleOperatorNode.parameters["path"]) ?? "", + path: (ruleEditorNodeParameterValue(ruleOperatorNode.parameters["path"]) ?? "") as string, type: "pathInput", }; return pathInput;