From 15d99fc5f1e15f898da6f8cb8f67424104fa995f Mon Sep 17 00:00:00 2001 From: Maciej Cichanowicz <30436981+Elmacioro@users.noreply.github.com> Date: Thu, 9 Jan 2025 21:43:10 +0100 Subject: [PATCH] Reduce response payload size for scenarios tab (#7354) --- .../fragment-input-definition/item/types.ts | 3 +- .../variants/fields/InputModeSelect.tsx | 12 +++--- designer/client/src/reducers/scenarioState.ts | 4 +- .../scenariodetails/ScenarioWithDetails.scala | 10 ++++- .../ui/api/AppApiHttpService.scala | 2 +- .../ui/api/ProcessesResources.scala | 7 ++-- .../ui/api/RemoteEnvironmentResources.scala | 4 +- .../ui/process/ProcessService.scala | 24 +++++++++++- .../ScenarioWithDetailsConversions.scala | 21 +++++++++++ ...sageStatisticsReportsSettingsService.scala | 2 +- .../ui/api/ProcessesResourcesSpec.scala | 37 +++++++++++++++++++ docs/Changelog.md | 1 + 12 files changed, 108 insertions(+), 19 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: 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/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 9b216dafb8e..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 @@ -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 ) } } @@ -232,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 8e5ae07a82d..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,11 +73,19 @@ object ProcessService { val detailsOnly: GetScenarioWithDetailsOptions = new GetScenarioWithDetailsOptions(SkipScenarioGraph, fetchState = false) - val withsScenarioGraph: GetScenarioWithDetailsOptions = + val withScenarioGraph: 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/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( 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..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,6 +129,43 @@ class ProcessesResourcesSpec } } + 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 + // 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 => + } + decodedScenarios.head.lastStateAction should matchPattern { + case Some( + ProcessAction(_, _, _, _, _, _, _, _, None, None, None, buildInfo) + ) if buildInfo.isEmpty => + } + 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 + } + } + test("return single process") { createDeployedExampleScenario(processName, category = Category1) MockableDeploymentManager.configureScenarioStatuses( 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