Skip to content
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.

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

Remove deprecated subscriptions #5662

Merged
merged 7 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
8 changes: 8 additions & 0 deletions .changeset/curly-taxis-sleep.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@neo4j/graphql": major
---

Remove support for relationship subscriptions:

- `*RelationshipCreated`
- `*RelatiosnhipDeleted`
angrykoala marked this conversation as resolved.
Show resolved Hide resolved
29 changes: 29 additions & 0 deletions .changeset/great-beans-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
"@neo4j/graphql": major
---

Removes support for non-cdc subscriptions. This means the only available engine for subscriptions is `Neo4jGraphQLSubscriptionsCDCEngine`:

```ts
new Neo4jGraphQL({
typeDefs,
driver,
features: {
subscriptions: new Neo4jGraphQLSubscriptionsCDCEngine({
driver,
}),
Comment on lines +12 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

I think let's leave this like this like now for this PR, but after seeing this and the fact that literally the only thing that can be passed in is the CDC engine, I'm definitely camp "this should be turned into an object for configuration" - maybe a follow-up PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

The one advantage of this is that it leaves the door open for capturing the events by extending this class, I know it is not the primary use case, but some users have been using the CDC subscriptions to capture these events for some external processing.

},
});
```

The default behaviour of subscriptions has also been updated to use CDC, so now passing `true` will use the CDC engine with the default parameters and driver:

