diff --git a/.changeset/fix-percent-in-artifact.md b/.changeset/fix-percent-in-artifact.md new file mode 100644 index 000000000..1bc8aeccb --- /dev/null +++ b/.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. diff --git a/.changeset/fix-persisted-queries-hash.md b/.changeset/fix-persisted-queries-hash.md new file mode 100644 index 000000000..452f8f564 --- /dev/null +++ b/.changeset/fix-persisted-queries-hash.md @@ -0,0 +1,6 @@ +--- +'houdini': patch +'houdini-core': patch +--- + +Fix persisted query hashes for queries containing fragments. diff --git a/packages/houdini-core/plugin/documents/artifacts/merge.go b/packages/houdini-core/plugin/documents/artifacts/merge.go index b6b004db0..c237fb973 100644 --- a/packages/houdini-core/plugin/documents/artifacts/merge.go +++ b/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) diff --git a/packages/houdini-core/plugin/documents/artifacts/print.go b/packages/houdini-core/plugin/documents/artifacts/print.go index 509c44b6b..1917ec44d 100644 --- a/packages/houdini-core/plugin/documents/artifacts/print.go +++ b/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) +} diff --git a/packages/houdini-core/plugin/documents/artifacts/selection.go b/packages/houdini-core/plugin/documents/artifacts/selection.go index 0b405ea57..1a6f3f0a1 100644 --- a/packages/houdini-core/plugin/documents/artifacts/selection.go +++ b/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, @@ -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 { diff --git a/packages/houdini-core/plugin/documents/artifacts/selection_test.go b/packages/houdini-core/plugin/documents/artifacts/selection_test.go index 84055470c..f0ed6d1ee 100644 --- a/packages/houdini-core/plugin/documents/artifacts/selection_test.go +++ b/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,\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,\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\"", + }, + }, }, }) } diff --git a/packages/houdini-core/plugin/documents/generate.go b/packages/houdini-core/plugin/documents/generate.go index ce14ecced..56a6558c7 100644 --- a/packages/houdini-core/plugin/documents/generate.go +++ b/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) diff --git a/packages/houdini-core/plugin/documents/persistentQueries.go b/packages/houdini-core/plugin/documents/persistentQueries.go index 9d854f239..daa75a2a9 100644 --- a/packages/houdini-core/plugin/documents/persistentQueries.go +++ b/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 { diff --git a/packages/houdini-core/plugin/documents/persistentQueries_test.go b/packages/houdini-core/plugin/documents/persistentQueries_test.go index ad0b43d7a..b39b4cc36 100644 --- a/packages/houdini-core/plugin/documents/persistentQueries_test.go +++ b/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,