Skip to content

Commit

Permalink
Refactor fixupEnsembleIdent(s) overloaded functions into separate fun…
Browse files Browse the repository at this point in the history
…ctions

Based on discussion: Remove overload, and explicitly implement fixup based on argument types. As the overloads had a bug, where currIdent could be of a type, and if the type does not exist in ensembleSet, an incorrect ident would be returned for a module not supporting it.

E.g. request RegularEnsembleIdent, ensemble set removes last RegularEnsembleIdent, then DeltaEnsembleIdent is returned, and the module might get error scenario.
  • Loading branch information
jorgenherje committed Dec 17, 2024
1 parent 96e5d89 commit 2c125e6
Show file tree
Hide file tree
Showing 10 changed files with 182 additions and 78 deletions.
109 changes: 55 additions & 54 deletions frontend/src/framework/utils/ensembleUiHelpers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { isEnsembleIdentOfType } from "./ensembleIdentUtils";

import { DeltaEnsembleIdent } from "../DeltaEnsembleIdent";
import { EnsembleSet } from "../EnsembleSet";
import { RegularEnsembleIdent } from "../RegularEnsembleIdent";
Expand Down Expand Up @@ -30,22 +28,10 @@ export function maybeAssignFirstSyncedEnsemble(
* will always return a reference to the exact same object that was passed in currIdent. This
* means that you can compare the references (fixedIdent !== currIdent) to detect any changes.
*/
export function fixupEnsembleIdent(
currIdent: RegularEnsembleIdent | null,
ensembleSet: EnsembleSet | null
): RegularEnsembleIdent | null;
export function fixupEnsembleIdent(
currIdent: DeltaEnsembleIdent | null,
ensembleSet: EnsembleSet | null
): DeltaEnsembleIdent | null;
export function fixupEnsembleIdent(
currIdent: RegularEnsembleIdent | DeltaEnsembleIdent | null,
ensembleSet: EnsembleSet | null
): (RegularEnsembleIdent | DeltaEnsembleIdent) | null;
export function fixupEnsembleIdent(
currIdent: RegularEnsembleIdent | DeltaEnsembleIdent | null,
ensembleSet: EnsembleSet | null
): (RegularEnsembleIdent | DeltaEnsembleIdent) | null {
): RegularEnsembleIdent | DeltaEnsembleIdent | null {
if (!ensembleSet?.hasAnyEnsembles()) {
return null;
}
Expand All @@ -54,46 +40,45 @@ export function fixupEnsembleIdent(
return currIdent;
}

const regularEnsembles = ensembleSet.getRegularEnsembleArray();
const deltaEnsembles = ensembleSet.getDeltaEnsembleArray();
return ensembleSet.getEnsembleArray()[0].getIdent();
}

if (currIdent && isEnsembleIdentOfType(currIdent, RegularEnsembleIdent) && regularEnsembles.length > 0) {
return regularEnsembles[0].getIdent();
/**
* Validates the the RegularEnsembleIdent specified in currIdent against the contents of the
* EnsembleSet and fixes the value if it isn't valid.
*
* Returns null if specified EnsembleSet does not contain any regular ensembles.
*
* Note that if the specified RegularEnsembleIdents is valid, this function will always return
* a reference to the exact same object that was passed in currIdent. This means that you can
* compare the references (fixedIdent !== currIdent) to detect any changes.
*/
export function fixupRegularEnsembleIdent(
currIdent: RegularEnsembleIdent | null,
ensembleSet: EnsembleSet | null
): RegularEnsembleIdent | null {
if (!ensembleSet?.hasAnyRegularEnsembles()) {
return null;
}

if (currIdent && isEnsembleIdentOfType(currIdent, DeltaEnsembleIdent) && deltaEnsembles.length > 0) {
return deltaEnsembles[0].getIdent();
if (currIdent && ensembleSet.hasEnsemble(currIdent)) {
return currIdent;
}

return regularEnsembles.length > 0
? regularEnsembles[0].getIdent()
: deltaEnsembles.length > 0
? deltaEnsembles[0].getIdent()
: null;
return ensembleSet.getRegularEnsembleArray()[0].getIdent();
}

/**
* Validates the the EnsembleIdents or DeltaEnsembleIdents specified in currIdents against the
* contents of the EnsembleSet and fixes the value if it isn't valid.
* Validates the the RegularEnsembleIdents or DeltaEnsembleIdents specified in currIdents
* against the contents of the EnsembleSet and fixes the value if it isn't valid.
*
* Returns null if an empty EnsembleSet is specified.
*
* Note that if the specified EnsembleIdents and DeltaEnsembleIdents are valid, this function
* will always return a reference to the exact same object that was passed in currIdent. This
* means that you can compare the references (fixedIdent !== currIdent) to detect any changes.
* Note that if the specified RegularEnsembleIdents and DeltaEnsembleIdents are valid, this
* function will always return a reference to the exact same object that was passed in
* currIdent. This means that you can compare the references (fixedIdent !== currIdent) to
* detect any changes.
*/
export function fixupEnsembleIdents(
currIdents: RegularEnsembleIdent[] | null,
ensembleSet: EnsembleSet | null
): RegularEnsembleIdent[] | null;
export function fixupEnsembleIdents(
currIdents: DeltaEnsembleIdent[] | null,
ensembleSet: EnsembleSet | null
): DeltaEnsembleIdent[] | null;
export function fixupEnsembleIdents(
currIdents: (RegularEnsembleIdent | DeltaEnsembleIdent)[] | null,
ensembleSet: EnsembleSet | null
): (RegularEnsembleIdent | DeltaEnsembleIdent)[] | null;
export function fixupEnsembleIdents(
currIdents: (RegularEnsembleIdent | DeltaEnsembleIdent)[] | null,
ensembleSet: EnsembleSet | null
Expand All @@ -103,17 +88,33 @@ export function fixupEnsembleIdents(
}

if (currIdents === null || currIdents.length === 0) {
// Provide first regular ensemble ident by default
if (ensembleSet.hasAnyRegularEnsembles()) {
return [ensembleSet.getRegularEnsembleArray()[0].getIdent()];
}
if (ensembleSet.hasAnyDeltaEnsembles()) {
return [ensembleSet.getDeltaEnsembleArray()[0].getIdent()];
}
return [];
return [ensembleSet.getEnsembleArray()[0].getIdent()];
}

return currIdents.filter((currIdent) => ensembleSet.hasEnsemble(currIdent));
}

/**
* Validates the the RegularEnsembleIdents specified in currIdents against the contents of the
* EnsembleSet and fixes the value if it isn't valid.
*
* Returns null if an empty EnsembleSet is specified.
*
* Note that if the specified RegularEnsembleIdents are valid, this function will always return
* a reference to the exact same object that was passed in currIdent. This means that you can
* compare the references (fixedIdent !== currIdent) to detect any changes.
*/
export function fixupRegularEnsembleIdents(
currIdents: RegularEnsembleIdent[] | null,
ensembleSet: EnsembleSet | null
): RegularEnsembleIdent[] | null {
if (!ensembleSet?.hasAnyRegularEnsembles()) {
return null;
}

if (currIdents === null || currIdents.length === 0) {
return [ensembleSet.getRegularEnsembleArray()[0].getIdent()];
}

return currIdents.filter((currIdent) => {
return ensembleSet.hasEnsemble(currIdent);
});
return currIdents.filter((currIdent) => ensembleSet.hasEnsemble(currIdent));
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { DatedFlowNetwork_api, FlowNetworkMetadata_api } from "@api";
import { EnsembleSetAtom } from "@framework/GlobalAtoms";
import { RegularEnsembleIdent } from "@framework/RegularEnsembleIdent";
import { fixupEnsembleIdent } from "@framework/utils/ensembleUiHelpers";
import { fixupRegularEnsembleIdent } from "@framework/utils/ensembleUiHelpers";

import { atom } from "jotai";

Expand All @@ -25,7 +25,7 @@ export const selectedEnsembleIdentAtom = atom<RegularEnsembleIdent | null>((get)
const ensembleSet = get(EnsembleSetAtom);
const userSelectedEnsembleIdent = get(userSelectedEnsembleIdentAtom);

const validEnsembleIdent = fixupEnsembleIdent(userSelectedEnsembleIdent, ensembleSet);
const validEnsembleIdent = fixupRegularEnsembleIdent(userSelectedEnsembleIdent, ensembleSet);
return validEnsembleIdent;
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { FluidZone_api, InplaceVolumetricResultName_api, InplaceVolumetricsIdentifierWithValues_api } from "@api";
import { EnsembleSetAtom } from "@framework/GlobalAtoms";
import { fixupEnsembleIdents } from "@framework/utils/ensembleUiHelpers";
import { fixupRegularEnsembleIdents } from "@framework/utils/ensembleUiHelpers";
import { fixupUserSelection } from "@lib/utils/fixupUserSelection";
import { fixupUserSelectedIdentifierValues } from "@modules/_shared/InplaceVolumetrics/fixupUserSelectedIdentifierValues";
import { RealSelector, SelectorColumn, SourceAndTableIdentifierUnion } from "@modules/_shared/InplaceVolumetrics/types";
Expand Down Expand Up @@ -41,7 +41,7 @@ export const selectedEnsembleIdentsAtom = atom((get) => {
ensembleSet.hasEnsemble(ensemble)
);

const validatedEnsembleIdents = fixupEnsembleIdents(newSelectedEnsembleIdents, ensembleSet);
const validatedEnsembleIdents = fixupRegularEnsembleIdents(newSelectedEnsembleIdents, ensembleSet);

return validatedEnsembleIdents ?? [];
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { FluidZone_api, InplaceVolumetricResultName_api, InplaceVolumetricsIdentifierWithValues_api } from "@api";
import { EnsembleSetAtom } from "@framework/GlobalAtoms";
import { fixupEnsembleIdents } from "@framework/utils/ensembleUiHelpers";
import { fixupRegularEnsembleIdents } from "@framework/utils/ensembleUiHelpers";
import { fixupUserSelection } from "@lib/utils/fixupUserSelection";
import { fixupUserSelectedIdentifierValues } from "@modules/_shared/InplaceVolumetrics/fixupUserSelectedIdentifierValues";
import { SourceAndTableIdentifierUnion, SourceIdentifier } from "@modules/_shared/InplaceVolumetrics/types";
Expand Down Expand Up @@ -36,7 +36,7 @@ export const selectedEnsembleIdentsAtom = atom((get) => {
ensembleSet.hasEnsemble(ensemble)
);

const validatedEnsembleIdents = fixupEnsembleIdents(newSelectedEnsembleIdents, ensembleSet);
const validatedEnsembleIdents = fixupRegularEnsembleIdents(newSelectedEnsembleIdents, ensembleSet);

return validatedEnsembleIdents ?? [];
});
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/modules/Map/settings/settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { useSettingsStatusWriter } from "@framework/StatusWriter";
import { SyncSettingKey, SyncSettingsHelper } from "@framework/SyncSettings";
import { useEnsembleSet } from "@framework/WorkbenchSession";
import { EnsembleDropdown } from "@framework/components/EnsembleDropdown";
import { fixupEnsembleIdent, maybeAssignFirstSyncedEnsemble } from "@framework/utils/ensembleUiHelpers";
import { fixupRegularEnsembleIdent, maybeAssignFirstSyncedEnsemble } from "@framework/utils/ensembleUiHelpers";
import { Checkbox } from "@lib/components/Checkbox";
import { CircularProgress } from "@lib/components/CircularProgress";
import { Input } from "@lib/components/Input";
Expand Down Expand Up @@ -53,7 +53,7 @@ export function MapSettings(props: ModuleSettingsProps<Interfaces>) {
const syncedValueDate = syncHelper.useValue(SyncSettingKey.DATE, "global.syncValue.date");

const candidateEnsembleIdent = maybeAssignFirstSyncedEnsemble(selectedEnsembleIdent, syncedValueEnsembles);
const computedEnsembleIdent = fixupEnsembleIdent(candidateEnsembleIdent, ensembleSet);
const computedEnsembleIdent = fixupRegularEnsembleIdent(candidateEnsembleIdent, ensembleSet);
const realizationSurfacesMetaQuery = useRealizationSurfacesMetadataQuery(
computedEnsembleIdent?.getCaseUuid(),
computedEnsembleIdent?.getEnsembleName()
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/modules/Rft/settings/atoms/derivedAtoms.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { EnsembleSetAtom } from "@framework/GlobalAtoms";
import { RegularEnsembleIdent } from "@framework/RegularEnsembleIdent";
import { fixupEnsembleIdent } from "@framework/utils/ensembleUiHelpers";
import { fixupRegularEnsembleIdent } from "@framework/utils/ensembleUiHelpers";

import { atom } from "jotai";

Expand Down Expand Up @@ -30,7 +30,7 @@ export const selectedEnsembleIdentAtom = atom<RegularEnsembleIdent | null>((get)
const ensembleSet = get(EnsembleSetAtom);
const userSelectedEnsembleIdent = get(userSelectedEnsembleIdentAtom);

const validEnsembleIdent = fixupEnsembleIdent(userSelectedEnsembleIdent, ensembleSet);
const validEnsembleIdent = fixupRegularEnsembleIdent(userSelectedEnsembleIdent, ensembleSet);
return validEnsembleIdent;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { RegularEnsembleIdent } from "@framework/RegularEnsembleIdent";
import { SyncSettingKey, SyncSettingsHelper } from "@framework/SyncSettings";
import { useEnsembleSet } from "@framework/WorkbenchSession";
import { EnsembleDropdown } from "@framework/components/EnsembleDropdown";
import { fixupEnsembleIdent, maybeAssignFirstSyncedEnsemble } from "@framework/utils/ensembleUiHelpers";
import { fixupRegularEnsembleIdent, maybeAssignFirstSyncedEnsemble } from "@framework/utils/ensembleUiHelpers";
import { Checkbox } from "@lib/components/Checkbox";
import { CircularProgress } from "@lib/components/CircularProgress";
import { CollapsibleGroup } from "@lib/components/CollapsibleGroup";
Expand Down Expand Up @@ -55,7 +55,7 @@ export function Settings({ settingsContext, workbenchSession, workbenchServices
const syncedValueSummaryVector = syncHelper.useValue(SyncSettingKey.TIME_SERIES, "global.syncValue.timeSeries");

const candidateEnsembleIdent = maybeAssignFirstSyncedEnsemble(selectedEnsembleIdent, syncedValueEnsembles);
const computedEnsembleIdent = fixupEnsembleIdent(candidateEnsembleIdent, ensembleSet);
const computedEnsembleIdent = fixupRegularEnsembleIdent(candidateEnsembleIdent, ensembleSet);

const vectorsListQuery = useVectorListQuery(
computedEnsembleIdent?.getCaseUuid(),
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/modules/SubsurfaceMap/settings/settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { RegularEnsembleIdent } from "@framework/RegularEnsembleIdent";
import { SyncSettingKey, SyncSettingsHelper } from "@framework/SyncSettings";
import { useEnsembleSet } from "@framework/WorkbenchSession";
import { EnsembleDropdown } from "@framework/components/EnsembleDropdown";
import { fixupEnsembleIdent, maybeAssignFirstSyncedEnsemble } from "@framework/utils/ensembleUiHelpers";
import { fixupRegularEnsembleIdent, maybeAssignFirstSyncedEnsemble } from "@framework/utils/ensembleUiHelpers";
import { Button } from "@lib/components/Button";
import { Checkbox } from "@lib/components/Checkbox";
import { CircularProgress } from "@lib/components/CircularProgress";
Expand Down Expand Up @@ -99,7 +99,7 @@ export function Settings({ settingsContext, workbenchSession, workbenchServices
const syncedValueEnsembles = syncHelper.useValue(SyncSettingKey.ENSEMBLE, "global.syncValue.ensembles");
const syncedValueSurface = syncHelper.useValue(SyncSettingKey.SURFACE, "global.syncValue.surface");
const candidateEnsembleIdent = maybeAssignFirstSyncedEnsemble(selectedEnsembleIdent, syncedValueEnsembles);
const computedEnsembleIdent = fixupEnsembleIdent(candidateEnsembleIdent, ensembleSet);
const computedEnsembleIdent = fixupRegularEnsembleIdent(candidateEnsembleIdent, ensembleSet);
if (computedEnsembleIdent && !computedEnsembleIdent.equals(selectedEnsembleIdent)) {
setSelectedEnsembleIdent(computedEnsembleIdent);
}
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/modules/Vfp/settings/atoms/derivedAtoms.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { EnsembleSetAtom } from "@framework/GlobalAtoms";
import { RegularEnsembleIdent } from "@framework/RegularEnsembleIdent";
import { fixupEnsembleIdent } from "@framework/utils/ensembleUiHelpers";
import { fixupRegularEnsembleIdent } from "@framework/utils/ensembleUiHelpers";
import { isProdTable } from "@modules/Vfp/utils/vfpTableClassifier";

import { atom } from "jotai";
Expand Down Expand Up @@ -34,7 +34,7 @@ export const selectedEnsembleIdentAtom = atom<RegularEnsembleIdent | null>((get)
const ensembleSet = get(EnsembleSetAtom);
const userSelectedEnsembleIdent = get(userSelectedEnsembleIdentAtom);

const validEnsembleIdent = fixupEnsembleIdent(userSelectedEnsembleIdent, ensembleSet);
const validEnsembleIdent = fixupRegularEnsembleIdent(userSelectedEnsembleIdent, ensembleSet);
return validEnsembleIdent;
});

Expand Down
Loading

0 comments on commit 2c125e6

Please sign in to comment.