Improve validation

[ClFeSc]

Closes #116

@Dassderdie This could be the only option to validate union types. It's not very pretty. Do you think it would be worth the effort doing this for all union types that need validation?

All validation of union types and other literals has been removed from this PR.

It now adds support for validating UUIDSets, { [key: UUID]: T } objects and ReachableTransferPoints.

Also, string literal unions will now be validated, albeit a little bit more manual than I'd hoped for.

[Dassderdie]

I think too that this isn't really pretty, but probably the best solution.

I would propose that we keep this in mind for now (especially when we refactor parts of the state) and implement all the correct checks after milestone 2 or 3. This should be mostly a laborious task.

[ClFeSc]

Note to myself: Maybe this could help for UUIDSet: https://github.com/typestack/class-validator#defining-validation-schema-without-decorators

[ClFeSc]

@Dassderdie Although it's now a bit different than the original aim, this should now be ready for review!

[Dassderdie]

I haven't tested it out.
The comments are mostly renaming variables.

I believe this could be further improved by deduplicating some code. E.g., a general DictionaryValidator utility function accepts a function to validate the value and the keys. This could then be used to create the current validators with less code. Because the current implementation is very nicely encapsulated, it is ok like it is now.
It would probably be best to wait a bit more, look at the new validators, and then figure out the best API for a refactoring.

[ClFeSc]

@ClFeSc Note to myself: Look into @Transform from class-transformer for propagating the errors.

[ClFeSc]

@ClFeSc Note to myself: Look into @Transform from class-transformer for propagating the errors.

@Transform(
        (params) => {
            const plainChildren = params.value as {
                [key: UUID]: Viewport;
            };
            return Object.values(plainChildren).map((plainViewport) =>
                plainToInstance(Viewport, plainViewport)
            );
        },
        { toClassOnly: true }
)
@ValidateNested({ each: true })

is a good alternative for validating the values of the id map while keeping the error locations. It needs additional work to (a) use just one decorator for it and (b) validate the keys and the consistency.

[ClFeSc]

@Dassderdie What do you think of 62618b8?

[Dassderdie]

I haven't tested it yet.

[Dassderdie]

I've noticed a problem: The type-safe approach needs to be able to work both with simple properties (like string) and also with arrays (like string[]) as it's possible to use { each: true } as an option to validate arrays (see https://github.com/typestack/class-validator#validating-arrays), and we're using this for StateHistoryCompound.actionHistory (and also in VehicleTemplate which now fails the build).

Would it be a good solution to change the type based on the ValidationOptions in makeValidator? If this is only a special case for { each: true } and with T[] (or Record<any, T> dictionary) this should be feasible.

[ClFeSc]

I've noticed a problem: The type-safe approach needs to be able to work both with simple properties (like string) and also with arrays (like string[]) as it's possible to use { each: true } as an option to validate arrays (see https://github.com/typestack/class-validator#validating-arrays), and we're using this for StateHistoryCompound.actionHistory (and also in VehicleTemplate which now fails the build).

Would it be a good solution to change the type based on the ValidationOptions in makeValidator? If this is only a special case for { each: true } and with T[] (or Record<any, T> dictionary) this should be feasible.

This should now be included.

[Dassderdie]

Only smaller stuff.

Please also look at the commit I just pushed.

[Dassderdie]

Good job.

I added one last thing. If you add the change, I'm happy with this PR.

[ClFeSc]

I think I'll squash-merge here as there are a lot of commits that somewhat represent the line of thought throughout this year but add no meaningful information about the contents of this PR.

[ClFeSc]

@Dassderdie I've added two commits with small changes I noticed while writing the commit message. When you're happy with the changes, resolve the previous conversation, and I'll enable auto-merge (squash).