Architecture Guidelines

[ascott18]

Some proposals for guidelines based on things I've seen recently, and over the years:

Project documentation:

1. All projects should have a README file in their repository root that includes the following information, or contains links or instructions on where to find the following information:

   1. How to setup a local development environment for working on the project, including:
      1. Prerequisite installed software (IDEs, SDKs, Databases, other tooling, recommended IDE extensions, etc.), including version requirements.
      2. How to open the project in the recommended IDE/editor (e.g. "Open the .sln file in the repository root with Visual Studio 2019")
   2. How to build and run the project, including:
      1. How to restore any packages that aren't automatically restored. For example, Nuget packages are auto-restored by dotnet tooling, but npm packages are not.
      2. How to obtain a local copy of any required database, if necessary.
   3. How to run the project's tests
   4. How to contribute to the project's tests, including a brief summary of the different types of testing that are utilized in the application (unit testing, integration testing, UI testing, etc).
   5. How to contribute changes to the primary branch of the project, including a description of branching strategy, the project's pull request process & requirements, any review processes the project is subject to, including code reviews, architecture review, security reviews, etc.
   6. How the project is deployed, including:
      1. A description of each environment that the project may be deployed to
      2. Any URL(s) where the deployed/published project may be found
      3. A description of the process for promotion from one environment to another, including necessary approvals, manual testing/verification, and actual actions that must be taken to achieve the environment promotion (e.g. release pipelines).

2. All possible sources of application configuration that is environment-dependent MUST be clearly enumerated in a README file in the root of the project's repository, or must otherwise be linked to from such a README file. This file MUST be updated when changes are made to how an application is configured.

   1. Possible sources of configuration include, but are not limited to:
      1. Injection by an automated build or release process
      2. Stored on a cloud service, such as Azure App Service configuration (injected into the app via Environment Variables), or Azure Key Vault
      3. Stored in database table(s)
      4. Stored in files checked into the repository, including appsettings.*.json, launchSettings.json, package.json, app.config, and any other file containing configuration.
      5. Stored from predefined files/directories in environments where the application is running.
      6. Hardcoded configuration (DO NOT use hardcoded configuration, but if it truly cannot be moved elsewhere, please document it.)
   2. Possible types of configuration include, but are not limited to:
      1. Any credentials, including username/password pairs and API keys
      2. Any configuration the instructs the application how to locate an external resource, including database connection strings, URLs, IP addresses, email addresses, cloud provider subscription IDs, resource IDs, or any other external resource.

General Architecture:

1. Utilize Dependency Injection in all projects whenever possible This probably needs to be expanded upon - its maybe too open-ended and vague
2. In projects utilizing dependency injection:
   1. DO NOT store global singletons in static fields/properties. Always resolve such objects from the DI container, including services, configuration, or any other singleton.
   2. DO NOT manually instantiate classes that are registered with the DI container. This is especially applicable to Entity Framework DbContext instances, because manual instantiation implies that the DbContextOptions<> are also being manually created, are hardcoded into the DbContext constructor, or are being stored as a global singleton.
      1. Exception: Instantiating DbContext instances during setup when the instance is being configured with some an in-memory provider.
   3. AVOID using the service locator pattern, PREFER using the standard mechanisms of injection provided by your DI framework (e.g. constructor injection, ASP.NET Core parameter injection via [FromServices], etc.)

Web Application Architecture:

1. When an application needs to run long-running or recurring background jobs, you SHOULD prefer Hangfire (https://www.hangfire.io/) over homegrown out-of-process job engines, manual spawning of threads, using Task.Run, or utilizing a cloud-native solution.
   1. Exceptions may be made if:
      1. The principal purpose of the web application is to spawn, facilitate, manage, execute, or monitor such background jobs, or
      2. Running background jobs in the same process as the web application will cause significant performance, stability, or security issues.
2. DO NOT write business logic in controller classes. DO write business logic in service classes that do not have any concerns or knowledge of web requests & responses. Such classes can be directly tested with unit tests, can be easily called from other services classes to facilitate code re-use and consistency.

[ascott18]

cc @MarkMichaelis

[GrantErickson]

Great ideas.

The only one I would take any issue with is the DI 'whenever possible.'

My main issue with DI is that if it isn't used in a way that is commonly accepted, it is very confusing to understand what object I am getting and where it is coming from. In ASP.NET Core, this is a great thing since it is baked in and pretty much everyone knows where it gets set up. In other cases, I have spent hours looking around to find where some custom DI container was set up.

I would add an additional item in Hangfire that if background tasks is the main thing the app does, maybe it shouldn't be a web site. Unfortunately, there are not super good multi-cloud options for this outside web.

[GrantErickson]

BTW: Seems like we should build this into some sort of MD files.

[Keboo]

@GrantErickson we already have the web page for this stuff. Just a matter of finalizing what we want it to be, then do a PR )

[Keboo]

Initial start of this covered by #107 , will circle back on individual proposals from here after that is done.

[MarkMichaelis]

Call for volunteers posted to Yammer.

[Steborino]

Regarding Project Documentation 1.i.a, in the specific scenario where there are recommended VSCode extensions, setting up workspace recommended extensions is a good means of augmenting this README.

[Steborino]

I have spent hours looking around to find where some custom DI container was set up.

At least on the .NET side, we could create an IntelliTect.Debugging (or whatever) package that may be installed into any webserver and expose any number of debug endpoints in development environments.

One such endpoint would allow a developer to enter a symbol name and see various details via reflection:

1) Installation path of the injected symbol 2) Serialized value in the current context 3) etc.

[SteveByerly]

The recommendation in `Web Application Architecture" to use Hangfire feels like it might only cater to a subset of use-cases and a bit too strong/specific a recommendation for a general set of Architecture Guidelines. I would generally recommend not maintaining a job queue stack at all if it can be avoided, at least in the types of projects we work on.

[GrantErickson]

@SteveByerly: Seems that in this case there is a caveat about "When an application needs to run long-running or recurring background jobs." I agree that these are undesirable, and we should do everything in 'real-time' if possible. I think the idea here is that if we need to run 'batch' jobs and if we are using ASP.NET, we should be using Hangfire as the default and not another 3rd party or homegrown method. One of my driving factors behind architecture choices for us is changing. We are encountering more customers who are looking to us for ongoing technical support (as opposed to where they support the product with existing staff). Having a standard set of tools to solve common problems makes it much easier to for one of our people who might not be familiar with a project to jump in and assist a customer.

[ascott18]

@GrantErickson @SteveByerly Yeah, the point about Hangfire here is to start with the smallest, simplest solution for the problem at hand. Obviously if you don't have a need for scheduled background jobs or on-demand background jobs, don't use it. Also don't use it if you're not writing a .NET application.

It should be used in preference to a tangled web of Azure Queues/Azure Functions (or other cloud-native solution) because it doesn't vendor-lock you to a cloud provider and is far, far simpler to maintain and develop. It also should be used in preference to a home-grown, probably-implemented-wrong new Thread() that you initialize on application start, or some other similarly janky solution.

Hangfire is a solution that itself is completely self-contained, and it also stays contained within the application in which it is implemented with no external dependencies other than the database that your application already has.

Of the dozen(s?) of .NET web projects I've worked on, I could count on one hand the number that have NOT had a need for some kind of background job processing. I could go into more detail here, but this is a public forum.