Remove direct execution feature flag - WIP (#19254)
Bug fixes exposed by always-on direct execution
1. GraphQL spec compliance — data[field] = null on resolver error
direct-execution.service.ts — Changed from Promise.allSettled (which
lost the responseKey on rejection) to Promise.all with per-field
try/catch; errors now set data[responseKey] = null per spec
2. Empty object arguments skipped (extractArgumentsFromAst)
extract-arguments-from-ast.util.ts — Removed isEmptyObject check;
filter: {}, data: {} now correctly passed to resolvers instead of
silently dropped (which caused permissions to never be checked)
3. orderBy: {} factory default treated as "no ordering"
direct-execution.service.ts — Before calling the resolver, strips
orderBy: {} and orderByForRecords: {} (empty-object factory defaults
that mean "no ordering")
assert-find-many-args.util.ts / assert-group-by-args.util.ts — Accept {}
for orderBy without throwing
4. orderBy: { field: '...' } object auto-coerced to [{ field: '...' }]
array
direct-execution.service.ts — Applies GraphQL list coercion: a
non-array, non-empty orderBy object is wrapped in an array before
assertion and resolver call
5. totalCount and aggregate fields returned as strings from PostgreSQL
graphql-format-result-from-selected-fields.util.ts — Added
coerceAggregateValue that parses numeric strings to numbers for
totalCount, sum*, avg*, min*, max*, count*, percentageOf* fields
Test updates
nested-relation-queries.integration-spec.ts — Updated expected error
message from Yoga schema-validation message to direct execution resolver
message
~30 snapshot files — Updated to reflect direct execution's error
messages (different from Yoga schema-validation messages for input type
errors)
This commit is contained in:
+51
-72
@@ -203,74 +203,73 @@ export class DirectExecutionService {
|
||||
const variables = req.body.variables ?? {};
|
||||
const data: Record<string, unknown> = {};
|
||||
|
||||
const { graphQLResolverNameMap } =
|
||||
await this.workspaceCacheService.getOrRecompute(workspaceId, [
|
||||
'graphQLResolverNameMap',
|
||||
]);
|
||||
|
||||
const {
|
||||
graphQLResolverNameMap,
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
objectIdByNameSingular,
|
||||
} = await this.loadWorkspaceMetadata(workspaceId);
|
||||
} = await this.workspaceCacheService.getOrRecompute(workspaceId, [
|
||||
'graphQLResolverNameMap',
|
||||
'flatObjectMetadataMaps',
|
||||
'flatFieldMetadataMaps',
|
||||
]);
|
||||
|
||||
const results = await Promise.allSettled(
|
||||
const { idByNameSingular: objectIdByNameSingular } =
|
||||
buildObjectIdByNameMaps(flatObjectMetadataMaps);
|
||||
|
||||
const errors: GraphQLFormattedError[] = [];
|
||||
|
||||
await Promise.all(
|
||||
topLevelFields.map(async (field) => {
|
||||
const entry = graphQLResolverNameMap[field.name.value];
|
||||
const responseKey = field.alias?.value ?? field.name.value;
|
||||
|
||||
const args = extractArgumentsFromAst(field.arguments, variables);
|
||||
try {
|
||||
const args = extractArgumentsFromAst(field.arguments, variables);
|
||||
|
||||
const graphqlPartialResolveInfo = graphQLBuildPartialResolveInfo(
|
||||
field,
|
||||
fragmentMap,
|
||||
);
|
||||
|
||||
const workspaceSchemaBuilderContext =
|
||||
buildWorkspaceSchemaBuilderContext(
|
||||
entry,
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
objectIdByNameSingular,
|
||||
const graphqlPartialResolveInfo = graphQLBuildPartialResolveInfo(
|
||||
field,
|
||||
fragmentMap,
|
||||
);
|
||||
|
||||
const result = (await this.executeField({
|
||||
entry,
|
||||
args,
|
||||
graphqlPartialResolveInfo,
|
||||
workspaceSchemaBuilderContext,
|
||||
})) as ResolverOutput;
|
||||
const workspaceSchemaBuilderContext =
|
||||
buildWorkspaceSchemaBuilderContext(
|
||||
entry,
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
objectIdByNameSingular,
|
||||
);
|
||||
|
||||
const formattedResult = graphQLFormatResultFromSelectedFields(
|
||||
result,
|
||||
graphqlFields(
|
||||
graphqlPartialResolveInfo as GraphQLResolveInfo,
|
||||
{},
|
||||
{ excludedFields: [] },
|
||||
),
|
||||
workspaceSchemaBuilderContext.flatObjectMetadata.nameSingular,
|
||||
{
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
objectIdByNameSingular,
|
||||
method: entry.method,
|
||||
},
|
||||
);
|
||||
const result = (await this.executeField({
|
||||
entry,
|
||||
args,
|
||||
graphqlPartialResolveInfo,
|
||||
workspaceSchemaBuilderContext,
|
||||
})) as ResolverOutput;
|
||||
|
||||
return { responseKey, result: formattedResult };
|
||||
const formattedResult = graphQLFormatResultFromSelectedFields(
|
||||
result,
|
||||
graphqlFields(
|
||||
graphqlPartialResolveInfo as GraphQLResolveInfo,
|
||||
{},
|
||||
{ excludedFields: [] },
|
||||
),
|
||||
workspaceSchemaBuilderContext.flatObjectMetadata.nameSingular,
|
||||
{
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
objectIdByNameSingular,
|
||||
method: entry.method,
|
||||
},
|
||||
);
|
||||
|
||||
data[responseKey] = formattedResult;
|
||||
} catch (error) {
|
||||
data[responseKey] = null;
|
||||
errors.push(this.formatError(error, req));
|
||||
}
|
||||
}),
|
||||
);
|
||||
|
||||
const errors: GraphQLFormattedError[] = [];
|
||||
|
||||
for (const settled of results) {
|
||||
if (settled.status === 'fulfilled') {
|
||||
data[settled.value.responseKey] = settled.value.result;
|
||||
} else {
|
||||
errors.push(this.formatError(settled.reason, req));
|
||||
}
|
||||
}
|
||||
|
||||
if (errors.length > 0) {
|
||||
return { data, errors };
|
||||
}
|
||||
@@ -440,24 +439,4 @@ export class DirectExecutionService {
|
||||
seen.add(name);
|
||||
}
|
||||
}
|
||||
|
||||
private async loadWorkspaceMetadata(workspaceId: string) {
|
||||
const { flatObjectMetadataMaps, flatFieldMetadataMaps } =
|
||||
await this.workspaceFlatEntityMapsCacheService.getOrRecomputeManyOrAllFlatEntityMaps(
|
||||
{
|
||||
workspaceId,
|
||||
flatMapsKeys: ['flatObjectMetadataMaps', 'flatFieldMetadataMaps'],
|
||||
},
|
||||
);
|
||||
|
||||
const { idByNameSingular } = buildObjectIdByNameMaps(
|
||||
flatObjectMetadataMaps,
|
||||
);
|
||||
|
||||
return {
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
objectIdByNameSingular: idByNameSingular,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
-11
@@ -1,7 +1,6 @@
|
||||
import { type Request } from 'express';
|
||||
import { DocumentNode, parse } from 'graphql';
|
||||
import { type Plugin } from 'graphql-yoga';
|
||||
import { FeatureFlagKey } from 'twenty-shared/types';
|
||||
|
||||
import { isNull } from '@sniptt/guards';
|
||||
import { type DirectExecutionService } from 'src/engine/api/graphql/direct-execution/direct-execution.service';
|
||||
@@ -26,16 +25,6 @@ export function useDirectExecution(
|
||||
return;
|
||||
}
|
||||
|
||||
const isDirectExecutionEnabled =
|
||||
await config.featureFlagService.isFeatureEnabled(
|
||||
FeatureFlagKey.IS_DIRECT_GRAPHQL_EXECUTION_ENABLED,
|
||||
req.workspace.id,
|
||||
);
|
||||
|
||||
if (!isDirectExecutionEnabled) {
|
||||
return;
|
||||
}
|
||||
|
||||
const queryString = req.body.query as string;
|
||||
const operationName = req.body.operationName as string | undefined;
|
||||
|
||||
|
||||
+1
-1
@@ -1,10 +1,10 @@
|
||||
import { isObject } from 'class-validator';
|
||||
|
||||
import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant';
|
||||
import {
|
||||
GraphqlDirectExecutionException,
|
||||
GraphqlDirectExecutionExceptionCode,
|
||||
} from 'src/engine/api/graphql/direct-execution/errors/graphql-direct-execution.exception';
|
||||
import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant';
|
||||
import { type DeleteManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface';
|
||||
|
||||
export function assertDeleteManyArgs(
|
||||
|
||||
+1
-1
@@ -1,10 +1,10 @@
|
||||
import { isObject } from 'class-validator';
|
||||
|
||||
import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant';
|
||||
import {
|
||||
GraphqlDirectExecutionException,
|
||||
GraphqlDirectExecutionExceptionCode,
|
||||
} from 'src/engine/api/graphql/direct-execution/errors/graphql-direct-execution.exception';
|
||||
import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant';
|
||||
import { type DestroyManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface';
|
||||
|
||||
export function assertDestroyManyArgs(
|
||||
|
||||
+8
-2
@@ -1,6 +1,6 @@
|
||||
import { isArray, isNumber, isObject, isString } from 'class-validator';
|
||||
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { isDefined, isEmptyObject } from 'twenty-shared/utils';
|
||||
|
||||
import {
|
||||
GraphqlDirectExecutionException,
|
||||
@@ -66,7 +66,13 @@ export function assertFindManyArgs(
|
||||
);
|
||||
}
|
||||
|
||||
if ('orderBy' in args && isDefined(args.orderBy) && !isArray(args.orderBy)) {
|
||||
if (
|
||||
'orderBy' in args &&
|
||||
isDefined(args.orderBy) &&
|
||||
!isEmptyObject(args.orderBy) &&
|
||||
!isArray(args.orderBy) &&
|
||||
!isObject(args.orderBy)
|
||||
) {
|
||||
throw new GraphqlDirectExecutionException(
|
||||
'Invalid argument: "orderBy" must be an array',
|
||||
GraphqlDirectExecutionExceptionCode.INVALID_QUERY_INPUT,
|
||||
|
||||
+1
-1
@@ -2,11 +2,11 @@ import { isObject } from 'class-validator';
|
||||
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant';
|
||||
import {
|
||||
GraphqlDirectExecutionException,
|
||||
GraphqlDirectExecutionExceptionCode,
|
||||
} from 'src/engine/api/graphql/direct-execution/errors/graphql-direct-execution.exception';
|
||||
import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant';
|
||||
import { type FindOneResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface';
|
||||
|
||||
export function assertFindOneArgs(
|
||||
|
||||
+11
-3
@@ -6,7 +6,7 @@ import {
|
||||
isString,
|
||||
} from 'class-validator';
|
||||
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { isDefined, isEmptyObject } from 'twenty-shared/utils';
|
||||
|
||||
import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant';
|
||||
import {
|
||||
@@ -68,7 +68,13 @@ export function assertGroupByArgs(
|
||||
);
|
||||
}
|
||||
|
||||
if ('orderBy' in args && isDefined(args.orderBy) && !isArray(args.orderBy)) {
|
||||
if (
|
||||
'orderBy' in args &&
|
||||
isDefined(args.orderBy) &&
|
||||
!isEmptyObject(args.orderBy) &&
|
||||
!isArray(args.orderBy) &&
|
||||
!isObject(args.orderBy)
|
||||
) {
|
||||
throw new GraphqlDirectExecutionException(
|
||||
'Invalid argument: "orderBy" must be an array',
|
||||
GraphqlDirectExecutionExceptionCode.INVALID_QUERY_INPUT,
|
||||
@@ -79,7 +85,9 @@ export function assertGroupByArgs(
|
||||
if (
|
||||
'orderByForRecords' in args &&
|
||||
isDefined(args.orderByForRecords) &&
|
||||
!isArray(args.orderByForRecords)
|
||||
!isEmptyObject(args.orderByForRecords) &&
|
||||
!isArray(args.orderByForRecords) &&
|
||||
!isObject(args.orderByForRecords)
|
||||
) {
|
||||
throw new GraphqlDirectExecutionException(
|
||||
'Invalid argument: "orderByForRecords" must be an array',
|
||||
|
||||
+1
-1
@@ -1,10 +1,10 @@
|
||||
import { isObject } from 'class-validator';
|
||||
|
||||
import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant';
|
||||
import {
|
||||
GraphqlDirectExecutionException,
|
||||
GraphqlDirectExecutionExceptionCode,
|
||||
} from 'src/engine/api/graphql/direct-execution/errors/graphql-direct-execution.exception';
|
||||
import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant';
|
||||
import { type RestoreManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface';
|
||||
|
||||
export function assertRestoreManyArgs(
|
||||
|
||||
+1
-1
@@ -1,10 +1,10 @@
|
||||
import { isObject } from 'class-validator';
|
||||
|
||||
import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant';
|
||||
import {
|
||||
GraphqlDirectExecutionException,
|
||||
GraphqlDirectExecutionExceptionCode,
|
||||
} from 'src/engine/api/graphql/direct-execution/errors/graphql-direct-execution.exception';
|
||||
import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant';
|
||||
import { type UpdateManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface';
|
||||
|
||||
export function assertUpdateManyArgs(
|
||||
|
||||
+4
-3
@@ -1,5 +1,5 @@
|
||||
import { type ArgumentNode, valueFromASTUntyped } from 'graphql';
|
||||
import { isDefined, isEmptyObject } from 'twenty-shared/utils';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
// Converts GraphQL AST argument nodes into a plain JS object,
|
||||
// resolving variable references from the variables map.
|
||||
@@ -15,8 +15,9 @@ export const extractArgumentsFromAst = (
|
||||
|
||||
for (const arg of argumentNodes) {
|
||||
const value = valueFromASTUntyped(arg.value, variables);
|
||||
if (!isDefined(value) || isEmptyObject(value)) continue;
|
||||
result[arg.name.value] = valueFromASTUntyped(arg.value, variables);
|
||||
|
||||
if (!isDefined(value)) continue;
|
||||
result[arg.name.value] = value;
|
||||
}
|
||||
|
||||
return result;
|
||||
|
||||
+22
-4
@@ -1,5 +1,9 @@
|
||||
import { isNull } from '@sniptt/guards';
|
||||
import { ObjectRecord, RelationType } from 'twenty-shared/types';
|
||||
import { isNonEmptyString, isNull } from '@sniptt/guards';
|
||||
import {
|
||||
FieldMetadataType,
|
||||
ObjectRecord,
|
||||
RelationType,
|
||||
} from 'twenty-shared/types';
|
||||
|
||||
import { STANDARD_ERROR_MESSAGE } from 'src/engine/api/common/common-query-runners/errors/standard-error-message.constant';
|
||||
import {
|
||||
@@ -181,6 +185,16 @@ const backfillNullValuesAndComputeTypeNameForObjectRecord = (
|
||||
continue;
|
||||
}
|
||||
|
||||
if (
|
||||
isDefined(fieldMetadata) &&
|
||||
fieldMetadata.type === FieldMetadataType.NUMBER &&
|
||||
isNonEmptyString(value) &&
|
||||
isFinite(Number(value))
|
||||
) {
|
||||
formatted[key] = Number(value);
|
||||
continue;
|
||||
}
|
||||
|
||||
formatted[key] = value;
|
||||
}
|
||||
|
||||
@@ -275,9 +289,13 @@ const backfillNullValuesAndComputeTypeNameForConnection = (
|
||||
continue;
|
||||
}
|
||||
|
||||
//aggregate fields
|
||||
formatted[key] =
|
||||
const rawAggregateValue =
|
||||
(connection as unknown as Record<string, unknown>)[key] ?? null;
|
||||
|
||||
formatted[key] =
|
||||
isNonEmptyString(rawAggregateValue) && isFinite(Number(rawAggregateValue))
|
||||
? Number(rawAggregateValue)
|
||||
: rawAggregateValue;
|
||||
}
|
||||
|
||||
return formatted;
|
||||
|
||||
Reference in New Issue
Block a user