Discuss Architecture Patterns

[hassanhabib]

This issue is a thread to discuss potential patterns that we could adapt in OtripleS to streamline the development of the project, the current pattern follows the Models, Brokers, Services, Controllers and Views pattern, you can learn more about the pattern in the following sessions:
Fundamentals of Building ASP.NET Core Applications
Fundamentals of Building ASP.NET Core Applications (Brokers)
Fundamentals of Building ASP.NET Core Applications (Brokers & Services)
Architecting Industrial Software - Services Deep Dive
Exception Handling Noise in C#

Some of the contributors on this project have proposed some patterns like MediatR and some others had concerns about exception handling - this is the thread to discuss all these ideas and dive into their details.
This issue should not be a blocker for the current or future work until the team reaches a decision about which pattern we need to follow, it's never too late because eventually the current monolith we are building should dissolve into microservices this is the current mindset behind how we are building what we are building - building a monolith with microservices mindset.

[AgentEnder]

Just watched the video Exception Handling Noise in C# and I really like that idea, makes a ton of sense and seems easy to implement.

[Driedas]

First of all, I greatly appreciate the effort put into this, it's great to see so many people finding time to participate and churn out ideas and code for just the good feels of it, kudos. I'm adding my 5 cents to the discussion, nothing below is meant to be personal, or addressed to any specific person (I didn't do a git blame to check commit authors). I'm also not advocating for any changes in framework or technology in this post (however will do so if prompted), this is purely my observation and is to be taken as constructive criticism with no ill intent, purely my attempt at improving things (some of it subjectively I'm sure) and starting a dialog as I'd like the project to succeed, long term.

1) project structure
services, entities, API controllers everything in a single project. This would not be an issue on a small, single use, single logical deployment project. For anything larger than that, you will most likely have separate components, deployed separately (at least logically if not physically). For example handling background processes like sending emails, scheduled jobs, etc. All of these would need to host everything, including the API. It would make more sense to me to at least split out the model and services into separate assemblies and having the API as just an (or one of) optional communication layer on top of that.

2) API
2.1) missing authentication. How will the user authenticate against the API? How will his security credentials (roles or permissions) be verified? Even a hardcoded Principal to demonstrate use is alright at this stage I think.

2.2) noise (exceptions) to signal (actual logic) ratio is quite high, at a glance around 3/4 of the code is catching exceptions of different types, while doing similar (or in some cases exactly the same) things with them afterwards. Doing it like this is very costly from a maintenance and awareness point of view. Developers would typically copy paste these around. Code like this belongs into an action filter that will be applied globally for all of the endpoints, and the individual controller actions are thus freed up to only handle the actual logic

2.3) entities are being returned and accepted verbatim via the API including system attributes that should never be modified externally in an audited system. I made this example before, but with an interface like this it is possible to register a student with all A grades and graduated. Only a restricted set of properties should be modifiable via the API, the rest should be filled in by the system. @hassanhabib I know you mentioned that there will be some orchestration done on these to prevent a scenario like the above, but I see nothing in that direction in the project.
Do we even have a use case to update the properties of a student after registration? Unless the operator made a typo when registering, the student's name, sex, birthdate, etc remain the same throughout his stay in the institution.

3) service layer
3.1) service implementation split into 3 files. While I admire the intent to keep things like exception handling and validation outside of the service logic, in order to get a full view of what an actual service method does you need to open 3 files and switch between them. This breaks the flow of reading the code so it is more difficult to spot potential issues with it. The TryCatch func is an elegant solution to a problem that doesn't even need to exist however.

3.2) sooo many custom exception types. I count 8 custom exceptions for the Student entity only. Why is it necessary to distinguish between them? It is not clear to me which one to use when, e. g. NotFoundStudentException and NullStudentException, InvalidStudentInputException and StudentValidationException. Are there going to be another 8 exception types for the teacher as well?
Why is it even useful to use custom exception types? Broadly speaking from an interface point of view, there are only 2 kinds of exceptions. Ones that are caused by the caller, that contain error messages that the caller will understand and will be able to do something about (e. g. validation exceptions), that can be converted to the semantics of the communication interface, e. g. 400/422 status code responses with a web API. This can be handled by a shared ValidationException or similar.
The other kind are internal exceptions, whose internals should never be revealed to the outside, merely logged and reported on. The caller should receive a generic exception messages (my favorite one being "Something happened." that I actually got when installing Windows 10 for the first time), that could be translated to a 500 status code on the API.
What is the point of the service layer handling and logging and rethrowing all of those exceptions? All of those exceptions the service layer cannot do anything useful about and may just as well let them bubble up to the caller (with the distinction above, validation exceptions separate from all other exceptions, but I don't see why differentiate between "all other"). I could maybe see a use case where you want to alert operations based on certain exception types, but all of this can be done globally, on the communication interface and the services can more or less ignore all exceptions (except validation ones and maybe others where it can actually do something useful about).

3.3) services accept entities as parameters. I've made this observation in a PR, but will copy it here as well.
the Student entity will grow over time. It will not (and even now is not) apparent what data is required for the registration of a new student based on the entity alone. Having a method with explicit primitive parameters makes it obvious what the required data is. I think that the student registration method only should accept primitive parameters that are required for the registration of a new student, and fill in all the rest (createddate, createdby, status, ...).

3.4) validation. The validation of a student is now 200+ lines of case and if statements. The cyclomatic complexity of this is enormous and writing all of that code is tedious, error prone and ultimately can be replaced by using a smart validation framework that allows you to apply validation rules declaratively. Even out of the box Data Annotations offer all of that functionality and could be applied straight on the entity if need be, or on a command object that would represent the intent of student registration/modification

3.5) storage brokers. What is the added value of these? All they do is abstract EF as the repository, which is both a leaky abstraction (e. g. necessity to do eager loading via Include, using Find where appropriate, etc) and an unnecessarily complex one. Purely for the sake of mocking, would it not suffice to expose a Set<T>, Find<T>, SaveChanges and the ConnectionString?

3.6) auditing. How will this be performed? This ties a bit into the authentication system, and how the identity of the caller will flow through the system (if at all), but I'm also missing history tables attached to the entities (or something to approximate that functionality, e. g. event sourcing).

4) model
4.1) There is no domain model. From the looks of it, all entities will just be C# classes with public getters and setters. This is a viable approach for an anemic domain model with a transaction script pattern, but a domain model could be that much more than just getters and setters. The alternative would be trying to follow an always-valid approach for entities, and only exposing their state as read only, with methods designated for modifying their state. In this case e. g. Register, Suspend, Expell, etc. The Student entity would be responsible for its own validation, the service layer would essentially just retrieve a Student entity from the repository, call a method on it, and persist it back, potentially adding some orchestration on top of that (e. g. to facilitate the cooperation of multiple entities, raise events, etc)

4.2) I like the use of EF migrations. I have 0 experience with using these, but it looks useful at a glance.

0) git usage
This is maybe purely subjective, but I like to able to easily see what feature a commit belongs to, and ideally a commit would contain all of the required changes for that feature (excluding future bug fixes of course). If I take a look at the git project history, I see 33 sequential commits, one with a failing test and one with the test passing (if I'm counting correctly) for only the feature of updating a Student. What is the rationale behind this? This makes it almost impossible to do things like cherrypick a feature to be released separately, revert a feature that has proven problematic and needs to be rolled back, or even see what modifications a feature actually did. Also, when pushed one by one to origin (which hopefully would not happen frequently), this wastes CI server time with builds that will intentionally fail half of the time,

-1) design
Again, maybe something purely subjective, but I find it difficult to have a conversation on the structure of an entity without knowing what the actual use cases are that that part of the system will be responsible for.

[hassanhabib]

@Driedas thank you for your dedication and your detailed identification of potential areas of growth in our architecture and technology.
let's start discussing your point one by one so we don't lose track.

1. project structure
   services, entities, API controllers everything in a single project. This would not be an issue on a small, single use, single logical deployment project. For anything larger than that, you will most likely have separate components, deployed separately (at least logically if not physically). For example handling background processes like sending emails, scheduled jobs, etc. All of these would need to host everything, including the API. It would make more sense to me to at least split out the model and services into separate assemblies and having the API as just an (or one of) optional communication layer on top of that.

The idea of building our components as brokers, services, controllers and models is that we are building vertical slices of flows for particular entities so eventually down the road it would be easier to split those off as microservices.
Each microservice should by a slice of controller -> service -> broker and so on.
It doesn't seem like a good idea to think about componentization and modularization at this early stage, we have to start with one big monolith then later on break it down into microservices or components however you want to materialize them.

