fix: key persisted queries by the hash the client actually sends

.changeset/fix-percent-in-artifact.md

@@ -0,0 +1,6 @@
+---
+'houdini': patch
+'houdini-core': patch
+---
+
+Fix generated artifacts being corrupted when a document contains a `%` inside a string literal.

.changeset/fix-persisted-queries-hash.md

@@ -0,0 +1,6 @@
+---
+'houdini': patch
+'houdini-core': patch
+---
+
+Fix persisted query hashes for queries containing fragments.

packages/houdini-core/plugin/documents/artifacts/merge.go

@@ -40,9 +40,9 @@ func FlattenSelection(
 	return fields.ToSelectionSet(), nil
 }
 
-// sortedKeys returns the keys of a set in a stable (alphabetical) order so walking
+// sortedKeys returns the keys of a map in a stable (alphabetical) order so walking
 // them produces the same result on every run
-func sortedKeys(set map[string]bool) []string {
+func sortedKeys[V any](set map[string]V) []string {
 	keys := make([]string, 0, len(set))
 	for key := range set {
 		keys = append(keys, key)

packages/houdini-core/plugin/documents/artifacts/print.go

@@ -439,3 +439,19 @@ func stringifyValue(value *collected.ArgumentValue, usedVariables map[string]boo
 func generateDocumentHash(content string) string {
 	return fmt.Sprintf("%x", sha256.Sum256([]byte(content)))
 }
+
+// PrintWireDocument joins a document with the printed definitions of every fragment it
+// references (keyed by name) into the text that travels over the wire, plus the hash of that
+// text trimmed of its trailing newline. The artifact embeds the text as `raw` and the
+// persisted queries file stores it under the hash, so both have to come from here or they
+// drift apart.
+func PrintWireDocument(printedByName map[string]string) (document string, hash string) {
+	bodies := make([]string, 0, len(printedByName))
+	for _, name := range sortedKeys(printedByName) {
+		bodies = append(bodies, printedByName[name])
+	}
+
+	printed := strings.TrimSpace(strings.Join(bodies, "\n\n"))
+
+	return printed + "\n", generateDocumentHash(printed)
+}

packages/houdini-core/plugin/documents/artifacts/selection.go

@@ -2,7 +2,6 @@ package artifacts
 
 import (
 	"context"
-	"crypto/sha256"
 	"encoding/json"
 	"fmt"
 	"sort"
@@ -482,7 +481,7 @@ const artifact = {
     "name": "%s",
     "kind": "%s",
     "hash": "%s",%s
-    "raw": `+"`"+printed+"\n`"+`,
+    "raw": `+"`%s`"+`,
 
     "rootType": "%s",
     "stripVariables": %s as Array<string>,
@@ -503,6 +502,7 @@ export default artifact
 		kind,
 		hash,
 		refetch,
+		printed,
 		doc.TypeCondition,
 		string(stripVariables),
 		selectionValues,
@@ -548,32 +548,25 @@ func getDocumentData(
 	query, err := conn.Prepare(fmt.Sprintf(`
     SELECT
       documents.printed,
-      documents.name,
-      documents.hash
+      documents.name
     FROM documents
     WHERE documents.name in (%s)
-    ORDER BY documents.name
   `, whereIn))
 	if err != nil {
 		return d, err
 	}
 	defer query.Finalize()
 
-	var printedBuilder strings.Builder
+	printedByName := map[string]string{}
 
 	err = db.StepStatement(ctx, query, func() {
-		printedBuilder.WriteString(query.GetText("printed"))
-		printedBuilder.WriteString("\n\n")
+		printedByName[query.GetText("name")] = query.GetText("printed")
 	})
 	if err != nil {
 		return d, err
 	}
 
-	// strip the trailing newlines
-	d.Printed = strings.TrimSpace(printedBuilder.String())
-
-	// compute hash based on the complete printed content (including dependencies)
-	d.Hash = fmt.Sprintf("%x", sha256.Sum256([]byte(d.Printed)))
+	d.Printed, d.Hash = PrintWireDocument(printedByName)
 
 	// get refetch data from collected document if available
 	if collectedDoc := docs.Selections[name]; collectedDoc != nil {

packages/houdini-core/plugin/documents/artifacts/selection_test.go

@@ -5129,6 +5129,16 @@ export type LoginForm$artifact = typeof artifact
 					"NullLiteralQuery": "const artifact = {\n    \"name\": \"NullLiteralQuery\",\n    \"kind\": \"HoudiniQuery\",\n    \"hash\": \"b168d64e381365250361866d0735e605dd687b515c90b497da2f60b02129fbe5\",\n\n    \"refetch\": {\n        \"path\": [\"users\"],\n        \"method\": \"offset\",\n        \"pageSize\": 0,\n        \"embedded\": false,\n        \"targetType\": \"Query\",\n        \"paginated\": false,\n        \"direction\": \"forward\",\n        \"mode\": \"Infinite\"\n    },\n\n    \"raw\": `query NullLiteralQuery($name: String = null) {\n    users(filter: {name: null}, intValue: null, stringValue: $name) {\n        firstName\n        __typename\n        id\n    }\n}\n`,\n\n    \"rootType\": \"Query\",\n    \"stripVariables\": [] as Array<string>,\n\n    \"selection\": {\n        \"fields\": {\n            \"users\": {\n                \"type\": \"User\",\n                \"keyRaw\": \"users(filter: {name: null}, intValue: null, stringValue: $name)\",\n\n                \"directives\": [{\n                    \"name\": \"list\",\n                    \"arguments\": {\n                        \"name\": {\n                            \"kind\": \"StringValue\",\n                            \"value\": \"Null_Users\"\n                        }\n                    }\n                }],\n\n                \"list\": {\n                    \"name\": \"Null_Users\",\n                    \"connection\": false,\n                    \"type\": \"User\"\n                },\n\n                \"selection\": {\n                    \"fields\": {\n                        \"__typename\": {\n                            \"type\": \"String\",\n                            \"keyRaw\": \"__typename\",\n                        },\n\n                        \"firstName\": {\n                            \"type\": \"String\",\n                            \"keyRaw\": \"firstName\",\n                            \"visible\": true,\n                        },\n\n                        \"id\": {\n                            \"type\": \"ID\",\n                            \"keyRaw\": \"id\",\n                        },\n                    },\n                },\n\n                \"filters\": {\n                    \"filter\": {\n                        \"kind\": \"Object\",\n                        \"value\": {\n                            \"name\": {\n                                \"kind\": \"Null\",\n                                \"value\": null\n                            }\n                        }\n                    },\n                    \"intValue\": {\n                        \"kind\": \"Null\",\n                        \"value\": null\n                    },\n                    \"stringValue\": {\n                        \"kind\": \"Variable\",\n                        \"value\": \"name\"\n                    },\n                },\n                \"visible\": true,\n            },\n        },\n    },\n\n    \"pluginData\": {},\n\n    \"input\": {\n        \"fields\": {\n            \"name\": \"String\",\n        },\n\n        \"types\": {},\n\n        \"defaults\": {\n            \"name\": null,\n        },\n\n        \"runtimeScalars\": {},\n    },\n\n    \"policy\": \"CacheOrNetwork\",\n    \"partial\": false\n} as const\n\nexport default artifact\n\nexport type NullLiteralQuery = {\n\treadonly \"input\": NullLiteralQuery$input;\n\treadonly \"result\": NullLiteralQuery$result | undefined;\n};\n\nexport type NullLiteralQuery$result = {\n\treadonly users: ({\n\t\treadonly firstName: string;\n\t})[];\n};\n\nexport type NullLiteralQuery$input = {\n\tname?: string | null;\n};\n\nexport type NullLiteralQuery$unmasked = {\n\treadonly users: ({\n\t\treadonly __typename: \"User\";\n\t\treadonly firstName: string;\n\t\treadonly id: string;\n\t})[];\n};\n\nexport type NullLiteralQuery$artifact = typeof artifact\n\n\"HoudiniHash=b168d64e381365250361866d0735e605dd687b515c90b497da2f60b02129fbe5\"",
 				},
 			},
+			{
+				Name: "percent in a string literal",
+				Pass: true,
+				Input: []string{
+					`query PercentQuery { user { field(filter: "100%") } }`,
+				},
+				Extra: map[string]any{
+					"PercentQuery": "const artifact = {\n    \"name\": \"PercentQuery\",\n    \"kind\": \"HoudiniQuery\",\n    \"hash\": \"f7405bc779e404e2cf401d43970f163098a1fb64b3467ed16cea305bd6133033\",\n    \"raw\": `query PercentQuery {\n    user {\n        field(filter: \"100%\")\n        __typename\n        id\n    }\n}\n`,\n\n    \"rootType\": \"Query\",\n    \"stripVariables\": [] as Array<string>,\n\n    \"selection\": {\n        \"fields\": {\n            \"user\": {\n                \"type\": \"User\",\n                \"keyRaw\": \"user\",\n\n                \"selection\": {\n                    \"fields\": {\n                        \"__typename\": {\n                            \"type\": \"String\",\n                            \"keyRaw\": \"__typename\",\n                        },\n\n                        \"field\": {\n                            \"type\": \"String\",\n                            \"keyRaw\": \"field(filter: \\\"100%\\\")\",\n                            \"nullable\": true,\n                            \"visible\": true,\n                        },\n\n                        \"id\": {\n                            \"type\": \"ID\",\n                            \"keyRaw\": \"id\",\n                        },\n                    },\n                },\n\n                \"visible\": true,\n            },\n        },\n    },\n\n    \"pluginData\": {},\n    \"policy\": \"CacheOrNetwork\",\n    \"partial\": false\n} as const\n\nexport default artifact\n\nexport type PercentQuery = {\n\treadonly \"input\"?: PercentQuery$input;\n\treadonly \"result\": PercentQuery$result | undefined;\n};\n\nexport type PercentQuery$result = {\n\treadonly user: {\n\t\treadonly field: string | null;\n\t};\n};\n\nexport type PercentQuery$input = null | undefined;\n\nexport type PercentQuery$unmasked = {\n\treadonly user: {\n\t\treadonly __typename: \"User\";\n\t\treadonly field: string | null;\n\t\treadonly id: string;\n\t};\n};\n\nexport type PercentQuery$artifact = typeof artifact\n\n\"HoudiniHash=f7405bc779e404e2cf401d43970f163098a1fb64b3467ed16cea305bd6133033\"",
+				},
+			},
 		},
 	})
 }

packages/houdini-core/plugin/documents/generate.go

@@ -68,7 +68,7 @@ func Generate(
 	})
 
 	// generate the persisted queries file. this has to happen here (not in GenerateRuntime)
-	// because it reads documents.printed/hash which EnsureDocumentsPrinted populated above,
+	// because it reads documents.printed which EnsureDocumentsPrinted populated above,
 	// and GenerateRuntime runs in parallel with GenerateDocuments
 	group.Go(func() error {
 		files, err := GeneratePersistentQueries(ctx, db, fs)

packages/houdini-core/plugin/documents/persistentQueries.go

@@ -8,14 +8,12 @@ import (
 
 	"github.com/spf13/afero"
 
+	"code.houdinigraphql.com/packages/houdini-core/plugin/documents/artifacts"
 	"code.houdinigraphql.com/plugins"
 )
 
-type OperationDoc struct {
-	ID      string
+type operationDoc struct {
 	Name    string
-	Kind    string
-	Hash    string
 	Printed string
 }
 
@@ -44,39 +42,27 @@ func GeneratePersistentQueries(
 	queryMap := make(map[string]string)
 
 	// Get all operations (queries, mutations, subscriptions)
-	operations := make(map[string]*OperationDoc)
-	fragments := make(map[string]*OperationDoc)
+	operations := make(map[string]*operationDoc)
+	fragments := make(map[string]string)
 	err = db.StepQuery(ctx, `
-		SELECT d.id, d.name, d.kind, d.hash, d.printed
+		SELECT d.id, d.name, d.kind, d.printed
 		FROM documents d
-		WHERE  d.hash IS NOT NULL
-			AND d.hash != ''
-			AND d.printed IS NOT NULL
+		WHERE  d.printed IS NOT NULL
 			AND d.printed != ''
 	`, nil, func(stmt plugins.Row) {
 		id := stmt.ColumnText(0)
 		name := stmt.ColumnText(1)
 		kind := stmt.ColumnText(2)
-		hash := stmt.ColumnText(3)
-		printed := stmt.ColumnText(4)
+		printed := stmt.ColumnText(3)
 
 		if kind == "fragment" {
 			// named map for faster lookup
-			fragments[name] = &OperationDoc{
-				ID:      id,
-				Name:    name,
-				Kind:    kind,
-				Hash:    hash,
-				Printed: printed,
-			}
+			fragments[name] = printed
 			return
 		}
 
-		operations[id] = &OperationDoc{
-			ID:      id,
+		operations[id] = &operationDoc{
 			Name:    name,
-			Kind:    kind,
-			Hash:    hash,
 			Printed: printed,
 		}
 	})
@@ -111,9 +97,9 @@ func GeneratePersistentQueries(
 	}
 
 	// For each operation, BFS the fragment dependency graph to find the transitive set.
-	for _, op := range operations {
+	for id, op := range operations {
 		seen := make(map[string]bool)
-		queue := docToDirectFrags[op.ID]
+		queue := docToDirectFrags[id]
 		for len(queue) > 0 {
 			name := queue[0]
 			queue = queue[1:]
@@ -124,19 +110,18 @@ func GeneratePersistentQueries(
 			queue = append(queue, docToDirectFrags[name]...)
 		}
 
-		var fragmentDefinitions []string
+		printedByName := map[string]string{op.Name: op.Printed}
 		for name := range seen {
-			if frag := fragments[name]; frag != nil {
-				fragmentDefinitions = append(fragmentDefinitions, frag.Printed)
+			if printed, ok := fragments[name]; ok {
+				printedByName[name] = printed
 			}
 		}
 
-		completeGraphQL := op.Printed
-		if len(fragmentDefinitions) > 0 {
-			completeGraphQL += "\n\n" + strings.Join(fragmentDefinitions, "\n\n")
-		}
+		// the client sends the artifact's hash as the document id, so the entry has to be keyed
+		// by the hash of the whole document, operation and fragments together
+		document, hash := artifacts.PrintWireDocument(printedByName)
 
-		queryMap[op.Hash] = completeGraphQL
+		queryMap[hash] = document
 	}
 
 	if len(queryMap) == 0 {

packages/houdini-core/plugin/documents/persistentQueries_test.go

@@ -165,16 +165,7 @@ func TestPersistentQueriesArtifactHashConsistency(t *testing.T) {
 			// For each hash in persistent queries, find corresponding artifact and compare hash
 			for hash, query := range result {
 				// Extract operation name from the query to match artifact filename
-				opNameRegex := regexp.MustCompile(`(?:query|mutation|subscription)\s+(\w+)`)
-				matches := opNameRegex.FindStringSubmatch(query)
-				require.True(
-					t,
-					len(matches) >= 2,
-					"Should be able to extract operation name from: %s",
-					query,
-				)
-
-				operationName := matches[1]
+				operationName := mustCapture(t, operationNameRegex, query, "operation name")
 				artifactPath := filepath.Join(artifactDir, operationName+".ts")
 
 				// Read the artifact file
@@ -185,18 +176,13 @@ func TestPersistentQueriesArtifactHashConsistency(t *testing.T) {
 				artifactContent, err := afero.ReadFile(p.Fs, artifactPath)
 				require.NoError(t, err)
 
-				// Extract hash from artifact - it's in "hash": "value" format (64 hex chars for SHA256)
-				hashRegex := regexp.MustCompile(`"hash":\s*"([a-f0-9]{64})"`)
-				hashMatches := hashRegex.FindStringSubmatch(string(artifactContent))
-				require.True(
+				artifactHash := mustCapture(
 					t,
-					len(hashMatches) >= 2,
-					"Should be able to extract hash from artifact: %s",
-					artifactPath,
+					artifactHashRegex,
+					string(artifactContent),
+					"artifact hash",
 				)
 
-				artifactHash := hashMatches[1]
-
 				// Verify the hashes match
 				require.Equal(
 					t,
@@ -207,6 +193,22 @@ func TestPersistentQueriesArtifactHashConsistency(t *testing.T) {
 					artifactHash,
 					operationName,
 				)
+
+				// the file has to be usable as-is by a server: the value stored under the hash
+				// the client sends is the exact document the artifact holds in `raw`
+				artifactRaw := mustCapture(
+					t,
+					artifactRawRegex,
+					string(artifactContent),
+					"artifact raw",
+				)
+				require.Equal(
+					t,
+					artifactRaw,
+					query,
+					"Persisted query text should match the artifact's raw for operation %s",
+					operationName,
+				)
 			}
 		},
 		Tests: []tests.Test[config.PluginConfig]{
@@ -218,10 +220,46 @@ func TestPersistentQueriesArtifactHashConsistency(t *testing.T) {
 					`mutation ArtifactMutationTest { updateUser { id name } }`,
 				},
 			},
+			{
+				// a document that embeds a fragment definition is the case that used to drift:
+				// the artifact hashed the operation and its fragments together while the
+				// persisted file keyed the entry by the hash of the operation alone
+				Name: "Artifact hash consistency with fragments",
+				Pass: true,
+				Input: []string{
+					`
+					query ArtifactTest($id: ID!) {
+						user(id: $id) {
+							...ArtifactFragmentUserInfo
+						}
+					}
+
+					fragment ArtifactFragmentUserInfo on User {
+						name
+						email
+					}
+					`,
+				},
+			},
 		},
 	})
 }
 
+var (
+	operationNameRegex = regexp.MustCompile(`(?:query|mutation|subscription)\s+(\w+)`)
+	// a document hash is 64 hex characters of SHA256
+	artifactHashRegex = regexp.MustCompile(`"hash":\s*"([a-f0-9]{64})"`)
+	artifactRawRegex  = regexp.MustCompile("(?s)\"raw\": `(.*?)`,")
+)
+
+// mustCapture pulls the first capture group out of content, failing the test if the pattern
+// doesn't match
+func mustCapture(t *testing.T, pattern *regexp.Regexp, content string, what string) string {
+	matches := pattern.FindStringSubmatch(content)
+	require.Len(t, matches, 2, "Should be able to extract %s from: %s", what, content)
+	return matches[1]
+}
+
 func runFullGeneration(
 	t *testing.T,
 	p *plugin.HoudiniCore,