Error in AbstractMFlixDao::getConfiguration

MongoDB University M220J: MongoDB for Java Developers
#1

So far, I have managed to complete the course using a local database instance. However, when trying to validate the connection pooling ticket, I got an error even though the unit test passed.

I traced this through the code to AbstractMFlixDao::getConfiguration() which builds the data used for the response in this validation.

In your version of this method, ALL of the calls to configuration.put() are wrapped inside theif (!authUserRoles.isEmpty()) { block. However, the “pool_size” and “wtimeout” parameters are not dependant on “authInfo” being available, so IMO this method should look more like the following:-

  public Map<String, Object> getConfiguration() {
    Map<String, Object> configuration = new HashMap<>();

    ConnectionString connString = new ConnectionString(connectionString);
    Bson command = new Document("connectionStatus", 1);
    Document connectionStatus = this.mongoClient.getDatabase(MFLIX_DATABASE).runCommand(command);

    List authUserRoles =
        ((Document) connectionStatus.get("authInfo")).get("authenticatedUserRoles", List.class);
    if (!authUserRoles.isEmpty()) {
      configuration.put("role", ((Document)authUserRoles.get(0)).getString("role"));
    }

    configuration.put("pool_size", connString.getMaxConnectionPoolSize());
    configuration.put(
        "wtimeout",
        this.mongoClient
            .getDatabase("mflix")
            .getWriteConcern()
            .getWTimeout(TimeUnit.MILLISECONDS));

    return configuration;
  }

By limiting the if() block to only have the single concerning parameter, the rest of the response can be correctly built even on stand-alone single mongodb instances.

#2

Hi @gargoyle,

I see where in a local deployment this can cause some issues. However, we want to ensure that you have an authentication enabled Replica Set, regardless if it is running locally or on Atlas.
This method is there to enforce that setup. Having that if clause won’t help you pass any integration testing or completing the exercise as expected, making the suggested improvement unnecessary.

N.

#3

Thanks for the comment, @Norberto.

Although the error on my single instance is what started my investigation which then lead me down the path to this method, it is not the sole reason for the code change.

The code I propose is cleaner in that actions un-related to the authentication are not contained within the if() block.

If there is a “business” case for enforcing that a replicaset with authentication enabled is used, then add an else clause to throw an appropriate exception and error message. This would be both easier to read and significantly easier to debug.

The current code makes no such enforcement about a replica set and will simply return an empty configuration object. This only then became a problem during the connection pooling exercise - something completely un-related to authentication.

#4

Gotcha,
we will be adding in this change.
Thanks for reporting!