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

Stage 1 of behavior overhaul #2160

Merged
merged 51 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
9893ec3
graphile-config now supports lists of hook objects, to allow hooking …
benjie Aug 17, 2024
9cd9bb5
docs(changeset): Add support for lists of hook objects, so that the s…
benjie Aug 17, 2024
8a264e9
Type-safety around behaviors
benjie Aug 17, 2024
eca1ad6
Add descriptions and entities
benjie Aug 17, 2024
7f84b40
More descriptions
benjie Sep 24, 2024
4611bde
No connection for non-existant type
benjie Sep 24, 2024
ea6e6d1
Move into class
benjie Sep 24, 2024
22482ab
Move isValidBehaviorString to graphile-build
benjie Sep 24, 2024
9039b00
Warn when behaviors don't match the registry
benjie Sep 24, 2024
450c92d
Register more behaviors and fix unregistered behavior detection
benjie Sep 24, 2024
ab1d705
More behaviors registered
benjie Sep 24, 2024
b7f5f29
Behavior multiplication plugin
benjie Sep 24, 2024
82850b7
Must conform
benjie Sep 24, 2024
76f0444
-* prevents corruption
benjie Sep 24, 2024
421fcef
Add the parent behaviors too
benjie Sep 24, 2024
ec470d5
Add more missing behaviors, and fix behavior of connection-incapable …
benjie Sep 24, 2024
e070614
Implement new split inferred/override behavior system
benjie Sep 25, 2024
b368cb0
Getting combined behaviors for entities
benjie Sep 25, 2024
4a9ca7a
Fix PgPolymorphismPlugin
benjie Sep 25, 2024
6639bd7
Cleanup and fix default behavior
benjie Sep 25, 2024
2a3d6bd
Fix bad git patch add edits
benjie Sep 25, 2024
6d96108
Add a bug to fix the bug caused by the bug
benjie Sep 25, 2024
00a7816
Register behavior on demand
benjie Sep 25, 2024
c360637
Fix implied behavior for refs
benjie Sep 25, 2024
5c3301e
Don't inherit global behavior
benjie Sep 25, 2024
004d4ed
Actually we do want to start with default behaviors otherwise you can…
benjie Sep 25, 2024
8d0d903
Fix lint
benjie Sep 25, 2024
3660ec8
Almost completely remove incorrect addBehaviorToTags usage
benjie Sep 26, 2024
2dd58e6
Remove from final place
benjie Sep 26, 2024
47fa837
Must not init build inside createBuild; messes with debug tools
benjie Sep 26, 2024
6a3a520
@enum should disable the codec too
benjie Sep 26, 2024
96aa437
Remove more smart tag shenanigans
benjie Sep 26, 2024
99e4903
Fix bug in V4 behavior plugin
benjie Sep 26, 2024
ce6efaa
Reflect new non-behavior-modding approach in schema exports
benjie Sep 26, 2024
4b3dd3f
User has explicitly marked these non-executable fields as filterable/…
benjie Sep 26, 2024
7f7c267
Lint
benjie Sep 26, 2024
426e932
docs(changeset): Massive overhaul of the behavior system which now ha…
benjie Sep 26, 2024
e720702
Fix preset default behavior - it should override plugins
benjie Sep 26, 2024
4215786
Fix behavior string validation, it should allow whitespace padding
benjie Sep 26, 2024
12aa3e0
Fix lint
benjie Sep 26, 2024
822085b
Don't duplicate definition
benjie Sep 26, 2024
34fcfbe
Merge branch 'main' into behavior-overhaul
benjie Sep 26, 2024
cde36fd
Update release notes
benjie Sep 27, 2024
54054b8
docs(changeset): Fix bug where creating the build object also initial…
benjie Sep 27, 2024
205cfe3
Stricter behaviors in more places
benjie Sep 27, 2024
b67e222
Register yet more behaviors
benjie Sep 27, 2024
a47794a
Fix bug
benjie Sep 27, 2024
68dc392
Fix accidental loss of '-'
benjie Sep 27, 2024
21f8988
Note about stricter typings
benjie Sep 27, 2024
979ab99
Fix types
benjie Sep 27, 2024
2dfc10f
docs(changeset): Fix interactions with behavior system, including fix…
benjie Sep 27, 2024
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
10 changes: 10 additions & 0 deletions .changeset/good-rocks-deny.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"graphile-build": patch
---

Fix bug where creating the build object also initialized it; this is incorrect
since if you just want the build object you don't necessarily want to register
all of the GraphQL types (and potentially discover naming conflicts) at that
moment. Introduced new
`schemaBuilder.initBuild(schemaBuilder.createBuild(input))` API to explicitly
handle initing if you need an initialized build object.
34 changes: 34 additions & 0 deletions .changeset/modern-bananas-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
"graphile-build-pg": patch
"graphile-build": patch
"postgraphile": patch
---

Massive overhaul of the behavior system which now has a centralized registry of
known behaviors and applies behaviors in a more careful and nuanced way,
removing many hacks and workarounds, and ultimately meaning that
`defaultBehavior: "-*"` should now operate correctly. Importantly,
`addBehaviorToTags()` has been removed - you should use
`plugin.schema.entityBehaviors` to indicate behaviors as shown in this PR - do
not mod the tags directly unless they're explicitly meant to be overrides.

