Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce Directory.build.props files and unify namespaces #355

Merged
merged 37 commits into from
Oct 26, 2023

Conversation

tnotheis
Copy link
Member

@tnotheis tnotheis commented Oct 23, 2023

Readiness checklist

  • I added/updated unit tests.
  • I added/updated integration tests.
  • I ensured that the PR title is good enough for the changelog.
  • I labeled the PR.

This PR contains 2 refactorings:

  1. I introduced a Directory.build.props file. This file helps us define common properties for all .csproj files. For example, I defined the RootNamespace in this file. Actually, I even added two such files. In the Modules folder, I added another one, which is used to set the namespace to Backbone.Modules.* (compared to Backbone.*, which it would be without the file).
  2. I then used the file to set RootNamespace for all files, which required me to modify a lot of files. But actually, these are all just namespace and using statements.

The goal of the second refactoring (next to unification) is that it will be simpler to set log levels for all Backbone components. Before my change we needed something like:

  "Logging": {
    "MinimumLevel": {
      "Default": "Information",
      "Override": {
        "AdminUi": "Debug",
        "Backbone": "Debug",
        "Enmeshed": "Debug",
        "ConsumerApi": "Debug"
      }
    }
  }

Now we just need:

  "Logging": {
    "MinimumLevel": {
      "Default": "Information",
      "Override": {
        "Backbone": "Debug"
      }
    }
  }

Also, there are two fixes that must have caused problems in the pipelines for quite some time and that for some reason let the integration tests only for me:

  1. The retry behavior @Dannyps implemented in the event busses had a flaw: when they actually retried something that caused an exception AFTER something was added to the dbcontext but BEFORE the DB transaction was commited, there was an error on every succeeding try, because the object that was trying to be added was already there. The solution for this was to create a new Service Provider scope and resolve a new IntegrationEventHandler instance (and therefore create a new DbContext, which didn't have the added entity). See corresponding commit
  2. The DB seeders were called at the wrong time during the startup logic. Before my change, the seeder for a specific DbContext was always called right after the migrations for said DbContext were applied. The problem with that is that the seeder for Devices creates a Tier, which triggers a TierCreatedIntegrationEvent the QuotasModule listens to, in order to create a corresponding Tier entity in its own schema. The problem is: at the time the event is received, the tables in the Quotas module don't exist yet, because the migrations are applied afterwards. I'm not quite sure why this led to failing tests in my branch but not in the other branches, but at least it's fixed now. See the new solution.

@tnotheis tnotheis added the refactoring Refactoring of code label Oct 23, 2023
@tnotheis tnotheis enabled auto-merge (squash) October 26, 2023 08:20
@tnotheis tnotheis merged commit e481161 into main Oct 26, 2023
@tnotheis tnotheis deleted the introduce-directory-build-props-file branch October 26, 2023 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Refactoring of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants