4

I am learning software design by building a CRUD web application (ASP.NET MVC with Entity Framework). I split it into two projects: first is Core library, that contains business logic, second is Web GUI. I got an idea, that separated Core library can be used elsewhere (for example desktop application) or GUI can be completely changed later if needed.

Business logic is stored in "managers" which operate with database entities and provide results using data transfer objects, because I do not want to expose db entities outside the Core. Web GUI accepts user input and uses managers to do the job. It also defines view model objects, which are basically copycat from data transfer objects, but enhanced with data annotation attributes.

Problem

Web project performs server side validation on view model objects because input from user should be verified. Core project performs same validation again on data transfer objects, because it knows nothing about GUI layer above it. Question is how can I avoid duplicated data validation, because sometimes it requires queries to database and in general adds overhead and duplicated code. What are the best practices in my case? Or the whole idea of separating solution into two projects is wrong?

Posts I've read so far discuss validation in normal 3 layered solutions, where validations are slightly different in each layer (client (javascript), business and data layer) and such duplicated validation is not an issue (both are business).

Solutions I came up to

  1. Let GUI to perform validation and ignore any in Core.
  2. Let managers to perform validation and catch raised exceptions in GUI. I've read that this is bad practice[citation-needed].
  3. Move validation inside the data transfer objects with private "already validated" flag. Provide managers with ValidateDto method which triggers validation on given DTO. Validation results in dictionary of error messages with property names as keys, sets flag, but if already set does nothing. Also mechanism is needed to unset the flag if DTO was changed.

Edit

I think I need to clarify: Core is class library, Web is normal ASP.NET MVC application with views, controllers, etc.

Edit 2

My question looks like a duplicate of Data input validation - Where? How much? [closed] and I disagree. This post discuss different validation scopes on different levels within an application (Client side javascript, business layer, data layer validation). Each level covers different validation scopes, but they tend to intersect. While I seek an answer how to remove duplicated validation within single level (business) when it is separated into two projects. Also linked post does not have accepted answer and most voted answer is basically telling to move core validation into single place. Which is ultimately correct but is not an answer to my issue.

All answers so far suggest to put validation into Core class library. So question now is how should I return information about invalid input errors.

  1. Will it be special methods in Core "API" which should be called every time before data are provided to Core. This API will return detailed error information which can be provided to user. This requires user of Core library (programmer) to know about such special behavior (or a lot of coding otherwise, to decide if validation was called by user before or not).

  2. Will it be specially defined exception which will be thrown by Core if input is invalid. This exception will contain detailed errors information.

Solution

Thank you all for answers and comments! I ended up with removing all server side validation from Web project and validating only inside Core project. I also created InvalidModelException as user Machado suggested and used Exception.Data property to store all model errors in form of key/value pairs, where key was name of invalid property and value was error code previously defined in Core as enum. Inside Web project I am catching this InvalidModelException exception and once caught I am filling ModelState with model errors to provide better feedback to user. I am storing all user friendly error messages in resource .resx file in Web project. Based on property name and error code from InvalidModelException I am generating ErrorMessageName string to pull out friendly error message from resource file.

Roman Artemov
  • 51
  • 1
  • 6
  • Sorry, you can't avoid some duplication. Or don't validate anything on the client, but typically user experience dictates some validation be done on the client. – Jon Raynor May 16 '17 at 15:50
  • 1
    see my answer here https://softwareengineering.stackexchange.com/questions/332314/validation-error-responses-in-rest-api/332528#332528 – Ewan May 16 '17 at 16:33
  • @JonRaynor I feel you miss understood my problem, I am not talking about client side validation. Both are performed on server side, but in different projects. See my edit. – Roman Artemov May 16 '17 at 16:48
  • @Ewan I am not sure how your answer solves my problem. In Core class library I throw an exception if DTO is invalid, but it still needs to perform whole validation process in order to complete operation in manager successfully. If you are talking about solution 3., I want to provide GUI with similar error handling as ModelState errors. So user is informed about all errors in the input. – Roman Artemov May 16 '17 at 16:52
  • throw simple errors and exceptions server side. do complex user friendly validation client side – Ewan May 16 '17 at 16:56
  • Can you provide a reference to where #2 is considered bad practice? In my experience, the farther away from the core you put your validation, the more flaws you will have. – JimmyJames May 16 '17 at 16:57
  • Possible duplicate of [Data input validation - Where? How much?](https://softwareengineering.stackexchange.com/questions/81062/data-input-validation-where-how-much) – gnat May 16 '17 at 17:01
  • @JimmyJames I meant that handling expected errors with exceptions is a bad practice. That exceptions should be used for unexpected behavior. Throwing them in Core is reasonable, because it does not receive direct user input. But I feel that in Web user input should be validated before it is passed to Core. I might be wrong. – Roman Artemov May 16 '17 at 17:07
  • OK, I think I have enough to write an answer. – JimmyJames May 16 '17 at 17:08
  • 1
    `...exceptions should be used for unexpected behavior.` -- **Exceptions should be used for conditions that you cannot automatically recover from.** If a user presents you with invalid data that prevents an operation from occurring, that's an *exceptional condition.* – Robert Harvey May 16 '17 at 19:46
  • @RobertHarvey, +1. We should work together. I could learn a lot from you. – Machado May 18 '17 at 13:07

4 Answers4

4

To recap/confirm: you have two layers of an application (in the same process space) where the outer layer of the application is performing validations that the inner layer is also performing. This is not to be confused with client-side validation which is about user experience. There are basically two main routes you can take with this: the simple way, or the complicated way.

The simple way is to remove the redundant validation. You can either remove it from the inner layer or the outer layer. Having it only in the outer layer doesn't really make much sense to me. Having two layers means you can use the inner layer in some other solution. You could also reuse the outer layer in theory but this is probably unlikely. If you reuse the inner layer with some other outer layer, that means you need to duplicate the validation there or somehow have ability to enable disable the validation on the core. The simplest approach is to put all the 'core' validation in the inner layer and only implement validation in the outer layer that is specific to that layer.

There is a question of exceptions. The cost of exceptions is often over-sold. I did some analysis on the cost of exceptions in Java and 1. It's completely dependent on the depth of the stack 2. Even with really deep stacks (thousands of calls) I had to run many iterations just to get meaningful measurements. I expect it's the same for C#. I would encourage you to do some experimentation to see if this is really a problem. Using them for control flow in a Pythonistic fashion is discouraged but protecting your system from bad data is one case where throwing an exception seems perfectly valid to me. if you really want to avoid this, you can use some sort of object associated with the request to track states and problems. This might be a great idea if you need to handle warnings or non-fatal errors.

The complicated way is that you use some sort of ledger in the validation logic to track whether each required validation has been executed on the given transaction. Then the core doesn't need to re-execute that logic. This is fancy and therefore there are lots of opportunities to create new problems.

JimmyJames
  • 24,682
  • 2
  • 50
  • 92
  • Great recap! Reading all answers so far, I will probably leave validation to the Core and will throw special predefined exception with detailed info about errors. Basically 2. solution despite I was against this option at the beginning. – Roman Artemov May 16 '17 at 17:51
  • "*Using them for control flow in a Pythonistic fashion is discouraged but protecting your system from bad data is one case where throwing an exception seems perfectly valid to me.*" - Perfectly said. Have a +1. :) – Machado May 16 '17 at 17:55
3

The core doesn't know where the data comes from, right ? So, since all data inputs must be sanitized, I'd say that's it's a must for the CORE to validate the inputs.

That being said, you'd have to create an API where you expect to receive bad input and return something meaningful for the caller, and not just throwing weird and cryptic exceptions everywhere. "Garbage in, garbage out" is just a bad API design.

One approach I've used in the past with a few MVC5 applications was to create a "BusinessException", which contained some key information for the caller to understand that they've been using the API wrong. One of the components of the BusinessException was a "user friendly error message", that could be directly shown to a user either in a Forms app or a Web app (the core never knew where it was running), along with some other important fields such as "FaultyProperty", "TechnicalException" and so on.

