Skip to content

feat: add bypassIfValuesNull to openfed__subscriptionFilter #2815

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jensneuse wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

feat: add bypassIfValuesNull to openfed__subscriptionFilter #2815

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4d3b15a
feat: add bypassIfValuesNull to openfed__subscriptionFilter
jensneuse May 4, 2026
ca888a4
Merge remote-tracking branch 'origin/main' into streams-bypass-filter…
jensneuse May 4, 2026
4093ad5
fix(router-tests): adapt to renamed WebSocketServerReadTimeout + rege…
jensneuse May 4, 2026
446531a
chore(composition): apply prettier formatting to subscription-filter …
jensneuse May 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions composition/src/router-configuration/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export type EventConfiguration = KafkaEventConfiguration | NatsEventConfiguratio
export type SubscriptionFilterValue = boolean | null | number | string;

export type SubscriptionFieldCondition = {
bypassIfValuesNull?: boolean;
fieldPath: string[];
values: SubscriptionFilterValue[];
};
Expand Down
1 change: 1 addition & 0 deletions composition/src/utils/string-constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const AUTHENTICATED = 'authenticated';
export const ARGUMENT_DEFINITION_UPPER = 'ARGUMENT_DEFINITION';
export const BOOLEAN = 'boolean';
export const BOOLEAN_SCALAR = 'Boolean';
export const BYPASS_IF_VALUES_NULL = 'bypassIfValuesNull';
export const CHANNEL = 'channel';
export const CHANNELS = 'channels';
export const COMPOSE_DIRECTIVE = 'composeDirective';
Expand Down
8 changes: 8 additions & 0 deletions composition/src/v1/constants/non-directive-definitions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
import { stringToNamedTypeNode, stringToNameNode } from '../../ast/utils';
import {
AND_UPPER,
BOOLEAN_SCALAR,
BYPASS_IF_VALUES_NULL,
CONSUMER_INACTIVE_THRESHOLD,
CONSUMER_NAME,
EDFS_NATS_STREAM_CONFIGURATION,
Expand Down Expand Up @@ -114,12 +116,18 @@ export const SCOPE_SCALAR_DEFINITION: ScalarTypeDefinitionNode = {
};

/* input openfed__SubscriptionFieldCondition {
* bypassIfValuesNull: Boolean
* fieldPath: String!
* values: [openfed__SubscriptionFilterValue]!
* }
*/
export const SUBSCRIPTION_FIELD_CONDITION_DEFINITION: InputObjectTypeDefinitionNode = {
fields: [
{
kind: Kind.INPUT_VALUE_DEFINITION,
name: stringToNameNode(BYPASS_IF_VALUES_NULL),
type: stringToNamedTypeNode(BOOLEAN_SCALAR),
},
{
kind: Kind.INPUT_VALUE_DEFINITION,
name: stringToNameNode(FIELD_PATH),
Expand Down
24 changes: 24 additions & 0 deletions composition/src/v1/federation/federation-factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ import {
import {
AND_UPPER,
AUTHORIZATION_DIRECTIVES,
BOOLEAN,
BYPASS_IF_VALUES_NULL,
CONDITION,
DEPRECATED,
ENUM_VALUE,
Expand Down Expand Up @@ -2628,10 +2630,32 @@ export class FederationFactory {
const duplicatedFieldNames = new Set<string>();
const invalidFieldNames = new Set<string>();
const fieldErrorMessages: string[] = [];
let hasSeenBypassIfValuesNull = false;
for (const objectFieldNode of objectValueNode.fields) {
const inputFieldName = objectFieldNode.name.value;
const inputFieldPath = inputPath + `.${inputFieldName}`;
switch (inputFieldName) {
case BYPASS_IF_VALUES_NULL: {
if (hasSeenBypassIfValuesNull) {
hasErrors = true;
duplicatedFieldNames.add(BYPASS_IF_VALUES_NULL);
break;
}
hasSeenBypassIfValuesNull = true;
if (objectFieldNode.value.kind !== Kind.BOOLEAN) {
fieldErrorMessages.push(
invalidInputFieldTypeErrorMessage(inputFieldPath, BOOLEAN, kindToNodeType(objectFieldNode.value.kind)),
);
hasErrors = true;
break;
}
// Only persist when explicitly true. Explicit false collapses to undefined so the
// serializer can emit the smallest proto for unchanged configs.
if (objectFieldNode.value.value === true) {
condition.bypassIfValuesNull = true;
}
break;
Comment on lines +2633 to +2657
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Still fail IN conditions when fieldPath or values is missing.

This new optional field makes IN: { bypassIfValuesNull: true } validate successfully, because leftover entries in validFieldNames never flip hasErrors. That lets composition emit an empty fieldPath/values filter downstream.

Suggested fix 
     }
-    if (!hasErrors) {
+    if (validFieldNames.size > 0) {
+      hasErrors = true;
+    }
+    if (!hasErrors) {
       return true;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let hasSeenBypassIfValuesNull = false;
for (const objectFieldNode of objectValueNode.fields) {
const inputFieldName = objectFieldNode.name.value;
const inputFieldPath = inputPath + `.${inputFieldName}`;
switch (inputFieldName) {
case BYPASS_IF_VALUES_NULL: {
if (hasSeenBypassIfValuesNull) {
hasErrors = true;
duplicatedFieldNames.add(BYPASS_IF_VALUES_NULL);
break;
}
hasSeenBypassIfValuesNull = true;
if (objectFieldNode.value.kind !== Kind.BOOLEAN) {
fieldErrorMessages.push(
invalidInputFieldTypeErrorMessage(inputFieldPath, BOOLEAN, kindToNodeType(objectFieldNode.value.kind)),
);
hasErrors = true;
break;
}
// Only persist when explicitly true. Explicit false collapses to undefined so the
// serializer can emit the smallest proto for unchanged configs.
if (objectFieldNode.value.value === true) {
condition.bypassIfValuesNull = true;
}
break;
let hasSeenBypassIfValuesNull = false;
for (const objectFieldNode of objectValueNode.fields) {
const inputFieldName = objectFieldNode.name.value;
const inputFieldPath = inputPath + `.${inputFieldName}`;
switch (inputFieldName) {
case BYPASS_IF_VALUES_NULL: {
if (hasSeenBypassIfValuesNull) {
hasErrors = true;
duplicatedFieldNames.add(BYPASS_IF_VALUES_NULL);
break;
}
hasSeenBypassIfValuesNull = true;
if (objectFieldNode.value.kind !== Kind.BOOLEAN) {
fieldErrorMessages.push(
invalidInputFieldTypeErrorMessage(inputFieldPath, BOOLEAN, kindToNodeType(objectFieldNode.value.kind)),
);
hasErrors = true;
break;
}
// Only persist when explicitly true. Explicit false collapses to undefined so the
// serializer can emit the smallest proto for unchanged configs.
if (objectFieldNode.value.value === true) {
condition.bypassIfValuesNull = true;
}
break;
}
if (validFieldNames.size > 0) {
hasErrors = true;
}
if (!hasErrors) {
return true;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@composition/src/v1/federation/federation-factory.ts` around lines 2535 -
2559, The parser currently lets an IN condition pass validation when only
BYPASS_IF_VALUES_NULL is present because validFieldNames leftovers don't flip
hasErrors; after parsing objectValueNode.fields for the IN condition (the switch
handling BYPASS_IF_VALUES_NULL and other keys), explicitly validate that
required keys "fieldPath" and "values" are present (check validFieldNames or
track presence of fieldPath/values), and if either is missing set hasErrors =
true and push appropriate field error messages (similar to
invalidInputFieldTypeErrorMessage) so you cannot emit an IN filter with empty
fieldPath/values even when condition.bypassIfValuesNull is true; ensure
duplicatedFieldNames/hasSeen handling remains unchanged for
BYPASS_IF_VALUES_NULL and reference the BYPASS_IF_VALUES_NULL constant,
condition.bypassIfValuesNull, validFieldNames, hasErrors, and objectValueNode to
locate where to add this check.

}
case FIELD_PATH: {
if (validFieldNames.has(FIELD_PATH)) {
validFieldNames.delete(FIELD_PATH);
Expand Down
204 changes: 200 additions & 4 deletions composition/tests/v1/directives/subscription-filter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, test } from 'vitest';
import {
BOOLEAN,
CONDITION,
FIRST_ORDINAL,
inaccessibleSubscriptionFieldConditionFieldPathFieldErrorMessage,
Expand All @@ -19,6 +20,7 @@ import {
OBJECT,
parse,
ROUTER_COMPATIBILITY_VERSION_ONE,
STRING_SCALAR,
type Subgraph,
subgraphValidationError,
SUBSCRIPTION,
Expand Down Expand Up @@ -453,6 +455,122 @@ describe('@openfed__subscriptionFilter tests', () => {
]);
});

test('that bypassIfValuesNull: true is preserved on the IN condition', () => {
const result = federateSubgraphsSuccess([subgraphB, subgraphBypass1], ROUTER_COMPATIBILITY_VERSION_ONE);
expect(result.success).toBe(true);
expect(result.fieldConfigurations).toStrictEqual([
{
argumentNames: [],
fieldName: 'field',
subscriptionFilterCondition: {
in: {
bypassIfValuesNull: true,
fieldPath: ['id'],
values: ['1'],
},
},
typeName: SUBSCRIPTION,
},
]);
});

test('that bypassIfValuesNull: false collapses to undefined', () => {
const result = federateSubgraphsSuccess([subgraphB, subgraphBypass2], ROUTER_COMPATIBILITY_VERSION_ONE);
expect(result.success).toBe(true);
expect(result.fieldConfigurations).toStrictEqual([
{
argumentNames: [],
fieldName: 'field',
subscriptionFilterCondition: {
in: {
fieldPath: ['id'],
values: ['1'],
},
},
typeName: SUBSCRIPTION,
},
]);
});

test('that omitting bypassIfValuesNull leaves the IN condition unchanged', () => {
const result = federateSubgraphsSuccess([subgraphB, subgraphC], ROUTER_COMPATIBILITY_VERSION_ONE);
expect(result.success).toBe(true);
expect(result.fieldConfigurations).toStrictEqual([
{
argumentNames: [],
fieldName: 'field',
subscriptionFilterCondition: {
in: {
fieldPath: ['id'],
values: ['1'],
},
},
typeName: SUBSCRIPTION,
},
]);
});

test('that bypassIfValuesNull is propagated through nested OR/IN conditions', () => {
const result = federateSubgraphsSuccess([subgraphB, subgraphBypass3], ROUTER_COMPATIBILITY_VERSION_ONE);
expect(result.success).toBe(true);
expect(result.fieldConfigurations).toStrictEqual([
{
argumentNames: [],
fieldName: 'field',
subscriptionFilterCondition: {
or: [
{
in: {
bypassIfValuesNull: true,
fieldPath: ['id'],
values: ['1'],
},
},
{
in: {
fieldPath: ['id'],
values: ['2'],
},
},
],
},
typeName: SUBSCRIPTION,
},
]);
});

test('that an error is returned if bypassIfValuesNull is not a Boolean', () => {
const result = federateSubgraphsFailure([subgraphB, subgraphBypass4], ROUTER_COMPATIBILITY_VERSION_ONE);
expect(result.errors).toHaveLength(1);
expect(result.errors).toStrictEqual([
invalidSubscriptionFilterDirectiveError('Subscription.field', [
subscriptionFieldConditionInvalidInputFieldErrorMessage(
'condition.IN',
[],
[],
[],
[invalidInputFieldTypeErrorMessage('condition.IN.bypassIfValuesNull', BOOLEAN, STRING_SCALAR)],
),
]),
]);
});

test('that an error is returned if bypassIfValuesNull is duplicated', () => {
const result = federateSubgraphsFailure([subgraphB, subgraphBypass5], ROUTER_COMPATIBILITY_VERSION_ONE);
expect(result.errors).toHaveLength(1);
expect(result.errors).toStrictEqual([
invalidSubscriptionFilterDirectiveError('Subscription.field', [
subscriptionFieldConditionInvalidInputFieldErrorMessage(
'condition.IN',
[],
['bypassIfValuesNull'],
[],
[],
),
]),
]);
});

test('that an entity can be defined as an extension in an EDG', () => {
const { federatedGraphSchema } = federateSubgraphsSuccess(
[subgraphQ, subgraphR],
Expand Down Expand Up @@ -659,23 +777,24 @@ const subgraphG: Subgraph = {
type Entity @key(fields: "id", resolvable: false) {
id: ID! @external
}

type Subscription {
field: Entity! @edfs__kafkaSubscribe(topics: ["employeeUpdated"]) @openfed__subscriptionFilter(condition: { IN: { fieldPath: "id", values: [1] } })
}

input openfed__SubscriptionFieldCondition {
bypassIfValuesNull: Boolean
fieldPath: String!
values: [openfed__SubscriptionFilterValue]!
}

input openfed__SubscriptionFilterCondition {
AND: [openfed__SubscriptionFilterCondition!]
IN: openfed__SubscriptionFieldCondition
NOT: openfed__SubscriptionFilterCondition
OR: [openfed__SubscriptionFilterCondition!]
}

scalar openfed__SubscriptionFilterValue
`),
};
Expand Down Expand Up @@ -927,3 +1046,80 @@ const subgraphR: Subgraph = {
}
`),
};

const subgraphBypass1: Subgraph = {
name: 'subgraph-bypass-1',
url: '',
definitions: parse(`
type Entity @key(fields: "id", resolvable: false) {
id: ID! @external
}

type Subscription {
field: Entity! @edfs__kafkaSubscribe(topics: ["employeeUpdated"]) @openfed__subscriptionFilter(condition: { IN: { fieldPath: "id", values: ["1"], bypassIfValuesNull: true } })
}
`),
};

const subgraphBypass2: Subgraph = {
name: 'subgraph-bypass-2',
url: '',
definitions: parse(`
type Entity @key(fields: "id", resolvable: false) {
id: ID! @external
}

type Subscription {
field: Entity! @edfs__kafkaSubscribe(topics: ["employeeUpdated"]) @openfed__subscriptionFilter(condition: { IN: { fieldPath: "id", values: ["1"], bypassIfValuesNull: false } })
}
`),
};

const subgraphBypass3: Subgraph = {
name: 'subgraph-bypass-3',
url: '',
definitions: parse(`
type Entity @key(fields: "id", resolvable: false) {
id: ID! @external
}

type Subscription {
field: Entity!
@edfs__kafkaSubscribe(topics: ["employeeUpdated"])
@openfed__subscriptionFilter(condition: {
OR: [
{ IN: { fieldPath: "id", values: ["1"], bypassIfValuesNull: true } },
{ IN: { fieldPath: "id", values: ["2"] } }
]
})
}
`),
};

const subgraphBypass4: Subgraph = {
name: 'subgraph-bypass-4',
url: '',
definitions: parse(`
type Entity @key(fields: "id", resolvable: false) {
id: ID! @external
}

type Subscription {
field: Entity! @edfs__kafkaSubscribe(topics: ["employeeUpdated"]) @openfed__subscriptionFilter(condition: { IN: { fieldPath: "id", values: ["1"], bypassIfValuesNull: "yes" } })
}
`),
};

const subgraphBypass5: Subgraph = {
name: 'subgraph-bypass-5',
url: '',
definitions: parse(`
type Entity @key(fields: "id", resolvable: false) {
id: ID! @external
}

type Subscription {
field: Entity! @edfs__kafkaSubscribe(topics: ["employeeUpdated"]) @openfed__subscriptionFilter(condition: { IN: { fieldPath: "id", values: ["1"], bypassIfValuesNull: true, bypassIfValuesNull: false } })
}
`),
};
1 change: 1 addition & 0 deletions composition/tests/v1/utils/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export const OPENFED_SCOPE = ` scalar openfed__Scope`;

export const OPENFED_SUBSCRIPTION_FIELD_CONDITION = `
input openfed__SubscriptionFieldCondition {
bypassIfValuesNull: Boolean
fieldPath: String!
values: [openfed__SubscriptionFilterValue]!
}
Expand Down
25 changes: 18 additions & 7 deletions connect-go/gen/proto/wg/cosmo/node/v1/node.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading