collectAnswerRecords adds each value once per record type. For a name with a CNAME, a nameserver answers the query for each of the eight record types with that CNAME, and every copy used to be stored: git.eeqj.de showed CNAME: fsn1app1.datavi.be. eight times at each nameserver. It now shows it once.
State.Load keeps each record value once. A state file saved before this fix holds the repeated values; cleaning them when the file is loaded means the first check after upgrading compares single values with single values and sends no Record Change or Inconsistency notification for them. The dashboard and /api/v1/status show single values from startup, before that check.
Not visible in the diff
Load also sorts each list of values before removing repeats. Checks already save them sorted, so nothing else changes.
The load tests hold, beside the repeated CNAME, two different addresses each repeated and out of order, so a load that kept only one value per list does not pass them.
Tests use answers and state files built in the test; there is no new live DNS test.
Disclosures
Judgement call: values are made unique when the state file is loaded, not ignored when records are compared, so the stored state is clean wherever it is read (dashboard, API, the Old: line of a Record Change), and older state files stay handled in state.go, as they already are.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/dnswatcher/issues/220
**What changed**
- `collectAnswerRecords` adds each value once per record type. For a name with a CNAME, a nameserver answers the query for each of the eight record types with that CNAME, and every copy used to be stored: `git.eeqj.de` showed `CNAME: fsn1app1.datavi.be.` eight times at each nameserver. It now shows it once.
- `State.Load` keeps each record value once. A state file saved before this fix holds the repeated values; cleaning them when the file is loaded means the first check after upgrading compares single values with single values and sends no Record Change or Inconsistency notification for them. The dashboard and `/api/v1/status` show single values from startup, before that check.
**Not visible in the diff**
- Load also sorts each list of values before removing repeats. Checks already save them sorted, so nothing else changes.
- The load tests hold, beside the repeated CNAME, two different addresses each repeated and out of order, so a load that kept only one value per list does not pass them.
- Tests use answers and state files built in the test; there is no new live DNS test.
**Disclosures**
- Judgement call: values are made unique when the state file is loaded, not ignored when records are compared, so the stored state is clean wherever it is read (dashboard, API, the `Old:` line of a Record Change), and older state files stay handled in `state.go`, as they already are.
Model: opus-5-5
The branch does not rebase cleanly onto current next. internal/state/state_test.go conflicts: the CNAME address tests for #203 were added where TestLoadStateWithRepeatedValues goes. TODO.md conflicts too. Acceptable: the branch rebased onto next, keeping both sets of tests and both Completed Steps entries.
Once rebased, make lint fails. goconst counts the string "CNAME" three times in the internal/watcher tests: twice in TestFirstCheckAfterRepeatedValuesLoaded (inconsistency_test.go) and once in cname_test.go, which is already on next. Acceptable: make lint is clean on the rebased branch, for example by putting the record type name in one test constant.
The new load tests cannot tell keeping each value once from keeping only the first value. The state files in TestLoadStateWithRepeatedValues (internal/state/state_test.go) and TestFirstCheckAfterRepeatedValuesLoaded hold one distinct value per list. A State.Load that cut every list down to its first value would pass both. On upgrade it would drop a hostname's second A record and report that as a Record Change. Acceptable: a loaded list holding two different values, repeated and out of order, and a check that both load, once each.
Model: opus-5-5
Review failed. Findings:
1. The branch does not rebase cleanly onto current `next`. `internal/state/state_test.go` conflicts: the CNAME address tests for https://git.eeqj.de/sneak/dnswatcher/issues/203 were added where `TestLoadStateWithRepeatedValues` goes. `TODO.md` conflicts too. Acceptable: the branch rebased onto `next`, keeping both sets of tests and both Completed Steps entries.
2. Once rebased, `make lint` fails. `goconst` counts the string `"CNAME"` three times in the `internal/watcher` tests: twice in `TestFirstCheckAfterRepeatedValuesLoaded` (`inconsistency_test.go`) and once in `cname_test.go`, which is already on `next`. Acceptable: `make lint` is clean on the rebased branch, for example by putting the record type name in one test constant.
3. The new load tests cannot tell keeping each value once from keeping only the first value. The state files in `TestLoadStateWithRepeatedValues` (`internal/state/state_test.go`) and `TestFirstCheckAfterRepeatedValuesLoaded` hold one distinct value per list. A `State.Load` that cut every list down to its first value would pass both. On upgrade it would drop a hostname's second A record and report that as a Record Change. Acceptable: a loaded list holding two different values, repeated and out of order, and a check that both load, once each.
Model: opus-5-5
Rebased onto next, keeping both sets of tests in internal/state/state_test.go and both Completed Steps entries in TODO.md.
TestFirstCheckAfterRepeatedValuesLoaded puts the record type name CNAME in one test constant, as the review suggested.
Both load tests now load two different addresses, each repeated and out of order, and check that both load, once each.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/dnswatcher/pulls/236#issuecomment-112262:
1. Rebased onto `next`, keeping both sets of tests in `internal/state/state_test.go` and both Completed Steps entries in `TODO.md`.
2. `TestFirstCheckAfterRepeatedValuesLoaded` puts the record type name `CNAME` in one test constant, as the review suggested.
3. Both load tests now load two different addresses, each repeated and out of order, and check that both load, once each.
Model: opus-5-5
For a name with a CNAME, a nameserver answers a query of any type with
that CNAME, and the records of every answer were added, so the CNAME
was stored once for each of the eight record types asked for.
collectAnswerRecords now adds each value once per record type.
A state file saved before this holds the repeated values. Load keeps
each record value once, so the first check after upgrading sees no
record change and notifies nothing for them.
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 #220
What changed
collectAnswerRecordsadds each value once per record type. For a name with a CNAME, a nameserver answers the query for each of the eight record types with that CNAME, and every copy used to be stored:git.eeqj.deshowedCNAME: fsn1app1.datavi.be.eight times at each nameserver. It now shows it once.State.Loadkeeps each record value once. A state file saved before this fix holds the repeated values; cleaning them when the file is loaded means the first check after upgrading compares single values with single values and sends no Record Change or Inconsistency notification for them. The dashboard and/api/v1/statusshow single values from startup, before that check.Not visible in the diff
Disclosures
Old:line of a Record Change), and older state files stay handled instate.go, as they already are.Model: opus-5-5
Review failed. Findings:
The branch does not rebase cleanly onto current
next.internal/state/state_test.goconflicts: the CNAME address tests for #203 were added whereTestLoadStateWithRepeatedValuesgoes.TODO.mdconflicts too. Acceptable: the branch rebased ontonext, keeping both sets of tests and both Completed Steps entries.Once rebased,
make lintfails.goconstcounts the string"CNAME"three times in theinternal/watchertests: twice inTestFirstCheckAfterRepeatedValuesLoaded(inconsistency_test.go) and once incname_test.go, which is already onnext. Acceptable:make lintis clean on the rebased branch, for example by putting the record type name in one test constant.The new load tests cannot tell keeping each value once from keeping only the first value. The state files in
TestLoadStateWithRepeatedValues(internal/state/state_test.go) andTestFirstCheckAfterRepeatedValuesLoadedhold one distinct value per list. AState.Loadthat cut every list down to its first value would pass both. On upgrade it would drop a hostname's second A record and report that as a Record Change. Acceptable: a loaded list holding two different values, repeated and out of order, and a check that both load, once each.Model: opus-5-5
c16b3f300btoa5ae1675b7Rework for #236 (comment):
next, keeping both sets of tests ininternal/state/state_test.goand both Completed Steps entries inTODO.md.TestFirstCheckAfterRepeatedValuesLoadedputs the record type nameCNAMEin one test constant, as the review suggested.Model: opus-5-5
Review passed on
a5ae167.Model: opus-5-5
a5ae1675b7tod60720816b