We try to build a monolith with a microservices mindset. meaning that we are building our monolith intentionally to be broken down eventually once the application gets larger (and it will) - and when that happen offline and online processes will need to be their own microservices, we will make that call when we get to that point, but for now, we shouldn't start optimizing for a workload and a size that we are not sure of yet.

A lot of people have asked me about the plan or big picture, or the high level goals, this is also very legit concern, however, as we are still simply just building the familiar entities of the system, I think that building these relationships and overviews are a bit too early.

We know that we want to build a schooling system, and that the system has students, classes, courses and teachers and exams, great let's start building those with full CRUD operations then we will discuss where to go from there, I like to plan for the foreseeable future not a year down the road, because I can guarantee you right now, that whatever we plan now is going to almost fully change by the time we get to where we are going, I am not a big fan of waterfalls :-)

Let's discuss this first, then we will move on to the other points, I will use your initial post as an overview of what this thread's discussion is all about, does that sound like a good plan?

[Driedas]

We try to build a monolith with a microservices mindset. meaning that we are building our monolith intentionally to be broken down eventually once the application gets larger (and it will) - and when that happen offline and online processes will need to be their own microservices

That is fair enough, I can see this potentially working, however I am skeptical that the core domain (students, teachers, guardians, courses, exams) is ever going to be split into multiple microservices in the future as that is the very core functionality of the system. Who knows, hard to predict the future, I agree.
What I do see though is non-core concepts already bleeding over to the core, like UserId. Authentication is not a part of the core domain, the entities should not handle functionality like this. The opposite direction would make more sense to me, i. e. an authentication microservice whose only concern will be authentication (likely built on some product that already handles this domain) and the mapping of identities to students/teachers/guardians via a standard mechanism like claims.

let's start building those with full CRUD operations

I don't see the core domain as a good candidate for a CRUD style approach (you don't create or update or delete a student or teacher), that's not how future users visualize an education system so there would be misunderstandings around the mappings of concepts like a student update to mark a student as graduated. There are more interesting concepts (and a ubiquitous language to be found), but this is a discussion for a later point.

Let's discuss this first, then we will move on to the other points, I will use your initial post as an overview of what this thread's discussion is all about, does that sound like a good plan?

Awesome, I would also hope that at some point others would join in as well with their own suggestions and critique of my points as well.

[hidegh]

@Driedas I also expressed my concerns over some architectural details, but must admit, not in such details as you did.

