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

Multiple points for the improvements #281

Open
lmolotii opened this issue Feb 2, 2022 · 1 comment
Open

Multiple points for the improvements #281

lmolotii opened this issue Feb 2, 2022 · 1 comment

Comments

@lmolotii
Copy link

lmolotii commented Feb 2, 2022

Summary

Currently, MongoFramework is a nice tool for small projects, however, it still requires a lot of improvements.

Issue 1: _t value

When the persistence layer writes any document into the collection, MongoFramework introduces an additional field called "_t" with the name of the type. Something like:
_t: "LocationDocument"

In addition to that, it introduces the same field into each subdocument in the collection. I don't want to know what could happen if one of my software engineers will rename the document object class. Unfortunately, I found no options to disable this behavior.

Issue 2: Compound indexes

In our situation, we have two properties inside the document:

  • reviewers
  • watchers

Each of the properties is represented as a collection of Assignee. If I set the [Index] property inside the Assignee class, the framework will automatically create the following index:
CompanyId, Department.Watchers._id, Department.Reviewers._id

I would rather have two separate indexes like instead of one mentioned above:

  • CompanyId, Department.Watchers._id
  • CompanyId, Department.Reviewers._id

Issue 3: IMongoDbConnection as IDisposable

Frankly, I don't understand why there is a need to have this structure as IDisposable if it doesn't use any disposable resources inside.

Issue 4: Database name as a part of a connection string cause MongoDB.Driver.MongoAuthenticationException

If somebody would like to reproduce the error, they could do the following

  1. Setup the mongodb using the following docker-compose file
version: '3.7'

services:
  mongodb:
    container_name: mongodb4.4
    hostname: "mongo"
    image: mongo:4.4.0
    volumes:
      - mongodb_data:/data/db
    ports:
      - 27017:27017
    environment:
      MONGO_INITDB_ROOT_PASSWORD: "password1234"
      MONGO_HOSTNAME: "mongo"
      MONGO_INITDB_ROOT_USERNAME: "root"
    networks:
      - app-tier
volumes:
  mongodb_data:
  1. Configure connection in the following way:
    mongodb://root:password1234@localhost:27017/myservice-db

  2. Run the application and observe the error:

MongoDB.Driver.MongoAuthenticationException: Unable to authenticate using sasl protocol mechanism SCRAM-SHA-1.

I found the following workaround to fix it (basically I needed to implement my own IMongoDbConnection), here is the implementation sample:

    public sealed class DocumentDbConnection : IMongoDbConnection
    {
        private readonly MongoClient _client;
        private readonly IMongoDatabase _database;

        public DocumentDbConnection(IOptions<MongoDbOptions> options)
        {
            var config = options.Value;
            _client = new MongoClient(config.Connection);
            _database = _client.GetDatabase(config.DbName);
        }
        
        public IMongoClient Client => _client;
        
        public IDiagnosticListener DiagnosticListener { get; set; } = new NoOpDiagnosticListener();
        
        public IMongoDatabase GetDatabase()
        {
            return _database;
        }

        public void Dispose()
        {
        }
    }

Thanks for your great efforts to make the current MongoDB .Net environment a bit friendlier.

@Turnerj
Copy link
Member

Turnerj commented Feb 3, 2022

Hey @lmolotii - thanks for raising these issues!

Issue 1: _t value

That's actually the MongoDB C# driver, which MongoFramework sits on top of, doing that. I have the same concerns you have and I don't currently have a perfect strategy to solve it.

Because of inheritance, _t is kinda needed to discern between subtypes. Like, it could try and determine by properties/fields on a type but that is a very expensive lookup to do.

Maybe if there was a way to describe that certain objects do not have inheritance at all, then you might be able to get away with it. Gets complicated if you use an interface as the type on the model but have varied concrete types too.

A method to support migration could help but that adds a whole bunch of different complications. Alternatively, I thought about perhaps attributes that could specify type aliases so if you do change the name, you'd need to just specify what the old name was as an attribute (though this likely has the drawback of making the existing name off-limits indefinitely).

But yeah, the type discriminators certainly have a few big issues:

Issue 2: Compound indexes

What you want to do is set a name on the index. I show an example in the readme but I haven't really described what it does. Basically you can have the Index attribute multiple times on the same property with different names. Each name group becomes a new compound index. When you don't specify a name, they all join as part of the same group (what you're experiencing).

Issue 3: IMongoDbConnection as IDisposable

I've been asked that question before and wrote a lengthy response about it here: #162 (comment)

The only additional thing I'd add is to not be concerned about the implementation - whether it is disposes anything or not. The implementation could change at any point in the future and then be disposed. It being disposable now is effectively preventing that type of breaking change in the future.

Issue 4: Database name as a part of a connection string cause MongoDB.Driver.MongoAuthenticationException

While I've seen that issue before, I haven't encountered it for a while. As far as I can tell from doing a bit of research now, it seems to be by-design of how the connection strings work. The database in the connection string is used, by default, for authentication. This can, however, be overridden by specifying ?authSource=admin.

What makes this a little more confusing is that I currently use MongoDB Atlas for my own projects and I specify the database I want in the connection string - it doesn't have the same issue.

That said, I have thought about having an IOptions way of setting the connection plus an additional fluent API similar to how Entity Framework has it. They haven't been my highest priority however understanding how the database is used in the connection string by default, it does look like something that should be addressed sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants