NIFI-16206 connector toggle to handle "true"/"false" strings as booleans

[scottyaslan]

### Description

BOOLEAN properties in the Connector Configuration wizard could misdisplay and fail to update dependents when rendered as slide toggles.

Symptoms

• Dependent properties did not update live when a BOOLEAN toggle changed
• Stored or default "false" could render as checked
• Dependency-gated properties/steps could stay wrong after toggle, navigation, or reopen

Root cause
BOOLEAN values cross the wire as strings ("true" / "false") via ConnectorValueReference and property defaults. MatSlideToggle coerces with !!value, so the string "false" is truthy. Separately, dependsOn.dependentValues is string[] from the API while the live form value is a native boolean; Array.prototype.includes uses strict equality, so true does not match "true".

Fix

• Coerce BOOLEAN wire values to real booleans via toBooleanValue at form init, CVA writeValue, and fromValueReference
• Shared isDependencyValueSatisfied with String(value) coercion for step dependencies, property dependencies, and validation visibility
• Keep save-delta comparison symmetric so an untouched BOOLEAN with wire "false" is not treated as changed
• Regression unit tests for coercion, BOOLEAN-gated visibility, and dirty-delta behavior

Jira

https://issues.apache.org/jira/browse/NIFI-16206

Testing

• Unit tests added/updated under libs/shared for value-reference coercion, dependency matching, configuration-step BOOLEAN visibility, and dirty-delta
• Manual: connector with BOOLEAN + dependsOn — default/saved "false" renders unchecked; toggle on/off updates dependents; navigate away/back and reopen stays consistent; changing an unrelated field does not send an untouched BOOLEAN as changed

[mcgilman]

Reviewing...

[mcgilman]

The mechanism is correctly diagnosed and the fix lands in the right places. All gates pass locally (897 shared tests, lint, build). One blocking item, three should-fixes, and some nits.

Blocking: toBooleanValue is case-sensitive, but the backend is not

// nifi-frontend/src/main/frontend/libs/shared/src/services/value-reference.helper.ts:48
export function toBooleanValue(value: unknown): boolean {
    return value === true || value === 'true';
}

StandardConnectorPropertyValue.asBoolean() reads the same wire value with Boolean.parseBoolean, which accepts "True" and "TRUE":

// nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/components/connector/StandardConnectorPropertyValue.java:53
public Boolean asBoolean() {
    return rawValue == null ? null : Boolean.parseBoolean(rawValue);
}

So a connector declaring a mixed-case BOOLEAN defaultValue renders the toggle off while the connector itself reads the property as on. Because buildChangedConfiguration normalizes the original value through this same helper (connector-configuration-step.component.ts:909), toggling once would not register as dirty either, so the user has no way to correct it from the UI. Suggest:

return value === true || (typeof value === 'string' && value.toLowerCase() === 'true');

The JSDoc above asserts the narrower contract ("Any value other than true / \"true\" is therefore treated as false") and should move with it.

Should fix: extract the dependency predicate to a leaf module

isDependencyValueSatisfied is a pure predicate but lives at components/connector-wizard/step-dependency.utils.ts:75, so utils/connector-validation.utils.ts:26 now imports upward into components/. There is no cycle today, but connector-configuration-summary-step already imports connector-validation.utils, so we are one import away from one.

Suggest a leaf utils/dependency-value.utils.ts with step-dependency.utils.ts as a consumer alongside the other two call sites, and moving the new isDependencyValueSatisfied block out of step-dependency.utils.spec.ts into a matching dependency-value.utils.spec.ts so the tests sit with the unit they cover. No barrel change needed, since this directory is imported by direct path.

Should fix: the read-only summary path does not honor descriptor defaults

PropertyGroupCard (components/property-group-card/property-group-card.component.ts) renders the summary step and was not touched:

// property-group-card.component.ts:75
hasValue(propertyName: string): boolean {
    const valueRef = this.propertyGroup().propertyValues?.[propertyName];
    if (!valueRef) return false;
    ...
}

Neither hasValue nor getDisplayValueForProperty consults getDescriptor(propertyName), so a property whose effective value comes from defaultValue renders as "No value set" in the summary while the configuration step -- which now correctly applies the descriptor default -- shows it populated. A user who never opens the configuration step sees a summary that disagrees with what will actually be applied. That is the same inconsistency this PR is fixing, one component over.

Suggest falling back to getDescriptor(propertyName)?.defaultValue in both methods and routing BOOLEAN through toBooleanValue. Rendering BOOLEAN as a typed Yes/No rather than the raw wire string is a separate gap in this component and I would leave it out of scope here.

Should fix: no direct test for toBooleanValue

toBooleanValue and the new BOOLEAN branch of fromValueReference are covered only transitively through the component spec -- there is no value-reference.helper.spec.ts at all. Suggest adding one covering toBooleanValue (including 'TRUE' and 'True', which is what surfaces the blocking issue above) and fromValueReference(ref, 'BOOLEAN') returning null for a null value so callers can still fall back to the descriptor default.