That way you can make your MVC app a thin layer doing basic validation (very basic) before dispatching the heavy load to the Core, and keep a single idiomatic way to handle errors by catching the "BusinessException" when needed and reacting accordingly.

This way you get to keep the single responsibility principle and DRY in place while still holding your validations and keeping code easy to read.

And by giving details of the error back to the caller by filling specific fields/properties in the BusinessException, the GUI layer can highlight whatever it needs to in order to provide a nice user experience.

Machado
  • 4,090
  • 3
  • 25
  • 37
1

An exception from core is probably not appropriate, since you might have multiple independent validation error in an input dataset, and you want to show all the errors in the UI. So I think the best approach is to have a validate(InputDataSet) method in the core which returns a collection of validation errors for the input data.

You could have the validate method return a different type, a ValidatedDataset, if the validation succeeded. Then you can have the business methods accept only a ValidatedDataset.

JacquesB
  • 57,310
  • 21
  • 127
  • 176
  • 1
    I don't see why you can't leverage the existence of [AggregateException](https://msdn.microsoft.com/en-us/library/system.aggregateexception(v=vs.110).aspx) if needed. For a CRUD application a single exception may be enough, though. – Machado May 16 '17 at 17:23
  • @Machado: This would make sense if you *expect* valid input, but perform a precondition check to be defensive. But if there is a specific method for validating user input, then it shouldn't throw an exception if there is validation errors, since this a legitimate outcome. Exceptions should IMHO only be for cases where a method is prevented from doing its designated job, which is not really the case here. – JacquesB May 16 '17 at 17:32
  • while I agree with you on the principle, I have some concerns on the practical use. Having a side validation method may be idiomatic on the API, but it's also a 2-step operation that requires the user of your code to read the documentation before trying it, in order to learn "the protocol". While just shoving things to the service and letting it figure out what to do and returning an error on a single step looks messy, but allows the programmer to learn by exploring your code instead of reading some lost wiki somewhere. – Machado May 16 '17 at 17:52
  • @Machado: If you want an API to be discoverable, exceptions are actually the worse choice since they are not indicated in the method signature (in .net) but are only discovered if you happen to trigger one...or read the documentation. – JacquesB May 16 '17 at 18:05
  • Yes, you're right. And that's the point. Explore the code and learn as you go. That would be exactly the same scenario with 2-step validation, without the need of the extra if. The 2-step validation requires you to learn the protocol, which you'll do by either triggering an exception because you didn't validate before calling or by reading documentation. Why not just embrace the exception in this case and reduce the complexity of your code since you'll still have the obligation to catch a exception ? – Machado May 16 '17 at 18:16
  • Also, why doesn't the @ works with your username ? I can't tag you on my comments! Your name just vanishes... – Machado May 16 '17 at 18:19
  • You have the two steps because you need to separate the validated input from the unvalidated input - and the actual business logic should only accept validated input. Otherwise you would have to perform the same validation on every single method in the call chain, which would be crazy. – JacquesB May 16 '17 at 18:47
  • All the public surface of your API must perform input validation. Internals, private and protected methods can rely on the fact that the public surface of the API did its job, and did not rely on the user calling an outside method to validate the input. Your API must protect itself against misuse. – Machado May 16 '17 at 18:55
  • Are you thinking about a web API or similar? In that case you should indeed validate all external input. But internally you would have a business layer which accept input which is already validated, otherwise you will get some kind of infinite regress. – JacquesB May 16 '17 at 19:05
1

If this is an in-house project with small teams, I think you can safely do away with one of the layers of validation. If the project is large or complex, or you have to deal with multiple teams, or if third parties will be accessing your APIs, you probably want the redundant validation.

If the act of validating something has a lot of overhead due to database lookups, consider implementing a common cache. You would inject your cache provider in the composition root to ensure all layers of your application use the same, common cache. When a redundant validation occurs, and it has to query the database for some lookup values, there will be a cache hit and the performance impact will be minimal.

John Wu
  • 26,032
  • 10
  • 63
  • 84