Skip to content

Commit

Permalink
Reduce response payload size for scenarios tab (#7354)
Browse files Browse the repository at this point in the history
  • Loading branch information
Elmacioro authored Jan 9, 2025
1 parent 42717d0 commit 15d99fc
Show file tree
Hide file tree
Showing 12 changed files with 108 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isNil } from "lodash";
import { Expression, ReturnedType } from "../../../../../types";
import { resolveRefClazzName } from "./utils";

Expand Down Expand Up @@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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 (
<>
<FormControl>
Expand Down
4 changes: 3 additions & 1 deletion designer/client/src/reducers/scenarioState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import { ProcessStateType } from "../components/Process/types";
export const reducer: Reducer<ProcessStateType> = (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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class AppApiHttpService(
processService
.getLatestProcessesWithDetails(
ScenarioQuery.unarchivedProcesses,
GetScenarioWithDetailsOptions.withsScenarioGraph.withValidation
GetScenarioWithDetailsOptions.withScenarioGraph.withValidation
)
.map { processes =>
processes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import pl.touk.nussknacker.ui.process.ProcessService.{
CreateScenarioCommand,
FetchScenarioGraph,
GetScenarioWithDetailsOptions,
SkipAdditionalFields,
UpdateScenarioCommand
}
import pl.touk.nussknacker.ui.process.ScenarioWithDetailsConversions._
Expand Down Expand Up @@ -81,7 +82,7 @@ class ProcessesResources(
complete {
processService.getLatestProcessesWithDetails(
query,
GetScenarioWithDetailsOptions.detailsOnly.withFetchState
GetScenarioWithDetailsOptions.withoutAdditionalFields.withFetchState
)
}
}
Expand Down Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class RemoteEnvironmentResources(
for {
processes <- processService.getLatestProcessesWithDetails(
ScenarioQuery.unarchived,
GetScenarioWithDetailsOptions.withsScenarioGraph
GetScenarioWithDetailsOptions.withScenarioGraph
)
comparison <- compareProcesses(processes)
} yield NuDesignerErrorToHttp.toResponseEither(comparison)
Expand Down Expand Up @@ -137,7 +137,7 @@ class RemoteEnvironmentResources(
.getProcessWithDetails(
processIdWithName,
version,
GetScenarioWithDetailsOptions.withsScenarioGraph
GetScenarioWithDetailsOptions.withScenarioGraph
)
.flatMap(fun)
.map(NuDesignerErrorToHttp.toResponseEither[T])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ object UsageStatisticsReportsSettingsService extends LazyLogging {
processService
.getLatestProcessesWithDetails(
ScenarioQuery.unarchived,
GetScenarioWithDetailsOptions.withsScenarioGraph.withFetchState
GetScenarioWithDetailsOptions.withScenarioGraph.withFetchState
)(user)
.map { scenariosDetails =>
Right(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions docs/Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 15d99fc

Please sign in to comment.