```ts
new Neo4jGraphQL({
typeDefs,
driver,
features: {
subscriptions: true,
},
});
```
16 changes: 11 additions & 5 deletions packages/graphql/src/classes/Neo4jGraphQL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import { getNeo4jDatabaseInfo } from "./Neo4jDatabaseInfo";
import type Node from "./Node";
import type Relationship from "./Relationship";
import { Neo4jGraphQLAuthorization } from "./authorization/Neo4jGraphQLAuthorization";
import { Neo4jGraphQLSubscriptionsDefaultEngine } from "./subscription/Neo4jGraphQLSubscriptionsDefaultEngine";
import { Neo4jGraphQLSubscriptionsCDCEngine } from "./subscription/Neo4jGraphQLSubscriptionsCDCEngine";
import type { AssertIndexesAndConstraintsOptions } from "./utils/asserts-indexes-and-constraints";
import { assertIndexesAndConstraints } from "./utils/asserts-indexes-and-constraints";
import { generateResolverComposition } from "./utils/generate-resolvers-composition";
Expand Down Expand Up @@ -295,7 +295,7 @@ class Neo4jGraphQL {

const isSubscriptionEnabled = !!this.features.subscriptions;
const wrapSubscriptionResolverArgs = {
subscriptionsEngine: this.features.subscriptions,
subscriptionsEngine: this.features.subscriptionsEngine,
schemaModel: this.schemaModel,
authorization: this.authorization,
jwtPayloadFieldsMap: this.jwtFieldsMap,
Expand Down Expand Up @@ -338,14 +338,20 @@ class Neo4jGraphQL {
private parseNeo4jFeatures(features: Neo4jFeaturesSettings | undefined): ContextFeatures {
let subscriptionPlugin: Neo4jGraphQLSubscriptionsEngine | undefined;
if (features?.subscriptions === true) {
subscriptionPlugin = new Neo4jGraphQLSubscriptionsDefaultEngine();
if (!this.driver) {
throw new Error("Driver required for CDC subscriptions");
}

subscriptionPlugin = new Neo4jGraphQLSubscriptionsCDCEngine({
driver: this.driver,
});
} else {
subscriptionPlugin = features?.subscriptions || undefined;
}

return {
...features,
subscriptions: subscriptionPlugin,
subscriptionsEngine: subscriptionPlugin,
};
}

Expand Down Expand Up @@ -485,7 +491,7 @@ class Neo4jGraphQL {
}

const setup = async () => {
const subscriptionsEngine = this.features?.subscriptions;
const subscriptionsEngine = this.features?.subscriptionsEngine;
if (subscriptionsEngine) {
subscriptionsEngine.events.setMaxListeners(0); // Removes warning regarding leak. >10 listeners are expected
if (subscriptionsEngine.init) {
Expand Down
4 changes: 2 additions & 2 deletions packages/graphql/src/schema/make-augmented-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ import { attributeAdapterToComposeFields, graphqlDirectivesToCompose } from "./t
// GraphQL type imports
import type { GraphQLToolsResolveMethods } from "graphql-compose/lib/SchemaComposer";
import type { Subgraph } from "../classes/Subgraph";
import { Neo4jGraphQLSubscriptionsCDCEngine } from "../classes/subscription/Neo4jGraphQLSubscriptionsCDCEngine";
import { SHAREABLE } from "../constants";
import { CreateInfo } from "../graphql/objects/CreateInfo";
import { DeleteInfo } from "../graphql/objects/DeleteInfo";
Expand Down Expand Up @@ -294,7 +293,8 @@ function makeAugmentedSchema({
});

if (features?.subscriptions && nodes.length) {
const isCDCEngine = features.subscriptions instanceof Neo4jGraphQLSubscriptionsCDCEngine;
// TODO: Update CDC
const isCDCEngine = true;
generateSubscriptionTypes({
schemaComposer: composer,
schemaModel,
Expand Down
2 changes: 1 addition & 1 deletion packages/graphql/src/schema/resolvers/mutation/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export function createResolver({
info,
});

publishEventsToSubscriptionMechanism(executeResult, context.features?.subscriptions, context.schemaModel);
publishEventsToSubscriptionMechanism(executeResult, context.features?.subscriptionsEngine, context.schemaModel);

const nodeProjection = info.fieldNodes[0]?.selectionSet?.selections.find(
(selection): selection is FieldNode =>
Expand Down
2 changes: 1 addition & 1 deletion packages/graphql/src/schema/resolvers/mutation/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export function deleteResolver({
info,
});

publishEventsToSubscriptionMechanism(executeResult, context.features?.subscriptions, context.schemaModel);
publishEventsToSubscriptionMechanism(executeResult, context.features?.subscriptionsEngine, context.schemaModel);

return executeResult.statistics;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/graphql/src/schema/resolvers/mutation/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export function updateResolver({
info,
});

publishEventsToSubscriptionMechanism(executeResult, context.features?.subscriptions, context.schemaModel);
publishEventsToSubscriptionMechanism(executeResult, context.features?.subscriptionsEngine, context.schemaModel);

const nodeProjection = info.fieldNodes[0]?.selectionSet?.selections.find(
(selection): selection is FieldNode =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export function generateSubscribeMethod({
});
}

// TODO: Remove
if (["create_relationship", "delete_relationship"].includes(type)) {
return filterAsyncIterator<SubscriptionsEvent[]>(iterable, (data) => {
if (!isRelationshipSubscriptionEvent(data[0])) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import type {
TypeNode,
UnionTypeDefinitionNode,
} from "graphql";
import { extendSchema, GraphQLSchema, Kind, specifiedDirectives, validateSchema } from "graphql";
import { GraphQLSchema, Kind, extendSchema, specifiedDirectives, validateSchema } from "graphql";
import { specifiedSDLRules } from "graphql/validation/specifiedRules";
import pluralize from "pluralize";
import * as directives from "../../graphql/directives";
Expand Down
10 changes: 4 additions & 6 deletions packages/graphql/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type { Directive } from "graphql-compose";
import type { ResolveTree } from "graphql-parse-resolve-info";
import type { JWTVerifyOptions, RemoteJWKSetOptions } from "jose";
import type { Integer } from "neo4j-driver";
import type { Neo4jGraphQLSubscriptionsCDCEngine } from "../classes/subscription/Neo4jGraphQLSubscriptionsCDCEngine";
import type { RelationshipNestedOperationsOption, RelationshipQueryDirectionOption } from "../constants";
import type { Neo4jGraphQLSchemaModel } from "../schema-model/Neo4jGraphQLSchemaModel";
import type { DefaultAnnotationValue } from "../schema-model/annotation/DefaultAnnotation";
Expand Down Expand Up @@ -470,7 +471,7 @@ export type Neo4jFeaturesSettings = {
filters?: Neo4jFiltersSettings;
populatedBy?: Neo4jPopulatedBySettings;
authorization?: Neo4jAuthorizationSettings;
subscriptions?: Neo4jGraphQLSubscriptionsEngine | boolean;
subscriptions?: boolean | Neo4jGraphQLSubscriptionsCDCEngine;
/** If set to `true`, removes `@neo4j/graphql` fields that are marked as deprecated to reduce schema size.
*
* NOTE: this will not remove user defined deprecated fields
Expand All @@ -485,11 +486,8 @@ export type Neo4jFeaturesSettings = {
};

/** Parsed features used in context */
export type ContextFeatures = {
filters?: Neo4jFiltersSettings;
populatedBy?: Neo4jPopulatedBySettings;
authorization?: Neo4jAuthorizationSettings;
subscriptions?: Neo4jGraphQLSubscriptionsEngine;
export type ContextFeatures = Neo4jFeaturesSettings & {
subscriptionsEngine?: Neo4jGraphQLSubscriptionsEngine;
};

export type PredicateReturn = {
Expand Down
2 changes: 1 addition & 1 deletion packages/graphql/tests/e2e/setup/ws-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export class WebSocketTestClient {
};
const timeout = setTimeout(() => {
this.eventsEmitter.removeListener(NEW_EVENT, newEventListener);
reject("Timed out.");
reject("[waitForEvents] Timed out.");
}, 500);
this.eventsEmitter.on(NEW_EVENT, newEventListener);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,43 +17,23 @@
* limitations under the License.
*/

import type { Driver } from "neo4j-driver";
import type { Response } from "supertest";
import supertest from "supertest";
import { Neo4jGraphQLSubscriptionsCDCEngine, type Neo4jGraphQLSubscriptionsEngine } from "../../../../src";
import { Neo4jGraphQLSubscriptionsDefaultEngine } from "../../../../src/classes/subscription/Neo4jGraphQLSubscriptionsDefaultEngine";
import { createBearerToken } from "../../../utils/create-bearer-token";
import type { UniqueType } from "../../../utils/graphql-types";
import { TestHelper } from "../../../utils/tests-helper";
import type { TestGraphQLServer } from "../../setup/apollo-server";
import { ApolloTestServer } from "../../setup/apollo-server";
import { WebSocketTestClient } from "../../setup/ws-client";

describe.each([
{
name: "Neo4jGraphQLSubscriptionsDefaultEngine",
engine: (_driver: Driver, _db: string) => new Neo4jGraphQLSubscriptionsDefaultEngine(),
},
{
name: "Neo4jGraphQLSubscriptionsCDCEngine",
engine: (driver: Driver, db: string) =>
new Neo4jGraphQLSubscriptionsCDCEngine({
driver,
pollTime: 100,
queryConfig: {
database: db,
},
}),
},
])("$name - Subscription authentication roles", ({ engine }) => {
describe("Subscription authentication roles", () => {
const testHelper = new TestHelper({ cdc: true });

let typeMovie: UniqueType;

let jwtToken: string;
let server: TestGraphQLServer;
let wsClient: WebSocketTestClient;
let subscriptionEngine: Neo4jGraphQLSubscriptionsEngine;

let typeDefs: string;

Expand All @@ -74,15 +54,13 @@ describe.each([

jwtToken = createBearerToken("secret", { roles: ["admin"] });

const driver = await testHelper.getDriver();
subscriptionEngine = engine(driver, testHelper.database);
const neoSchema = await testHelper.initNeo4jGraphQL({
typeDefs,
features: {
authorization: {
key: "secret",
},
subscriptions: subscriptionEngine,
subscriptions: await testHelper.getSubscriptionEngine(),
},
});

Expand All @@ -102,7 +80,6 @@ describe.each([

afterAll(async () => {
await server.close();
subscriptionEngine.close();
await testHelper.close();
});

Expand Down
Loading