USACE / cwms-data-api

Corps Water Management System RESTful Data Service
MIT License
9 stars 13 forks source link

remove redundant calls to CwmsDTOBase::validate #719

Closed adamkorynta closed 1 week ago

adamkorynta commented 1 week ago

the Formats deserialization process handles the validate call of content received from the client

rma-rripken commented 1 week ago

I missed that your previous Format changes slipped in a validate call. I'm hesitant.

adamkorynta commented 1 week ago

I struggle a little with the purpose of the validate() method. It seems to be designed to ensure that the client's payload deserializes to a working DTO. In which case I do think we need to call it on all deserialization in the Formats class. I didn't see any references to the validate() getting called prior to serialization going back to the client (might be a good idea?). It seems like we should just build it into the serialization through annotations somehow....

MikeNeilson commented 1 week ago

An annotation to mark fields as "required for operation" seems like a good idea, just a lot of effort.

The basic idea was enough validation for user provided data to meet minimum requirements if easily known and would save a lot of time going to the database. If the database fails quickly it's not a huge deal.

MikeNeilson commented 1 week ago

But since we're using the ObjectMapper more and we can get an object schema, I think Adam is right that validation function isn't as useful as it was in the past.

MikeNeilson commented 1 week ago

Or it might have been to handle those field exceptions better.