Nits

• connector-property-input.component.spec.ts covers both BOOLEAN directions through writeValue (lines 465, 479) but has no non-BOOLEAN passthrough case. Since this PR rewrites writeValue into a ternary (connector-property-input.component.ts:195), a "should not coerce values for non-BOOLEAN properties" test is the one guarding that refactor.
• step-dependency.utils.ts:68 says the helper is shared by two sites; there are three (step-dependency.utils.ts:117, connector-configuration-step.component.ts:661, connector-validation.utils.ts:143). The comment exists to deter drift, so it should name all three.
• Worth adding a line to that same doc noting that dependency matching stays case-sensitive to mirror the backend evaluators. The contrast with the now case-insensitive toBooleanValue next door is otherwise easy to misread as an oversight.
• The @param propertyType doc on fromValueReference still only mentions STRING_LIST splitting and no longer describes what the parameter does.
• The PR title and commit subject describe only the coercion half and omit the dependsOn matching half. Something like "Fix BOOLEAN toggle coercion and dependsOn matching in connector wizard" covers both.

The added tests are otherwise well targeted -- the getConfigurationForSave case covering a "false" default not being falsely reported as changed is the subtle one and I am glad it is there.

[mcgilman]

Re-reviewed the updated branch. The blocking case-sensitivity issue is fixed, the leaf-module extraction is done correctly and completely, and the new value-reference.helper.spec.ts closes the coverage gap. Gates pass locally: 905/905 tests, lint clean, build clean.

Two follow-ups on the PropertyGroupCard change, one of which I would like addressed before merge.

1. hasValue duplicates hasPropertyValue, and its SECRET branch is weaker

The new descriptor-default fallback re-implements logic that already exists in this library:

// components/property-group-card/property-group-card.component.ts:75
hasValue(propertyName: string): boolean {
    const valueRef = this.propertyGroup().propertyValues?.[propertyName];
    if (valueRef?.valueType === 'SECRET_REFERENCE') return true;
    if (valueRef?.valueType === 'ASSET_REFERENCE') {
        return (valueRef.assetReferences?.length ?? 0) > 0;
    }

    const value = valueRef?.value ?? this.getDescriptor(propertyName)?.defaultValue;
    return value !== null && value !== undefined && value !== '';
}

hasPropertyValue in utils/connector-validation.utils.ts:31 already handles all three branches, including the defaultValue fallback at lines 65-67, and handles SECRET more carefully. It requires the reference to actually be a SECRET_REFERENCE and to produce a non-empty composite key (lines 44-57), whereas the card returns true on the valueType check alone.

That difference has a real consequence one method down. getDisplayValueForProperty masks on the same weak condition:

// property-group-card.component.ts:86
getDisplayValueForProperty(propertyName: string): string {
    const valueRef = this.propertyGroup().propertyValues?.[propertyName];
    if (valueRef?.valueType === 'SECRET_REFERENCE') return '••••••••';
    ...
    return valueRef?.value ?? this.getDescriptor(propertyName)?.defaultValue ?? '';
}

A SECRET descriptor that carries a defaultValue and has no saved value fails the mask check, falls through to line 94, and returns the raw default -- which the template then renders as visible text at property-group-card.component.html:37-39. Whether a SECRET descriptor ever ships a defaultValue is a server-side question, but the card should not be the component that decides it.

Suggest delegating:

hasValue(propertyName: string): boolean {
    const valueRef = this.propertyGroup().propertyValues?.[propertyName];
    const descriptor = this.getDescriptor(propertyName);
    return hasPropertyValue(valueRef, descriptor?.type ?? 'STRING', descriptor?.defaultValue);
}

and gating the default fallback in getDisplayValueForProperty on the same type check, so the mask cannot be bypassed. That removes the duplication and closes the gap in one change.

2. The new visibility tests could exercise the reactive path

// connector-configuration-step.component.spec.ts:454, :472
component.setPropertyValue('enableImageExtraction', true);
component['computeAllPropertyVisibility']();

This matches the convention already in the file (two other call sites predate this PR), so it is a suggestion rather than a defect. But in production the recompute is driven by the valueChanges subscription established at connector-configuration-step.component.ts:528, inside a Promise.resolve().then(...). Calling the private method directly means these tests would still pass if that subscription were broken or never wired up -- which for the two new BOOLEAN cases is arguably the more interesting half of the behavior, since the whole point is that a toggle flip propagates to dependent properties. Wrapping in fakeAsync, calling flush() after setup, and dropping the manual invocation would cover the wiring as well as the comparison.

Everything else I looked at held up: the coercion is applied consistently at every point where a BOOLEAN enters the form or the save payload, the dirty check is symmetric (matching false fallbacks on both sides, ?? rather than || throughout), and an untouched BOOLEAN is never included in the payload, so an unusual stored value survives a read/render/save cycle unchanged.

[scottyaslan]

Thanks for the review @mcgilman ! I have addressed your feedback.

[mcgilman]

Thanks for the updates @scottyaslan!