Technically this is a significant breaking change (besides the removal of the
`addBehaviorToTags()` helper) because the order in which behaviors are applied
has changed, and so a different behavior might ultimately "win". This shows up
in places where there is ambiguity, for example if you add `@filterable` to a
function that you don't have execute permissions on, that function will now show
up in the schema since user overrides (smart tags) "win" versus inferred
behaviors such as introspected permissions; this wasn't the case before.
Hopefully most users will not notice any difference, and for those who do, the
`graphile behavior debug` CLI may be able to help you figure out what's going
on.

Be sure to print your schema before and after this update and look for changes;
if there are changes then you likely need to fix the relevant behaviors/smart
tags. (Hopefully there's no changes for you!)

You'll also need to change any places where you're specifying behaviors that
will be type checked; you can either cast your existing strings e.g.
`defaultBehavior: "+connection -list" as GraphileBuild.BehaviorString`, or
preferably you can specify your behaviors as an array, which should give you
auto-complete on each entry; e.g. `defaultBehavior: ["connection", "-list"]`.
6 changes: 6 additions & 0 deletions .changeset/serious-cheetahs-dream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"graphile": patch
---

Fix interactions with behavior system, including fixing debugging behaviors when
naming conflicts occur in the schema.
6 changes: 6 additions & 0 deletions .changeset/wise-chefs-dance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"graphile-config": patch
---

Add support for lists of hook objects, so that the same hook can be applied
multiple times in the same plugin but with different priorities.
32 changes: 9 additions & 23 deletions graphile-build/graphile-build-pg/src/behavior.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {
PgResourceExtensions,
} from "@dataplan/pg";
import { isDev } from "grafast";
import { isValidBehaviorString } from "graphile-build";
import { inspect } from "util";

// NOTE: 'behaviour' is the correct spelling in UK English; we try and stick to
Expand All @@ -26,9 +27,9 @@ export function getBehavior(
>
| undefined
>,
): string {
): GraphileBuild.BehaviorString {
const allExtensions = Array.isArray(extensions) ? extensions : [extensions];
const behaviors: string[] = [];
const behaviors: GraphileBuild.BehaviorString[] = [];
for (const extensions of allExtensions) {
// LOGGING: all of these are just for user convenience, users should be guided not to use them.
add(extensions?.tags?.behaviours);
Expand All @@ -38,7 +39,7 @@ export function getBehavior(
// This is the real one
add(extensions?.tags?.behavior);
}
return behaviors.join(" ");
return behaviors.join(" ") as GraphileBuild.BehaviorString;

function add(
rawBehavior: (string | true)[] | string | true | null | undefined,
Expand All @@ -49,17 +50,19 @@ export function getBehavior(
return;
}
if (Array.isArray(behavior)) {
if (isDev && !behavior.every(isValidBehavior)) {
if (isDev && !behavior.every(isValidBehaviorString)) {
throw new Error(
`Invalid value for behavior; expected a string or string array using simple alphanumeric strings, but found ${inspect(
behavior,
)}`,
);
}
behaviors.push(behavior.join(" "));
for (const b of behavior) {
behaviors.push(b as GraphileBuild.BehaviorString);
}
return;
}
if (isValidBehavior(behavior)) {
if (isValidBehaviorString(behavior)) {
behaviors.push(behavior);
return;
}
Expand All @@ -70,20 +73,3 @@ export function getBehavior(
);
}
}

/**
* We're strict with this because we want to be able to expand this in future.
* For example I want to allow `@behavior all some` to operate the same as
* `@behavior all\n@behavior some`. I also want to be able to add
* `@behavior -all` to remove a previously enabled behavior.
*
* @internal
*/
function isValidBehavior(behavior: unknown): behavior is string {
return (
typeof behavior === "string" &&
/^[+-]?([a-zA-Z](?:[_:]?[a-zA-Z0-9])+|\*)(?:\s+[+-]?(?:[a-zA-Z]([_:]?[a-zA-Z0-9])+|\*))*$/.test(
behavior,
)
);
}
9 changes: 4 additions & 5 deletions graphile-build/graphile-build-pg/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,7 @@ export { PgTableNodePlugin } from "./plugins/PgTableNodePlugin.js";
export { PgTablesPlugin } from "./plugins/PgTablesPlugin.js";
export { PgTypesPlugin } from "./plugins/PgTypesPlugin.js";
export { defaultPreset } from "./preset.js";
export {
addBehaviorToTags,
parseDatabaseIdentifier,
parseDatabaseIdentifiers,
} from "./utils.js";
export { parseDatabaseIdentifier, parseDatabaseIdentifiers } from "./utils.js";
export { version } from "./version.js";

declare global {
Expand All @@ -57,6 +53,9 @@ declare global {
deprecated: string | string[];
/** For functions returning polymorphic type, which type to choose? */
returnType: string;

/** For enum tables; we shouldn't expose these through GraphQL */
enum: string | true;
}

interface PgResourceUniqueTags extends PgSmartTagsDict {
Expand Down
18 changes: 18 additions & 0 deletions graphile-build/graphile-build-pg/src/plugins/PgAllRowsPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ declare global {
}

namespace GraphileBuild {
interface BehaviorStrings {
"query:resource:connection": true;
"query:resource:list": true;
}

interface Inflection {
/**
* The base inflector used by `allRowsConnection` and `allRowsList`.
Expand Down Expand Up @@ -69,6 +74,19 @@ export const PgAllRowsPlugin: GraphileConfig.Plugin = {
},

schema: {
behaviorRegistry: {
add: {
"query:resource:connection": {
description:
'"connection" field for a resource at the root Query level',
entities: ["pgCodec", "pgResource", "pgResourceUnique"],
},
"query:resource:list": {
description: '"list" field for a resource at the root Query level',
entities: ["pgCodec", "pgResource", "pgResourceUnique"],
},
},
},
hooks: {
GraphQLObjectType_fields(fields, build, context) {
const {
Expand Down
112 changes: 81 additions & 31 deletions graphile-build/graphile-build-pg/src/plugins/PgAttributesPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ declare global {
}

namespace GraphileBuild {
interface BehaviorStrings {
"condition:attribute:filterBy": true;
"attribute:select": true;
"attribute:base": true;
"attribute:insert": true;
"attribute:update": true;
"attribute:filterBy": true;
"attribute:orderBy": true;
}
interface Build {
pgResolveOutputType(
codec: PgCodec,
Expand Down Expand Up @@ -288,41 +297,82 @@ export const PgAttributesPlugin: GraphileConfig.Plugin = {
},

schema: {
behaviorRegistry: {
add: {
"attribute:select": {
description: "can this attribute be selected?",
entities: ["pgCodecAttribute"],
},
"attribute:insert": {
description: "can this attribute be written on create?",
entities: ["pgCodecAttribute"],
},
"attribute:update": {
description: "can this attribute be updated?",
entities: ["pgCodecAttribute"],
},
"attribute:base": {
description: "should we add this attribute to the 'base' input type?",
entities: ["pgCodecAttribute"],
},
"attribute:filterBy": {
description: "can we filter by this attribute?",
entities: ["pgCodecAttribute"],
},
"condition:attribute:filterBy": {
description:
"can we filter by this attribute in the `condition` argument?",
entities: ["pgCodecAttribute"],
},
"attribute:orderBy": {
description: "can we order by this attribute?",
entities: ["pgCodecAttribute"],
},
},
},

entityBehavior: {
pgCodecAttribute: {
provides: ["default"],
before: ["inferred", "override"],
callback(behavior, [codec, attributeName]) {
const behaviors = new Set([
"select base update insert filterBy orderBy",
]);
const attribute = codec.attributes[attributeName];
function walk(codec: PgCodec) {
if (codec.arrayOfCodec) {
behaviors.add("-condition:attribute:filterBy");
behaviors.add(`-attribute:orderBy`);
walk(codec.arrayOfCodec);
} else if (codec.rangeOfCodec) {
behaviors.add(`-condition:attribute:filterBy`);
behaviors.add(`-attribute:orderBy`);
walk(codec.rangeOfCodec);
} else if (codec.domainOfCodec) {
// No need to add a behavior for domain
walk(codec.domainOfCodec);
} else if (codec.attributes) {
behaviors.add(`-condition:attribute:filterBy`);
behaviors.add(`-attribute:orderBy`);
} else if (codec.isBinary) {
// Never filter, not in condition plugin nor any other
behaviors.add(`-attribute:filterBy`);
behaviors.add(`-attribute:orderBy`);
} else {
// Done
inferred: {
provides: ["default"],
before: ["inferred", "override"],
callback(behavior, [codec, attributeName]) {
const behaviors = new Set<GraphileBuild.BehaviorString>([
"select",
"base",
"update",
"insert",
"filterBy",
"orderBy",
]);
const attribute = codec.attributes[attributeName];
function walk(codec: PgCodec) {
if (codec.arrayOfCodec) {
behaviors.add("-condition:attribute:filterBy");
behaviors.add(`-attribute:orderBy`);
walk(codec.arrayOfCodec);
} else if (codec.rangeOfCodec) {
behaviors.add(`-condition:attribute:filterBy`);
behaviors.add(`-attribute:orderBy`);
walk(codec.rangeOfCodec);
} else if (codec.domainOfCodec) {
// No need to add a behavior for domain
walk(codec.domainOfCodec);
} else if (codec.attributes) {
behaviors.add(`-condition:attribute:filterBy`);
behaviors.add(`-attribute:orderBy`);
} else if (codec.isBinary) {
// Never filter, not in condition plugin nor any other
behaviors.add(`-attribute:filterBy`);
behaviors.add(`-attribute:orderBy`);
} else {
// Done
}
}
}
walk(attribute.codec);
walk(attribute.codec);

return [...behaviors, behavior];
return [...behaviors, behavior];
},
},
},
},
Expand Down
Loading
Loading