feat(NODE-4924)!: remove mapReduce collection helper

[baileympearson]

Description

What is changing?

Collection.mapReduce is removed.

Is there new documentation needed for these changes?

Nope.

Double check the following

• Ran npm run check:lint script
• Self-review completed using the steps outlined here
• PR title follows the correct format: type(NODE-xxxx)[!]: description
  • Example: feat(NODE-1234)!: rewriting everything in coffeescript
• Changes are covered by tests
• New TODOs have a related JIRA ticket

[addaleax]

Is it okay to remove this here? If I follow the code correctly, this method is called for all commands, including client.db().command(), so this change would also affect users who run mapReduce through that instead of the helper (and since the migration notes mention this as an alternative for cases in which users can’t use agg pipelines, that seems to be a supported use case).

[baileympearson]

I think this is a judgement call. Do you have an opinion?

When a user manually constructs the command, they can attach their own read concern. I think it's reasonable to say that because we're dropping support for mapReduce, that means we drop support for attaching readConcerns automatically when executing a runCommand.

Alternatively, we could continue to support automatically attaching the read concern to mapReduce commands. One argument in favor of this approach is that the Db.command() options does take a read concern, so users may assume that the read concern they supply as a part of the options object would get attached to their command.

[addaleax]

the Db.command() options does take a read concern, so users may assume that the read concern they supply as a part of the options object would get attached to their command.

I don’t have a strong opinion, but this seems like a reasonable expectation to me, esp. if this works for other commands.

[durran]

I think it's reasonable to to keep it in until the driver's minimum supported server version has completely removed the command.

[durran]

Just a note as well because it seemed a bit unclear here. This is only determining if the command supports read concern - it still must be explicitly provided. This change would have just ignored the provided one in the inlined mapReduce case.

[dariakp]

@baileympearson README changes LGTM

[nbbeeken]

The mapReduce retryable-reads tests are somehow skipping so this error didn't make those tests fail, I'm not seeing it in the filter in retryable_reads.spec.test.js, any ideas?

[baileympearson]

Yeah, this threw me for a loop initially too. The filter argument in generateTopologyTests returns true for tests that run, false otherwise. The filter for the retryable reads spec test is actually an inclusion filter, not exclusion.

I added the doc comment to generateTopologyTests after getting confused by that initially.

[baileympearson]

I considered a minor refactor but decided not to:

  generateTopologyTests(testSuites, testContext, spec => {
    const testsToRun = [
      ... list all test regexes here
    ];
    return testsToRun.some(regex => spec.description.match(regex));
  });

want me to make the change?

[nbbeeken]

Your call, I'm happy that we looked into it and know why they weren't run

[nbbeeken]

CI issues, I think something caused by the merge, import related