[java-shell] Fix createIndexes. Core did not pass dbOptions as a last argument

[kornilova203]

Note that it contains a small change in core code in collection.ts.
dbOptions there were not passed as last argument to serviceProvider.createIndexes that caused Arity error - expected: 3 actual: 4

[addaleax]

@aherlihy Should this be

Suggested change

	return await this._mongo._serviceProvider.createIndexes(this._database._name, this._name, [spec], this._database._baseOptions);
	return await this._mongo._serviceProvider.createIndexes(this._database._name, this._name, [spec], options, this._database._baseOptions);

? Or should the options really be passed as part of the index spec?

[kornilova203]

Looks like node.js driver expects only session in options and other properties in spec.
So maybe more refactoring and unification is required in core

[aherlihy]

We should definitely add more docs/tests since this is confusing, but there is a bug in the original code. That being said Ludmilla's more correct for this one, although we need to tweak the entire method a bit (which can happen in a different ticket).

The shell API allows users to pass index-specific options via the options argument, which are described here: https://docs.mongodb.com/manual/reference/method/db.collection.createIndex/index.html#options-for-all-index-types

The node driver it seems, can take those indexes-specific options as part of the index spec or as a second argument. Right now it looks like we are adding it to the spec (which is why the last two args of ServiceProvider.createIndexes were skipped).

But the session option cannot be set via the index spec and instead needs to be sent via options, so we had a bug where we were sending the session option via the index spec which would get ignored.

Note there are no DatabaseOptions to be sent at all so the last argument to service provider.createIndexes should still be empty or {}. (this._database._baseOptions are command options for all commands associated with that database, and not actual DatabaseOptions).

I think a better alternative for everyone is to try separating out the options from the index spec. So the method would look like:

@returnsPromise
  async createIndex(
    keys: Document,
    options: Document = {}
  ): Promise<Document> {
    assertArgsDefined(keys);
    if (typeof options !== 'object' || Array.isArray(options)) {
      throw new MongoshInvalidInputError('The "options" argument must be an object.');
    }
    this._emitCollectionApiCall('createIndex', { keys, options });

    return await this._mongo._serviceProvider.createIndexes(this._database._name, this._name, [{ key: keys }], { ...this._database._baseOptions, ...options});
  }

but this change should come with tests and such so I can make it separately: https://jira.mongodb.org/browse/MONGOSH-478

Sorry for the super long response!

[kornilova203]

@addaleax,
I've updated this branch
I changed all calls to createIndexes(this._database._name, this._name, specs, {}, this._database._baseOptions);. Note second to last parameter {}.
According to docs, this options parameter can contain a Session which seems to be not applicable here and commitQuorum which currently isn't passed to Collection.createIndexes

[addaleax]

Yeah, I think at the very least this won’t break anything, so it should be fine :)

[kornilova203]

I cannot see why tests failed because I don't have mongodb account
@addaleax could you please tell me?

[addaleax]

@kornilova203

[2020/11/27 10:47:21.175]   1025 passing (4m)
[2020/11/27 10:47:21.175]   1 failing
[2020/11/27 10:47:21.175]   1) Collection
[2020/11/27 10:47:21.175]        with session
[2020/11/27 10:47:21.175]          all commands that use other methods:
[2020/11/27 10:47:21.175]      AssertionError: expected undefined to equal undefined
[2020/11/27 10:47:21.175]       at Context.<anonymous> (src/collection.spec.ts:1282:35)

This is from the shell-api package … I assume this has something to do with the changes to the options handling here? Can you maybe try to run the tests for that package locally?

[kornilova203]

I fixed the test