From ad00f0d179b7cdce3f231251b3fc4d9f445bed6e Mon Sep 17 00:00:00 2001 From: Maciej Cichanowicz Date: Tue, 17 Dec 2024 14:02:10 +0100 Subject: [PATCH 1/6] Reduce reponse payload for scenarios tab --- .../ui/api/ProcessesResources.scala | 3 ++- .../ui/process/ProcessService.scala | 22 ++++++++++++++++- .../ScenarioWithDetailsConversions.scala | 21 ++++++++++++++++ .../ui/api/ProcessesResourcesSpec.scala | 24 +++++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ProcessesResources.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ProcessesResources.scala index 9b216dafb8e..62aaf0f0941 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ProcessesResources.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ProcessesResources.scala @@ -17,6 +17,7 @@ import pl.touk.nussknacker.ui.process.ProcessService.{ CreateScenarioCommand, FetchScenarioGraph, GetScenarioWithDetailsOptions, + SkipAdditionalFields, UpdateScenarioCommand } import pl.touk.nussknacker.ui.process.ScenarioWithDetailsConversions._ @@ -81,7 +82,7 @@ class ProcessesResources( complete { processService.getLatestProcessesWithDetails( query, - GetScenarioWithDetailsOptions.detailsOnly.withFetchState + GetScenarioWithDetailsOptions.withoutAdditionalFields.withFetchState ) } } diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ProcessService.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ProcessService.scala index 8e5ae07a82d..a1658608b93 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ProcessService.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ProcessService.scala @@ -75,9 +75,17 @@ object ProcessService { val withsScenarioGraph: GetScenarioWithDetailsOptions = new GetScenarioWithDetailsOptions(FetchScenarioGraph(), fetchState = false) + + val withoutAdditionalFields: GetScenarioWithDetailsOptions = + detailsOnly.copy(additionalFieldsOptions = SkipAdditionalFields(skipProcessActionOptionalFields = true)) + } - final case class GetScenarioWithDetailsOptions(fetchGraphOptions: ScenarioGraphOptions, fetchState: Boolean) { + final case class GetScenarioWithDetailsOptions( + fetchGraphOptions: ScenarioGraphOptions, + fetchState: Boolean, + additionalFieldsOptions: AdditionalFieldsOptions = DoNotSkipAdditionalFields + ) { def withValidation: GetScenarioWithDetailsOptions = { val newFetchGraphOptions = fetchGraphOptions match { @@ -105,6 +113,12 @@ object ProcessService { case class ValidateAndResolve(includeValidationNodeResults: Boolean = true) extends ValidationMode } + sealed trait AdditionalFieldsOptions + + case object DoNotSkipAdditionalFields extends AdditionalFieldsOptions + + case class SkipAdditionalFields(skipProcessActionOptionalFields: Boolean) extends AdditionalFieldsOptions + } trait ProcessService { @@ -257,6 +271,12 @@ class DBProcessService( case FetchScenarioGraph(validate) => fetchScenario[CanonicalProcess] .map(_.map(validateAndReverseResolve(_, validate))) + }).map(_.map { details => + options.additionalFieldsOptions match { + case DoNotSkipAdditionalFields => details + case skipFieldsOption: SkipAdditionalFields => + ScenarioWithDetailsConversions.skipAdditionalFields(details, skipFieldsOption) + } }).flatMap { details => if (options.fetchState) processStateProvider.enrichDetailsWithProcessState(details) diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ScenarioWithDetailsConversions.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ScenarioWithDetailsConversions.scala index d0e327f8ca9..d05791ce086 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ScenarioWithDetailsConversions.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ScenarioWithDetailsConversions.scala @@ -1,9 +1,11 @@ package pl.touk.nussknacker.ui.process +import pl.touk.nussknacker.engine.api.deployment.ProcessAction import pl.touk.nussknacker.engine.api.graph.ScenarioGraph import pl.touk.nussknacker.engine.api.process.ProcessId import pl.touk.nussknacker.restmodel.scenariodetails.{ScenarioParameters, ScenarioWithDetails} import pl.touk.nussknacker.restmodel.validation.ScenarioGraphWithValidationResult +import pl.touk.nussknacker.ui.process.ProcessService.SkipAdditionalFields import pl.touk.nussknacker.ui.process.repository.ScenarioWithDetailsEntity object ScenarioWithDetailsConversions { @@ -55,6 +57,25 @@ object ScenarioWithDetailsConversions { ) } + def skipAdditionalFields(details: ScenarioWithDetails, skipOptions: SkipAdditionalFields): ScenarioWithDetails = { + def skipProcessActionOptionalFields(processAction: Option[ProcessAction]) = processAction.map( + _.copy( + failureMessage = None, + commentId = None, + comment = None, + buildInfo = Map.empty + ) + ) + def getProcessAction(processAction: Option[ProcessAction]) = + if (skipOptions.skipProcessActionOptionalFields) skipProcessActionOptionalFields(processAction) else processAction + + details.copy( + lastDeployedAction = getProcessAction(details.lastDeployedAction), + lastStateAction = getProcessAction(details.lastStateAction), + lastAction = getProcessAction(details.lastAction) + ) + } + implicit class Ops(scenarioWithDetails: ScenarioWithDetails) { // TODO: Instead of doing these conversions below, wee should pass around ScenarioWithDetails diff --git a/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ProcessesResourcesSpec.scala b/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ProcessesResourcesSpec.scala index 1689b0ece5f..c19505bf084 100644 --- a/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ProcessesResourcesSpec.scala +++ b/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ProcessesResourcesSpec.scala @@ -129,6 +129,30 @@ class ProcessesResourcesSpec } } + test("/api/processes should return lighter details without ProcessAction's additional fields") { + createDeployedWithCustomActionScenario(processName, category = Category1) + Get(s"/api/processes") ~> withReaderUser() ~> applicationRoute ~> check { + status shouldEqual StatusCodes.OK + val loadedProcess = responseAs[List[ScenarioWithDetails]] + + loadedProcess.head.lastAction should matchPattern { + case Some( + ProcessAction(_, _, _, _, _, _, _, _, None, None, None, buildInfo) + ) if buildInfo.isEmpty => + } + loadedProcess.head.lastStateAction should matchPattern { + case Some( + ProcessAction(_, _, _, _, _, _, _, _, None, None, None, buildInfo) + ) if buildInfo.isEmpty => + } + loadedProcess.head.lastDeployedAction should matchPattern { + case Some( + ProcessAction(_, _, _, _, _, _, _, _, None, None, None, buildInfo) + ) if buildInfo.isEmpty => + } + } + } + test("return single process") { createDeployedExampleScenario(processName, category = Category1) MockableDeploymentManager.configureScenarioStatuses( From 7f737d181ba8d97f3713991facc2292f43a2e204 Mon Sep 17 00:00:00 2001 From: Maciej Cichanowicz Date: Tue, 17 Dec 2024 14:03:07 +0100 Subject: [PATCH 2/6] fix typo --- .../scala/pl/touk/nussknacker/ui/api/AppApiHttpService.scala | 2 +- .../scala/pl/touk/nussknacker/ui/api/ProcessesResources.scala | 4 ++-- .../touk/nussknacker/ui/api/RemoteEnvironmentResources.scala | 4 ++-- .../scala/pl/touk/nussknacker/ui/process/ProcessService.scala | 2 +- .../ui/statistics/UsageStatisticsReportsSettingsService.scala | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/AppApiHttpService.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/AppApiHttpService.scala index 56465fc533e..77c0dff72dd 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/AppApiHttpService.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/AppApiHttpService.scala @@ -182,7 +182,7 @@ class AppApiHttpService( processService .getLatestProcessesWithDetails( ScenarioQuery.unarchivedProcesses, - GetScenarioWithDetailsOptions.withsScenarioGraph.withValidation + GetScenarioWithDetailsOptions.withScenarioGraph.withValidation ) .map { processes => processes diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ProcessesResources.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ProcessesResources.scala index 62aaf0f0941..6b4d05a74c5 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ProcessesResources.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ProcessesResources.scala @@ -233,12 +233,12 @@ class ProcessesResources( thisVersion <- processService.getProcessWithDetails( processId, thisVersion, - GetScenarioWithDetailsOptions.withsScenarioGraph + GetScenarioWithDetailsOptions.withScenarioGraph ) otherVersion <- processService.getProcessWithDetails( processId, otherVersion, - GetScenarioWithDetailsOptions.withsScenarioGraph + GetScenarioWithDetailsOptions.withScenarioGraph ) } yield ScenarioGraphComparator.compare(thisVersion.scenarioGraphUnsafe, otherVersion.scenarioGraphUnsafe) } diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/RemoteEnvironmentResources.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/RemoteEnvironmentResources.scala index a801d750773..a1a64a9ba93 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/RemoteEnvironmentResources.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/RemoteEnvironmentResources.scala @@ -51,7 +51,7 @@ class RemoteEnvironmentResources( for { processes <- processService.getLatestProcessesWithDetails( ScenarioQuery.unarchived, - GetScenarioWithDetailsOptions.withsScenarioGraph + GetScenarioWithDetailsOptions.withScenarioGraph ) comparison <- compareProcesses(processes) } yield NuDesignerErrorToHttp.toResponseEither(comparison) @@ -137,7 +137,7 @@ class RemoteEnvironmentResources( .getProcessWithDetails( processIdWithName, version, - GetScenarioWithDetailsOptions.withsScenarioGraph + GetScenarioWithDetailsOptions.withScenarioGraph ) .flatMap(fun) .map(NuDesignerErrorToHttp.toResponseEither[T]) diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ProcessService.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ProcessService.scala index a1658608b93..2f320bd8216 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ProcessService.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ProcessService.scala @@ -73,7 +73,7 @@ object ProcessService { val detailsOnly: GetScenarioWithDetailsOptions = new GetScenarioWithDetailsOptions(SkipScenarioGraph, fetchState = false) - val withsScenarioGraph: GetScenarioWithDetailsOptions = + val withScenarioGraph: GetScenarioWithDetailsOptions = new GetScenarioWithDetailsOptions(FetchScenarioGraph(), fetchState = false) val withoutAdditionalFields: GetScenarioWithDetailsOptions = diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/statistics/UsageStatisticsReportsSettingsService.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/statistics/UsageStatisticsReportsSettingsService.scala index 4988b353806..ee675e28a63 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/statistics/UsageStatisticsReportsSettingsService.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/statistics/UsageStatisticsReportsSettingsService.scala @@ -48,7 +48,7 @@ object UsageStatisticsReportsSettingsService extends LazyLogging { processService .getLatestProcessesWithDetails( ScenarioQuery.unarchived, - GetScenarioWithDetailsOptions.withsScenarioGraph.withFetchState + GetScenarioWithDetailsOptions.withScenarioGraph.withFetchState )(user) .map { scenariosDetails => Right( From cf6a0f2da31be5ee9f2a646704f295140e06ad9e Mon Sep 17 00:00:00 2001 From: Maciej Cichanowicz Date: Tue, 17 Dec 2024 16:17:09 +0100 Subject: [PATCH 3/6] changelog entry --- docs/Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/Changelog.md b/docs/Changelog.md index 97c5bc5e89f..7806c90a771 100644 --- a/docs/Changelog.md +++ b/docs/Changelog.md @@ -42,6 +42,7 @@ * [#7386](https://github.com/TouK/nussknacker/pull/7386) Improve Periodic DeploymentManager db queries, continuation of [#7323](https://github.com/TouK/nussknacker/pull/7323) * [#7360](https://github.com/TouK/nussknacker/pull/7360) Added Median aggregator * [#7388](https://github.com/TouK/nussknacker/pull/7388) Ability to configure a required permission for component links +* [#7354](https://github.com/TouK/nussknacker/pull/7354) Reduce response payload size when fetching scenarios for scenarios tab by removing unused fields and `null` attributes. ## 1.18 From 95331feae5cbfc58c64d147bbc117343f6e3d232 Mon Sep 17 00:00:00 2001 From: Maciej Cichanowicz Date: Thu, 2 Jan 2025 14:33:49 +0100 Subject: [PATCH 4/6] Remove null attributes from response --- .../scenariodetails/ScenarioWithDetails.scala | 10 ++++++++-- .../nussknacker/ui/api/ProcessesResourcesSpec.scala | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/designer/restmodel/src/main/scala/pl/touk/nussknacker/restmodel/scenariodetails/ScenarioWithDetails.scala b/designer/restmodel/src/main/scala/pl/touk/nussknacker/restmodel/scenariodetails/ScenarioWithDetails.scala index 570c29020b0..d884e8feac9 100644 --- a/designer/restmodel/src/main/scala/pl/touk/nussknacker/restmodel/scenariodetails/ScenarioWithDetails.scala +++ b/designer/restmodel/src/main/scala/pl/touk/nussknacker/restmodel/scenariodetails/ScenarioWithDetails.scala @@ -77,13 +77,19 @@ object ScenarioWithDetails { import io.circe.generic.semiauto._ + // TODO: We remove null values from json to make response payload lighter. + // The target solution would be to introduce pagination to the /api/processes endpoint. implicit val encoder: Encoder[ScenarioWithDetails] = - deriveEncoder[ScenarioWithDetails] - .contramap[ScenarioWithDetails](_.copy(processId = None)) + deepDropNulls( + deriveEncoder[ScenarioWithDetails] + .contramap[ScenarioWithDetails](_.copy(processId = None)) + ) implicit val decoder: Decoder[ScenarioWithDetails] = deriveDecoder[ScenarioWithDetails] + private def deepDropNulls[A](encoder: Encoder[A]): Encoder[A] = encoder.mapJson(_.deepDropNullValues) + } // This class is to enforce consistency of fields between CreateScenarioCommand and ScenarioWithDetails. diff --git a/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ProcessesResourcesSpec.scala b/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ProcessesResourcesSpec.scala index c19505bf084..349fd4fe18c 100644 --- a/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ProcessesResourcesSpec.scala +++ b/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ProcessesResourcesSpec.scala @@ -130,7 +130,7 @@ class ProcessesResourcesSpec } test("/api/processes should return lighter details without ProcessAction's additional fields") { - createDeployedWithCustomActionScenario(processName, category = Category1) + createDeployedScenario(processName, category = Category1) Get(s"/api/processes") ~> withReaderUser() ~> applicationRoute ~> check { status shouldEqual StatusCodes.OK val loadedProcess = responseAs[List[ScenarioWithDetails]] From 7f0981d6ef33a7b5008b85c29ae2a2176317857d Mon Sep 17 00:00:00 2001 From: Maciej Cichanowicz Date: Fri, 3 Jan 2025 10:12:54 +0100 Subject: [PATCH 5/6] Adjust FE --- .../fragment-input-definition/item/types.ts | 3 ++- .../settings/variants/fields/InputModeSelect.tsx | 12 ++++++------ designer/client/src/reducers/scenarioState.ts | 4 +++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/designer/client/src/components/graph/node-modal/fragment-input-definition/item/types.ts b/designer/client/src/components/graph/node-modal/fragment-input-definition/item/types.ts index 5efda562190..305c8507143 100644 --- a/designer/client/src/components/graph/node-modal/fragment-input-definition/item/types.ts +++ b/designer/client/src/components/graph/node-modal/fragment-input-definition/item/types.ts @@ -1,3 +1,4 @@ +import { isNil } from "lodash"; import { Expression, ReturnedType } from "../../../../../types"; import { resolveRefClazzName } from "./utils"; @@ -78,7 +79,7 @@ export function isAnyValueWithSuggestionsParameter(item: PermittedTypeParameterV } export function isAnyValueParameter(item: PermittedTypeParameterVariant): item is AnyValueParameterVariant { - return item.valueEditor === null; + return isNil(item.valueEditor); } const permittedTypesMapping = { diff --git a/designer/client/src/components/graph/node-modal/fragment-input-definition/settings/variants/fields/InputModeSelect.tsx b/designer/client/src/components/graph/node-modal/fragment-input-definition/settings/variants/fields/InputModeSelect.tsx index e568f6898dc..c5a232b35ae 100644 --- a/designer/client/src/components/graph/node-modal/fragment-input-definition/settings/variants/fields/InputModeSelect.tsx +++ b/designer/client/src/components/graph/node-modal/fragment-input-definition/settings/variants/fields/InputModeSelect.tsx @@ -1,4 +1,5 @@ import React from "react"; +import { isNil } from "lodash"; import { Option } from "../../../FieldsSelect"; import { TypeSelect } from "../../../TypeSelect"; import { useTranslation } from "react-i18next"; @@ -23,12 +24,11 @@ export default function InputModeSelect(props: Props) { const { t } = useTranslation(); const { temporaryUserDefinedList } = useSettings(); - const value = - item.valueEditor === null - ? InputMode.AnyValue - : item.valueEditor.allowOtherValue - ? InputMode.AnyValueWithSuggestions - : InputMode.FixedList; + const value = isNil(item.valueEditor) + ? InputMode.AnyValue + : item.valueEditor.allowOtherValue + ? InputMode.AnyValueWithSuggestions + : InputMode.FixedList; return ( <> diff --git a/designer/client/src/reducers/scenarioState.ts b/designer/client/src/reducers/scenarioState.ts index 7bbbae26dd6..202fafb64f6 100644 --- a/designer/client/src/reducers/scenarioState.ts +++ b/designer/client/src/reducers/scenarioState.ts @@ -4,7 +4,9 @@ import { ProcessStateType } from "../components/Process/types"; export const reducer: Reducer = (state = null, action: Action): ProcessStateType => { switch (action.type) { case "DISPLAY_PROCESS": - return action.scenario.state; + // Since scenario endpoint doesn't return null attributes the state will be undefined for fragments. + // Redux does not allow to return undefined values so in that case we return null explicitly. + return action.scenario.state ?? null; case "PROCESS_STATE_LOADED": return action.processState; default: From 9e4509641a3889c30dfa1aa6c54379e07bf47ad1 Mon Sep 17 00:00:00 2001 From: Maciej Cichanowicz Date: Thu, 9 Jan 2025 20:01:48 +0100 Subject: [PATCH 6/6] modify test to verify no null atributes are present in the response --- .../ui/api/ProcessesResourcesSpec.scala | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ProcessesResourcesSpec.scala b/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ProcessesResourcesSpec.scala index 349fd4fe18c..1cda12db9a8 100644 --- a/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ProcessesResourcesSpec.scala +++ b/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ProcessesResourcesSpec.scala @@ -129,27 +129,40 @@ class ProcessesResourcesSpec } } - test("/api/processes should return lighter details without ProcessAction's additional fields") { + test("/api/processes should return lighter details without ProcessAction's additional fields and null values") { + def hasNullAttributes(json: Json): Boolean = { + json.fold( + jsonNull = true, // null found + jsonBoolean = _ => false, + jsonNumber = _ => false, + jsonString = _ => false, + jsonArray = _.exists(hasNullAttributes), + jsonObject = _.values.exists(hasNullAttributes) + ) + } createDeployedScenario(processName, category = Category1) Get(s"/api/processes") ~> withReaderUser() ~> applicationRoute ~> check { status shouldEqual StatusCodes.OK - val loadedProcess = responseAs[List[ScenarioWithDetails]] - - loadedProcess.head.lastAction should matchPattern { + // verify that unnecessary fields were omitted + val decodedScenarios = responseAs[List[ScenarioWithDetails]] + decodedScenarios.head.lastAction should matchPattern { case Some( ProcessAction(_, _, _, _, _, _, _, _, None, None, None, buildInfo) ) if buildInfo.isEmpty => } - loadedProcess.head.lastStateAction should matchPattern { + decodedScenarios.head.lastStateAction should matchPattern { case Some( ProcessAction(_, _, _, _, _, _, _, _, None, None, None, buildInfo) ) if buildInfo.isEmpty => } - loadedProcess.head.lastDeployedAction should matchPattern { + decodedScenarios.head.lastDeployedAction should matchPattern { case Some( ProcessAction(_, _, _, _, _, _, _, _, None, None, None, buildInfo) ) if buildInfo.isEmpty => } + // verify that null values were not present in JSON response + val rawFetchedScenarios = responseAs[Json] + hasNullAttributes(rawFetchedScenarios) shouldBe false } }