Every reference from a model in internal/database to the record it belongs to is now json:"-", so a model marshals the records it holds but never its parent, and no two models can marshal each other in a loop. Preloading maps associations without reading json tags, so it is unchanged; the new test in model_parent_test.go preloads a webhook with its targets and entrypoints (and their webhook) and a target with its webhook, checks each reference is filled, and checks the JSON leaves it out.
Associations checked:
Left out of JSON (child to parent): Target.Webhook, Entrypoint.Webhook, Webhook.User, APIKey.User, Delivery.Event, Delivery.Target, DeliveryResult.Delivery, Event.Webhook, Event.Entrypoint.
Still marshalled (parent to children): User.Webhooks, User.APIKeys, Webhook.Entrypoints, Webhook.Targets, Target.Deliveries, Event.Deliveries, Delivery.DeliveryResults.
Nothing in the service or its tests round-trips a model through JSON; model_secrets_test.go marshals models but never a parent reference.
Worth knowing: a preload builds a finite tree, so before this change a preloaded model would have repeated its parent rather than recursed forever; a real loop needed code that points a child back at the value holding it. The test therefore checks that the parent is absent from the JSON.
Judgement call: Event.Webhook and Event.Entrypoint close no loop on their own (neither parent lists its events); they are left out too so the rule holds for every parent reference.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/webhooker/issues/177.
Every reference from a model in `internal/database` to the record it belongs to is now `json:"-"`, so a model marshals the records it holds but never its parent, and no two models can marshal each other in a loop. Preloading maps associations without reading json tags, so it is unchanged; the new test in `model_parent_test.go` preloads a webhook with its targets and entrypoints (and their webhook) and a target with its webhook, checks each reference is filled, and checks the JSON leaves it out.
Associations checked:
- Left out of JSON (child to parent): `Target.Webhook`, `Entrypoint.Webhook`, `Webhook.User`, `APIKey.User`, `Delivery.Event`, `Delivery.Target`, `DeliveryResult.Delivery`, `Event.Webhook`, `Event.Entrypoint`.
- Still marshalled (parent to children): `User.Webhooks`, `User.APIKeys`, `Webhook.Entrypoints`, `Webhook.Targets`, `Target.Deliveries`, `Event.Deliveries`, `Delivery.DeliveryResults`.
Nothing in the service or its tests round-trips a model through JSON; `model_secrets_test.go` marshals models but never a parent reference.
Worth knowing: a preload builds a finite tree, so before this change a preloaded model would have repeated its parent rather than recursed forever; a real loop needed code that points a child back at the value holding it. The test therefore checks that the parent is absent from the JSON.
Judgement call: `Event.Webhook` and `Event.Entrypoint` close no loop on their own (neither parent lists its events); they are left out too so the rule holds for every parent reference.
Model: opus-5-5
internal/database/model_parent_test.go pins only Target.Webhook and Entrypoint.Webhook. The other seven references this change leaves out of the JSON (Webhook.User, APIKey.User, Delivery.Event, Delivery.Target, DeliveryResult.Delivery, Event.Webhook, Event.Entrypoint) are never set in any test that marshals a model, so the json:"-" on any one of them can be removed with every test still passing. Five of them close the same kind of loop #177 is about (a user and its webhooks or API keys, a delivery and its event or target, a delivery result and its delivery), so for those the safety stays as unpinned as the issue found it. Acceptable: the test also marshals a model with each of those seven references set (built in memory is enough) and checks the JSON leaves each one out, so removing any single tag fails a test.
Judgement call: the definition of done names a test only for the webhook and target pair; finding 1 holds the other references this PR tags to the same bar.
Model: opus-5-5
1. `internal/database/model_parent_test.go` pins only `Target.Webhook` and `Entrypoint.Webhook`. The other seven references this change leaves out of the JSON (`Webhook.User`, `APIKey.User`, `Delivery.Event`, `Delivery.Target`, `DeliveryResult.Delivery`, `Event.Webhook`, `Event.Entrypoint`) are never set in any test that marshals a model, so the `json:"-"` on any one of them can be removed with every test still passing. Five of them close the same kind of loop https://git.eeqj.de/sneak/webhooker/issues/177 is about (a user and its webhooks or API keys, a delivery and its event or target, a delivery result and its delivery), so for those the safety stays as unpinned as the issue found it. Acceptable: the test also marshals a model with each of those seven references set (built in memory is enough) and checks the JSON leaves each one out, so removing any single tag fails a test.
Judgement call: the definition of done names a test only for the webhook and target pair; finding 1 holds the other references this PR tags to the same bar.
Model: opus-5-5
TestModelsMarshalWithoutTheirParent in internal/database/model_parent_test.go builds each of the seven models in memory with its parent set and checks the parent's id is absent from the JSON, so removing any single tag fails a test; it checks the id rather than a key name so that dropping the tag entirely (the key would become User) fails as well as restoring the old one.
Rebased onto next.
Model: opus-5-5
1. `TestModelsMarshalWithoutTheirParent` in `internal/database/model_parent_test.go` builds each of the seven models in memory with its parent set and checks the parent's id is absent from the JSON, so removing any single tag fails a test; it checks the id rather than a key name so that dropping the tag entirely (the key would become `User`) fails as well as restoring the old one.
Rebased onto `next`.
Model: opus-5-5
internal/database/model_parent_test.go, TestPreloadedModelsMarshalWithoutTheirParent: the test only checks that the lowercase key "webhook": is missing. If someone deletes the json:"-" tag outright from Target.Webhook or Entrypoint.Webhook, the key becomes Webhook, and every test still passes. These are the two references #177 is about. The previous finding named the same gap for the other seven references, and the new test closes it there by checking for the parent's id rather than a key name. Acceptable: removing the tag from either field, whether by deleting it or by restoring the old one, fails a test, as it now does for the other seven.
Judgement call: the earlier finding named only the other seven references; this finding holds the issue's own pair to the same standard.
Model: opus-5-5
1. `internal/database/model_parent_test.go`, `TestPreloadedModelsMarshalWithoutTheirParent`: the test only checks that the lowercase key `"webhook":` is missing. If someone deletes the `json:"-"` tag outright from `Target.Webhook` or `Entrypoint.Webhook`, the key becomes `Webhook`, and every test still passes. These are the two references https://git.eeqj.de/sneak/webhooker/issues/177 is about. The previous finding named the same gap for the other seven references, and the new test closes it there by checking for the parent's id rather than a key name. Acceptable: removing the tag from either field, whether by deleting it or by restoring the old one, fails a test, as it now does for the other seven.
Judgement call: the earlier finding named only the other seven references; this finding holds the issue's own pair to the same standard.
Model: opus-5-5
Every reference from a model to the record it belongs to is now
json:"-", so the webhook and target models (and the other parent and
child pairs in internal/database) can no longer marshal each other in
a loop. Preloading maps associations without reading json tags, so it
still fills these references. One test preloads a webhook with its
targets and entrypoints and a target with its webhook, checks the
references are filled, and checks the JSON leaves them out; another
builds each other model with its parent set and checks the parent's
id is absent from its JSON.
Model: opus-5-5
TestPreloadedModelsMarshalWithoutTheirParent now marshals the preloaded entrypoint and target on their own and checks that the webhook's id is absent from their JSON as an "id" value, as TestModelsMarshalWithoutTheirParent checks for the parent's id; it looks for the "id" field rather than the bare id because each child also holds the webhook's id as its webhookId.
Rebased onto next.
Model: opus-5-5
1. `TestPreloadedModelsMarshalWithoutTheirParent` now marshals the preloaded entrypoint and target on their own and checks that the webhook's id is absent from their JSON as an `"id"` value, as `TestModelsMarshalWithoutTheirParent` checks for the parent's id; it looks for the `"id"` field rather than the bare id because each child also holds the webhook's id as its `webhookId`.
Rebased onto `next`.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #177.
Every reference from a model in
internal/databaseto the record it belongs to is nowjson:"-", so a model marshals the records it holds but never its parent, and no two models can marshal each other in a loop. Preloading maps associations without reading json tags, so it is unchanged; the new test inmodel_parent_test.gopreloads a webhook with its targets and entrypoints (and their webhook) and a target with its webhook, checks each reference is filled, and checks the JSON leaves it out.Associations checked:
Target.Webhook,Entrypoint.Webhook,Webhook.User,APIKey.User,Delivery.Event,Delivery.Target,DeliveryResult.Delivery,Event.Webhook,Event.Entrypoint.User.Webhooks,User.APIKeys,Webhook.Entrypoints,Webhook.Targets,Target.Deliveries,Event.Deliveries,Delivery.DeliveryResults.Nothing in the service or its tests round-trips a model through JSON;
model_secrets_test.gomarshals models but never a parent reference.Worth knowing: a preload builds a finite tree, so before this change a preloaded model would have repeated its parent rather than recursed forever; a real loop needed code that points a child back at the value holding it. The test therefore checks that the parent is absent from the JSON.
Judgement call:
Event.WebhookandEvent.Entrypointclose no loop on their own (neither parent lists its events); they are left out too so the rule holds for every parent reference.Model: opus-5-5
internal/database/model_parent_test.gopins onlyTarget.WebhookandEntrypoint.Webhook. The other seven references this change leaves out of the JSON (Webhook.User,APIKey.User,Delivery.Event,Delivery.Target,DeliveryResult.Delivery,Event.Webhook,Event.Entrypoint) are never set in any test that marshals a model, so thejson:"-"on any one of them can be removed with every test still passing. Five of them close the same kind of loop #177 is about (a user and its webhooks or API keys, a delivery and its event or target, a delivery result and its delivery), so for those the safety stays as unpinned as the issue found it. Acceptable: the test also marshals a model with each of those seven references set (built in memory is enough) and checks the JSON leaves each one out, so removing any single tag fails a test.Judgement call: the definition of done names a test only for the webhook and target pair; finding 1 holds the other references this PR tags to the same bar.
Model: opus-5-5
d453f76fb3to1a48f0541aTestModelsMarshalWithoutTheirParentininternal/database/model_parent_test.gobuilds each of the seven models in memory with its parent set and checks the parent's id is absent from the JSON, so removing any single tag fails a test; it checks the id rather than a key name so that dropping the tag entirely (the key would becomeUser) fails as well as restoring the old one.Rebased onto
next.Model: opus-5-5
internal/database/model_parent_test.go,TestPreloadedModelsMarshalWithoutTheirParent: the test only checks that the lowercase key"webhook":is missing. If someone deletes thejson:"-"tag outright fromTarget.WebhookorEntrypoint.Webhook, the key becomesWebhook, and every test still passes. These are the two references #177 is about. The previous finding named the same gap for the other seven references, and the new test closes it there by checking for the parent's id rather than a key name. Acceptable: removing the tag from either field, whether by deleting it or by restoring the old one, fails a test, as it now does for the other seven.Judgement call: the earlier finding named only the other seven references; this finding holds the issue's own pair to the same standard.
Model: opus-5-5
1a48f0541ato950a5a51a7TestPreloadedModelsMarshalWithoutTheirParentnow marshals the preloaded entrypoint and target on their own and checks that the webhook's id is absent from their JSON as an"id"value, asTestModelsMarshalWithoutTheirParentchecks for the parent's id; it looks for the"id"field rather than the bare id because each child also holds the webhook's id as itswebhookId.Rebased onto
next.Model: opus-5-5
Review passed.
Model: opus-5-5