1. Feature folders - to keep things belonging together - this is similar as not splitting service into multiple files.
2. [FAIL] Entities as contracts - returning entities from the controller (I haven't even noticed it, as never ever would have thought that I do see it in any code) is not advised. Any internal change could easily break the entire API and all external systems depending on it. Not to mention some issues with serializing (can have cyclic data) and also the data (amount) sent will be harder controllable.
3. [FAIL] The abstraction (via Broker) over EF - don't get the point of it. It's just a repetitive tasks. Not to mention that now every single action there will immediately trigger an SQL call due the Save command (and what about eager fetching). This also raises an alarm: where are the transactions then!? I could imagine some query classes for every entity (in case of complex queries), but this brokers don't make sense (any more - because some similar code I have written 10 years before)... If somebody does not know why too much SQL call in the HTTP context (with .NET) not a good idea, maybe he should check out Udi Dahan's SOA course.
4. [FAIL] Too much exceptions (AOP usage is prefered), from the API part usually we do need to distinguish: 401 (authentication), 403 (autorization), 404 (not found - System.InvalidOperationException: Sequence contains no elements), 422 (validation failed), 409 (state conflict) and 500 (generic error) - now whether it's student or teacher not found - well, based on the method name which executes (call stack) it's probably easily "guessable". The only extra thing I'd do here is probably the one EF related "NotFound" exception that i'd transalte to a more specific NotFound exception (of course via middleware).
5. Validation - again, cross cutting concerns - AOP, I menitoned the Fluent Validation Framework, if we need to test nested properties, this is more safe to use, opposing to any less tested error-prone custom code.
6. Logging - again, cross cutting concerns, nothing to do in the controller, some tracing inside services, but error logging not!
7. 5 & 6 can be done via filters, but (even if I haven't used it on my small scale samples) MediatR is something worth to consider, it allows to solve 5 & 6 in an elegant way, resulting in a very slim controller code.
8. Middleware to ensure predicable (always same) error result (for API and for MVC/HTML content)

---

Regarding the GIT structure and commits and "unit" testing:

1. The red-green unit test cycle is about the development process. Committing a failing then a succeeding test is pointless. If the feature is complex, there's a likelihood I break other test as well.
2. The point should be this: we should create unit tests both for positive and negative situations (must success, must fail) and all should pass.
3. So the committing each phase somehow does not make sense.

---

Here the style how brokers are implemented are much "confusing". With non detached (and tracked) entities we do have much more comfort doing unit tests - especially with manually assigned guids (PK). Due to the ORM graph we can set up any situation in memory inside the entities and do the checks on those related objects. Would not say that I don't see some perv-like beauty in the no-tracking and repo-like (well, not repo as not aggregates) approach, but not sure this is the way EF should work (sure, this is the way how DAPPER or plain SQL works - worked, before ORM, so before 2004).

---

Would also check the broker's get-by-id methods, as they use no-tracking queries. This is normal for querying read-only data, for some reporting and so (see: https://docs.microsoft.com/en-us/ef/core/querying/tracking). In our case it has some side effects:

1. every time you request the same student, the db will be hit again and again,
2. any change on the object via broker will immediately trigger a SQL operation,
3. every request via get-by-id will return a new entity, so un-persisted changes on one won't be reflected on the newly acquired ones

---

Not sure if I posted it here, but these guys are more-over using similar approaches as I described above, nicely documented and explained: https://rules.ssw.com.au/rules-to-better-clean-architecture

---

Now whether monolith or micro-services: clean code and proper architecture belongs to every code base (as junior developers do copy-paste, so if you start with eased rules, you end up with...). I know, from the compiler perspective it's equal what you got, but if you want to keep this project a living, extensible one, then readability and simplicity MATTERS! < @hassanhabib

---

Ohh, btw: CRUD. I think there was a tool generating UI (for .NET) based on entities which required CRUD - and well, I can't remember, nor sure if ti still exists, but also a lot of banking stuff is processed via EXCEL :)

[Driedas]

@hidegh entities and their serialization issues is a good point I missed. Given a rich model (e. g. a Student has a list of Subjects he attends and the Subject has a collection of Students that attend it) a serializer can easily keep on going and going when lazy loading is enabled. This is one of the very few instances where I saw a production server going down in flames with an out of memory exception :-)

[hassanhabib]

We try to build a monolith with a microservices mindset. meaning that we are building our monolith intentionally to be broken down eventually once the application gets larger (and it will) - and when that happen offline and online processes will need to be their own microservices

That is fair enough, I can see this potentially working, however I am skeptical that the core domain (students, teachers, guardians, courses, exams) is ever going to be split into multiple microservices in the future as that is the very core functionality of the system. Who knows, hard to predict the future, I agree.
What I do see though is non-core concepts already bleeding over to the core, like UserId. Authentication is not a part of the core domain, the entities should not handle functionality like this. The opposite direction would make more sense to me, i. e. an authentication microservice whose only concern will be authentication (likely built on some product that already handles this domain) and the mapping of identities to students/teachers/guardians via a standard mechanism like claims.

let's start building those with full CRUD operations

I don't see the core domain as a good candidate for a CRUD style approach (you don't create or update or delete a student or teacher), that's not how future users visualize an education system so there would be misunderstandings around the mappings of concepts like a student update to mark a student as graduated. There are more interesting concepts (and a ubiquitous language to be found), but this is a discussion for a later point.

Let's discuss this first, then we will move on to the other points, I will use your initial post as an overview of what this thread's discussion is all about, does that sound like a good plan?

Awesome, I would also hope that at some point others would join in as well with their own suggestions and critique of my points as well.

@Driedas we create full CRUD operations on all entities not necessarily for business purposes, for instance, the delete or delete all operations, we have those so we can clean up what we create in our acceptance tests.
CRUD operations are also important to be able to modify a particular entity to validate a specific validation rule, the same thing goes with reporting for mid-layer entities that you don't necessarily have access to through admin or client portals there are so many scenarios there not all of them necessarily business related.

Now, regarding creating or deleting students ..etc - there's a higher layer of services that I call processing these processing services take care of these very specific business terminology and flows (as long as they don't intersect with other entities, in which case they become orchesterators, coordinators or aggregators depends on how complex your business is)- watch this video to have a better view about these layers - the broker-neighboring services only run CRUD for entities, but any higher layer services will tell you something like: DeactivateStudentAccount and ExpelStudent ... etc etc.

These higher layer services might combine two or more basic operations from a broker-neighboring service to create a business value, sometimes it just calls one operation and give a different name and so on.
Does that make sense?

[Driedas]

@Driedas we create full CRUD operations on all entities not necessarily for business purposes, for instance, the delete or delete all operations, we have those so we can clean up what we create in our acceptance tests.

Why not just create a throwaway database as part of the execution of acceptance tests? On test startup you generate an empty database with seed data and every test resets the database to the initial state (in whatever way, restore from a backup of the database generated in the beginning, or something like Respawn https://github.com/jbogard/Respawn).
Given you now have API endpoints for these operations, you have a vector of attack for potential bad actors, also service methods that anyone can call, but - if I understand you correctly - shouldn't outside of the scope of tests.

CRUD operations are also important to be able to modify a particular entity to validate a specific validation rule, the same thing goes with reporting for mid-layer entities that you don't necessarily have access to through admin or client portals there are so many scenarios there not all of them necessarily business related.

If those CRUD operations are there for testing and reporting, sooner or later somebody will call them as part of his use case (because why not, they are there after all) and introduce inconsistencies by modifying data that only the system is allowed to. I wouldn't trust an audit trail of a system that even exposes CRUD semantics on top of its core domain.
For reports, it's best to bypass the model (as anemic as it currently is) and EF core entirely anyway and create custom projections that can be optimized on the database server side. EF core sucks for queries that are only slightly more complex than a query by ID - last time, not that long ago, I checked EF core executed aggregate operations on the client.

Now, regarding creating or deleting students ..etc - there's a higher layer of services that I call processing these processing services take care of these very specific business terminology and flows (as long as they don't intersect with other entities, in which case they become orchesterators, coordinators or aggregators depends on how complex your business is)- watch this video to have a better view about these layers - the broker-neighboring services only run CRUD for entities, but any higher layer services will tell you something like: DeactivateStudentAccount and ExpelStudent ... etc etc.
These higher layer services might combine two or more basic operations from a broker-neighboring service to create a business value, sometimes it just calls one operation and give a different name and so on.
Does that make sense?

How and why? The current vertical slice already exposes CRUDs over the API. That's as high a layer as you can get. I think a sample would go a long way towards illustrating how that is supposed to look like. I saw the video before, and I agree on the public interface that should be exposed via the API (e. g. /students/1/deactivate) but that's not what I'm seeing in the vertical slice at the moment. Instead I see exposed CRUD methods that allow anyone (authenticated I assume?) to do anything.

[hassanhabib]

@Driedas we create full CRUD operations on all entities not necessarily for business purposes, for instance, the delete or delete all operations, we have those so we can clean up what we create in our acceptance tests.
Why not just create a throwaway database as part of the execution of acceptance tests? On test startup you generate an empty database with seed data and every test resets the database to the initial state (in whatever way, restore from a backup of the database generated in the beginning, or something like Respawn https://github.com/jbogard/Respawn).
Given you now have API endpoints for these operations, you have a vector of attack for potential bad actors, also service methods that anyone can call, but - if I understand you correctly - shouldn't outside of the scope of tests.

Creating a throwing away a temporary database doesn't give me the actual acceptance results considering we want to run this against live instances in the cloud - in which case that resetting option isn't really an option simply because we can't throw away our production database.
There's an infrastructural readiness aspect of these tests that is not yet visible because we are at early stages of the development process, Ideally I want to know that I can run a full happy path against the exact same components in my production environments and still be able to clean up the data I created without touching any other records.

CRUD operations are also important to be able to modify a particular entity to validate a specific validation rule, the same thing goes with reporting for mid-layer entities that you don't necessarily have access to through admin or client portals there are so many scenarios there not all of them necessarily business related.

If those CRUD operations are there for testing and reporting, sooner or later somebody will call them as part of his use case (because why not, they are there after all) and introduce inconsistencies by modifying data that only the system is allowed to. I wouldn't trust an audit trail of a system that even exposes CRUD semantics on top of its core domain.
For reports, it's best to bypass the model (as anemic as it currently is) and EF core entirely anyway and create custom projections that can be optimized on the database server side. EF core sucks for queries that are only slightly more complex than a query by ID - last time, not that long ago, I checked EF core executed aggregate operations on the client.

a lot of these non-business related CRUD operations endpoints will be private endpoints, hiding behind a middleware that requires particular roles, permissions and headers to be exposed and available only during particular test scenarios or reporting scenarios that team has to agree on, it's not free for all as some may think.
But the concept of private endpoints is very foreign, this is not the secure Azure private endpoints as some may think, it's a bit more than that - I will expand on that concept when the time comes.
You are jumping the gun a bit about what the end product is going to look like, there's more to what's being built currently than what's actually going to be delivered, exposed and deployed - we are just going to have to discuss that when we get there.

I am not sure what the complexity is of the queries that you have used with EF Core, I can only speak from my experience with large datasets, EF Core seemed to be doing a good job, but that's a different discussion for a different day.

Now, regarding creating or deleting students ..etc - there's a higher layer of services that I call processing these processing services take care of these very specific business terminology and flows (as long as they don't intersect with other entities, in which case they become orchesterators, coordinators or aggregators depends on how complex your business is)- watch this video to have a better view about these layers - the broker-neighboring services only run CRUD for entities, but any higher layer services will tell you something like: DeactivateStudentAccount and ExpelStudent ... etc etc.
These higher layer services might combine two or more basic operations from a broker-neighboring service to create a business value, sometimes it just calls one operation and give a different name and so on.
Does that make sense?

How and why? The current vertical slice already exposes CRUDs over the API. That's as high a layer as you can get. I think a sample would go a long way towards illustrating how that is supposed to look like. I saw the video before, and I agree on the public interface that should be exposed via the API (e. g. /students/1/deactivate) but that's not what I'm seeing in the vertical slice at the moment. Instead I see exposed CRUD methods that allow anyone (authenticated I assume?) to do anything.

As I said, these operations some of them are private endpoints, exposed for particular cases, if hit from outside they return 404 as in the URL itself doesn't exist, I don't have any published materials about this yet, but hopefully I will blog/vlog about it when time allows.

[Driedas]

Right, I get it, there is a bigger picture that I am not seeing yet. I'll take you by your word that there is a bigger picture and these pieces will somehow fall into place. To be honest, at the moment all of this seems to me like an extremely expensive and unnecessarily complex test suite. I hope I'll get to eat my words with a side of humble pie down the line :-)

[hidegh]

@Driedas @hassanhabib
regarding testing and throw away database and crud brokers (and to it's mocked versions)

with ORM the nice thing is, (if you avoid committing TX and/or using SaveChanges) that all your changes should be on the in memory ORM model. so you can do tests against! of course this works with some sample DB (or some seeding - even if in memory, or in a pre-seeded test DB).

is this not worth to consider?

---

There's one thing I did not really get, @hassanhabib wrote:

Creating a throwing away a temporary database doesn't give me the actual acceptance results considering we want to run this against live instances in the cloud - in which case that resetting option isn't really an option simply because we can't throw away our production database.

It wasn't clear, but I assume we're talking about a copy of the production database - as there should be no tests executed on a production database, because...

So in this case it's just a copy you we're referring to. Now:

• how does this copy differ from any temporary pre-seeded DB?
• And why should any kind of test depend on real, non static (changing) data?
• What test cases should such unit test cover, considering that it's a CRUD (anemic) domain environment we're creating... I mean in such cases here are even no stable data-rules inside or between entities enforced by the business logic - there's nothing really assumptions regard data we can rely on

---

Ohh and EF core: still miss SQL side grouping support (at least with OData it sill failed, see: dotnet/efcore#10012)

[chaouanabil]

Hi everyone,
Thank you @hassanhabib for this initiative, i did read the discussion about the architecturals and design choices. And i would like if you take a look guys in abp framework https://abp.io/, because it promote to give a solution to a lot of points mentionned here like rich model vs anemic, cross cutting concernes like entities validation , business rules validation, exception handling , authentication , tracing , auditing ...ect.

[hassanhabib]

Hi everyone,
Thank you @hassanhabib for this initiative, i did read the discussion about the architecturals and design choices. And i would like if you take a look guys in abp framework https://abp.io/, because it promote to give a solution to a lot of points mentionned here like rich model vs anemic, cross cutting concernes like entities validation , business rules validation, exception handling , authentication , tracing , auditing ...ect.

@chaouanabil thanks for sharing - this framework looks interesting definitely something to consider.

[hassanhabib]

@Driedas @hassanhabib
regarding testing and throw away database and crud brokers (and to it's mocked versions)

with ORM the nice thing is, (if you avoid committing TX and/or using SaveChanges) that all your changes should be on the in memory ORM model. so you can do tests against! of course this works with some sample DB (or some seeding - even if in memory, or in a pre-seeded test DB).

is this not worth to consider?

There's one thing I did not really get, @hassanhabib wrote:

Creating a throwing away a temporary database doesn't give me the actual acceptance results considering we want to run this against live instances in the cloud - in which case that resetting option isn't really an option simply because we can't throw away our production database.

It wasn't clear, but I assume we're talking about a copy of the production database - as there should be no tests executed on a production database, because...

So in this case it's just a copy you we're referring to. Now:

• how does this copy differ from any temporary pre-seeded DB?
• And why should any kind of test depend on real, non static (changing) data?
• What test cases should such unit test cover, considering that it's a CRUD (anemic) domain environment we're creating... I mean in such cases here are even no stable data-rules inside or between entities enforced by the business logic - there's nothing really assumptions regard data we can rely on

Ohh and EF core: still miss SQL side grouping support (at least with OData it sill failed, see: dotnet/efcore#10012)

@hidegh I wasn't talking about a copy from a production database, I was talking about the actual database in production, running our acceptance tests against our production API and making sure we can run full cycle of CRUD operations (and orchestration, coordination, management and aggregation in the future) against the actual instance that everyone else is using.

These tests we run are great, but they are limited by the place and time they are running within. it still doesn't report the overall health status of the instance that everyone else is using and that's what I want a status report about.
If we run this in memory, we are not really getting the true status of our production environments, and therefore we are not getting a true health check/scan of full cycles of operations within our production instances. does that make sense?

Now, as of the grouping support and OData, I am still not sure where grouping fits in our operations - I won't be looking for a feature that I don't need yet to toss off an entire technology stack, if we go this way we won't be using any technologies at all and we will be building everything by hand.
Luckily we are abstracting away our usage of EF in Brokers, if we decide to do Dapper or even raw SQL it will be as easy as simply changing a method implementation in our Brokers without touching our business layer/services.
Your earlier comment about using Brokers is a failure because it does eager fetching makes me wonder if you've actually read the code, our IQueryable SelectAll functions don't do eager fetching at all, I would recommend reviewing the content I shared and looking thoroughly at the code being developed to get a better understanding of what's being already accomplished.

Will address other concerns as soon as I get the time, and I appreciate the feedback.

[hassanhabib]

Shutting this down due to inactivity, feel free to revive if you have feedback.

[devmanzur]

I was wondering why are we throwing exceptions for validation cases? From what I have learned throwing exceptions for expected failures such as validation is not good practice. Or is throwing exceptions ok if we are catching them immediately? I do not question the architecture or decisions just want to know your thought on this @hassanhabib . Thank you.

[hassanhabib]

@devmanzur it depends on what you do with the exceptions.
In our case here exceptions are used to either terminate the current operation and report an error to the API consumer to fix their requests or an internal issue in which we report 5xx exceptions to end users and notify the engineers to handle the issue through logging.
we catch these inner exceptions for specific cases and wrap them in higher order exceptions so we can decide whether to just communicate a generic Bad Request 400 error or specify the error based on the case.
Exceptions are just as import as part of our business logic as everything else, the only difference is that we throw them when we decide the entire operation is invalid and we require external action to restart the operation.

Does that make sense?

[devmanzur]

I guess it makes sense for this context. Throwing exceptions makes the overall code simpler, But then we require creating a new exception type for each and every possible validation type.

[hassanhabib]

Awesome - please feel free to re-open if you have any further questions.

[youssefbennour]

@hassanhabib
I've read that exceptions are very expensive and shouldn't be thrown for trivial validation scenarios (raising an exception for an invalid user password doesn't seem to be the right thing to do), so are there any good reason to stick with exceptions instead of using something like the 'ServiceResult' pattern ?

[devmanzur]

@youssefbennour I have been there, as you can see the last one to question this behavior 4 years ago was me :p. Result pattern is a valid approach and i use that too. Quoting the last sentence from @hassanhabib "Exceptions are just as import as part of our business logic as everything else, the only difference is that we throw them when we decide the entire operation is invalid and we require external action to restart the operation.".

As you will see in the framework itself. you can either choose to use SingleAsync or SingleOrDefaultAsync. one throws exception straight away, another allows you to handle the null value and do as you like. You will find both of them are valid approach and it comes down to design decision.

Exceptions are expensive but allowing user to continue when you encounter a invalid state might be more expensive.