Stage 1 of behavior overhaul (#2160)

.changeset/good-rocks-deny.md

@@ -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.

.changeset/modern-bananas-drop.md

@@ -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"]`.

.changeset/serious-cheetahs-dream.md

@@ -0,0 +1,6 @@
+---
+"graphile": patch
+---
+
+Fix interactions with behavior system, including fixing debugging behaviors when
+naming conflicts occur in the schema.

.changeset/wise-chefs-dance.md

@@ -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.

graphile-build/graphile-build-pg/src/behavior.ts

@@ -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
@@ -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);
@@ -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,
@@ -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;
     }
@@ -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,
-    )
-  );
-}

graphile-build/graphile-build-pg/src/index.ts

@@ -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 {
@@ -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 {

graphile-build/graphile-build-pg/src/plugins/PgAllRowsPlugin.ts

@@ -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`.
@@ -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 {

graphile-build/graphile-build-pg/src/plugins/PgAttributesPlugin.ts

@@ -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,
@@ -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];
+          },
         },
       },
     },