Code review comment for lp:~trapnine/maas/fix-1564971

> This looks good in general to me. A few comments below.
>
> I just have one question: I see that you are doing the validation in a signal
> handler. Normally we would put it in the model or in the form. Was there any
> particular reason it was done this way? (Is there already a precedent in MAAS
> code somewhere? I have to admit I haven't done much with signals.)
>
> Did you do any manual testing with the API for this change?

I've actually be directed towards signals and away from complex clean() override processing in the past, most notably for the BMC validation work. There are lots of examples in the signals package this new signal handler lives in. This is better than a form clean() because it will be run for any IPRange model save(), not just a form save().

In this case, it was 'almost' necessary because I actually delete the to-be-saved instance, then do some testing, then recreate it. I was not sure I could get away with that in a model's actual save() override (maybe it'd be OK). Also, signals make it easy to store pre-modification state in the post-init signal, then read them in the post-save signal to see what was changed. Basically, after all the other normal form and model checking has been done, this will test for range conflicts.

I did not manually test the API for the change, just the unit tests. This was written as a 'make model save() self-protect against dupes and overlapped ranges' perspective, not a 'fix the API or UI' perspective. Is there a gotcha I'm missing in the API (like maybe it doesn't error to the API cleanly when an Exception is thrown from the model)?