0

What we usually have at work is an API that has a single consumer, usually a SPA web frontend, which is built by the guy sitting next to you.

I completely see the need for authentication and authorization.

What tends to happen in the controller methods though is a little insane to me. Basically the request is absolutely not to be trusted. Every Controller method start with "check this, check that, make sure its not this, make sure this is true" and give out "correct" response codes and messages. null check null check null check.

Lets say we have an Update Offer post endpoint. So the Request is some kind of offer with an Id and a list of Items with corresponding OfferIds. The only ones who even know the endpoints and requests are our employees.

  • First I validate its the right request, makes sense.
  • Then I validate whether the Offer actually exists.
  • Then I validate whether the items actually exist.
  • Then I validate whether this offer belongs to the user.
  • then I validate whether these items belong to that offer.
  • top all of that off with excessive null checks
  • if its the create offer endpoint, check that the request offer doesn't have an id

These are all just examples. Please do not focus in on how I could remove the Id from the CreateOffer request. Or similar.

None of these are that bad by themselves. Its just a lot of checking and you end up with 30 lines of checks and validation followed by 2 lines of actual code in every single request handler.

I feel like those best practices are for either public APIs or APIs with a lot of consumers.

To me this code has no purpose. Its written to give someone who puts complete gibberish into our endpoints as good of an experience as possible. But the only one who could do so is my coworker.

Lets say 2 people are working on the backend API. One on the actual request endpoints and another one on the database repository layer.

So I am the repository developer, its my job to create a OfferRepository.Update() method that takes an offer object and persists it to the database. My colleague handles business logic in the endpoints and then calls into my repository to make sure it gets persisted.

Do I as the repository developer have to do all the same checks again? Throw custom exceptions and make up status codes? I think most people would agree that, that would be stupid. When does it end? Do I stop trusting myself within my own method and do the checks again for good measure halfway through?

So why is Request => endpoint any different than endpoint => repository? An exception is not a big deal. Either the frontend guy or the request guy is just gonna tell you "hey man I called ur code, it crashed. How can we fix it?"

I think all of this comes from a totally good place. People google and try to follow what way more experienced people do. To me this code adds nothing to the project. It doesn't solve any problem or task. Its essentially "look you can put everything in here and get a nice response. Aren't I the cleanest coder?"

You cannot stop other developers from crashing your code, if they actually try. I need two lines of code to crash microsofts implementation of the division operator.

1
  • 1
    The short answer is this totally depends on the data you are handling. If you are only delivering data that should be available for all users and can be edited by all users, there is no point inn all this. But what if you deliver very sensitive data that only some few people can read, can create and update and even more rules on who are allowed to do something with only some parts of the data (example they can modfiy one record but not other records)
    – Mr Zach
    Jan 28, 2021 at 3:06

5 Answers 5

4

My colleague handles business logic in the endpoints and then calls into my repository to make sure it gets persisted.

Do I as the repository developer have to do all the same checks again?

This seems to be the root of your question. Repositories are abstractions between business logic and persistence logic. Sometimes repositories include "collections logic" — that is, logic surrounding a larger collection of objects than you might not want to load into memory.

Since repositories are primarily concerned with persistence logic, and another layer handles business logic, do not duplicate business logic checks. Assume the business layer has already checked things for you. The repository need only check for the prerequisites necessary to store or delete the data.

If you cannot trust the business logic layer, then the business logic layer needs to be redesigned to utilize encapsulation and data hiding where appropriate. The business layer should be satisfying business rules by enforcing class invariants. If a property should not be null, the business layer should throw the exception. If a product should have no more than 3 offers, the business layer should enforce that.

The repository layer should not check these things, unless the constraint is a data constraint. Checking for a null property might be a data constraint. The database might have a NOT NULL constraint on a foreign key column. The repository layer can check for things like this. But if the database does not restrict how many offers a product has, and this restriction is a business constraint, your repository does not need to check this. The business layer should.

So, it all depends. A property being "not null" might be a reflection of a business rule. This business rule is then reflected in the data model. The business layer and repository layer might check this constraint, but they do so for different reasons.

The business layer checks that the property is not null for business reasons (because some product team, the marketing team or management said so).

The repository checks that the property is not null for data storage reasons, so an INSERT, UPDATE or DELETE operation does not throw an obtuse exception that is difficult to debug.

Input validation for the request performs pre-checks on user input to ensure the input follows business rules. The business layer enforces class invariants to make sure any data follows business rules. The repository enforces basic data checks to make sure the data operation succeeds.

1
  • Oh I totally made that up as an extreme example. For me the move between frontend to api or between api to repository are completely equal. I can't tell whether you think so too.
    – Uhmmer
    Jan 27, 2021 at 19:37
4

So why is Request => endpoint any different than endpoint => repository? An exception is not a big deal. Either the frontend guy or the request guy is just gonna tell you "hey man I called ur code, it crashed. How can we fix it?"

The difference between "Front-end Request => endpoint" and "endpoint => repository" lies in how much trust we can place in the communication channel between the two.

If the communication channel is a function call within a single process, then we can have a high level of trust that in the production environment you will have the same two parties communicating as what you tested with.

If the communication channel is a network connection (like a REST API), then there is no guarantee at all that an incoming request actually originated from the front-end code you tested with, so that is why you can't trust the request to be well-formed or otherwise valid. On a network connection, it is far too easy to send requests also from other sources, which might be malicious.

0

Not entirely sure i understand your meaning, but..

The first call from the front end, possibly, has to validate user input and crucially, return the validation failures so they can be displayed to the user for correction.

Further calls can eliminate validation checks by defining the expected behavior for "non valid" input, or by using strong typing to remove the possibility of that non valid input.

ie.

  • I can't stop the user not filling in a field, but i can make a property non nullable.

  • I can't stop create being called twice, but I can ignore the second call if the item already exists

For edge case errors you can simply throw an exception and expect the calling code to check for that edge case before calling your method.

ie.

I can't stop you sending a non-existent language code, or logically interpret the request without a language. But I can throw an exception which will force the user to check before they send.

0

"I feel like those best practices are for either public APIs or APIs with a lot of consumers"

Consider:

  • Users: If you are taking input from a SPA you are usually taking input from users, and users can break input too. This seems unlikely but it really does happen. I've seen it. I've seen a user upload a file that brought down an entire production system. The backend never placed limits of what could be in the file.
  • Frontend Bugs: It may be true that the front end developer is sitting next to you, but bugs are often caught late when validation is missing. So your UI could be sending invalid data that is saved to your DB and you miss this, because there was no validation.
  • Documentation: It's nice to sit next to someone and you are able to communicate with them when something is wrong, but what if you are on vacation? If the response is telling the caller exactly what is wrong, then they don't even have to ask you. Consequently, you aren't interrupted either, and if you ever leave the project someone else can understand what the input limits are without scanning the front end code.

None of these considerations are absolute, but if you have an application that has any importance in production, checking your inputs is a good move.

If the validation code is clumsy, you can look into finding something like a built-in framework level validation system (I think ASP .Net has something like this) or using a third party component (.Net has FluentValidation).

0

Always assume that the API call is made by a hacker trying to attack your backend and act accordingly.

1
  • No need to be a hacker, usually, end-users have the innate capacity to cause corner-cases from interfaces battle-tested by hundreds of tests. They can twist the "reality" to a point to make possible what you strived to make impossible. And they do this by a sequence of two clicks in the home page
    – Laiv
    Jan 29, 2021 at 14:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.