[CCI][#240][eslint] Fixed duplicate conditions in indices and nodes API

[frost017]

Description

Summary about indices.js: This error is triggered when running yarn lint with next rule removed - "no-dupe-else-if": "off". Previously, there were else-if statements having the same condition, which means only one of them can be executed and others are ignored. These were removed because of duplication:

else if (index != null && type != null) {
  if (method == null) method = 'PUT';
  path = '/' + encodeURIComponent(index) + '/' + encodeURIComponent(type) + '/' + '_mapping';
} else if (index != null && type != null) {
  if (method == null) method = 'PUT';
  path = '/' + encodeURIComponent(index) + '/' + '_mapping' + '/' + encodeURIComponent(type);
} else if (index != null && type != null) {
  if (method == null) method = 'PUT';
  path = '/' + encodeURIComponent(index) + '/' + encodeURIComponent(type) + '/' + '_mappings';
} else if (index != null && type != null) {
  if (method == null) method = 'PUT';
  path = '/' + encodeURIComponent(index) + '/' + '_mappings' + '/' + encodeURIComponent(type);
}

The code above resulted in following:

  1572:14  error  This branch can never execute. Its condition is a duplicate or covered by previous conditions in the if-else-if chain  no-dupe-else-if
  1575:14  error  This branch can never execute. Its condition is a duplicate or covered by previous conditions in the if-else-if chain  no-dupe-else-if
  1578:14  error  This branch can never execute. Its condition is a duplicate or covered by previous conditions in the if-else-if chain  no-dupe-else-if
  1587:14  error  This branch can never execute. Its condition is a duplicate or covered by previous conditions in the if-else-if chain  no-dupe-else-if

FIX in indices.js: Removed duplicate conditions and made separate if statement for each case. By doing this, path is picked correctly and cases are not ignored because of duplicate conditions.

Summary about nodes.js: Same case as in indices.js. It has duplicate conditions, all check same case. It means all four are executed, which makes three conditions to be ignored. These lines of code were removed:

if ((node_id || nodeId) != null) {
  if (method == null) method = 'GET';
  path = '/' + '_nodes' + '/' + encodeURIComponent(node_id || nodeId) + '/' + 'hot_threads';
} else if ((node_id || nodeId) != null) {
  if (method == null) method = 'GET';
  path =
    '/' +
    '_cluster' +
    '/' +
    'nodes' +
    '/' +
    encodeURIComponent(node_id || nodeId) +
    '/' +
    'hotthreads';
} else if ((node_id || nodeId) != null) {
  if (method == null) method = 'GET';
  path = '/' + '_nodes' + '/' + encodeURIComponent(node_id || nodeId) + '/' + 'hotthreads';
} else if ((node_id || nodeId) != null) {
  if (method == null) method = 'GET';
  path =
    '/' +
    '_cluster' +
    '/' +
    'nodes' +
    '/' +
    encodeURIComponent(node_id || nodeId) +
    '/' +
    'hot_threads';
}

The code above resulted in following:

  189:14  error  This branch can never execute. Its condition is a duplicate or covered by previous conditions in the if-else-if chain  no-dupe-else-if
  200:14  error  This branch can never execute. Its condition is a duplicate or covered by previous conditions in the if-else-if chain  no-dupe-else-if
  203:14  error  This branch can never execute. Its condition is a duplicate or covered by previous conditions in the if-else-if chain  no-dupe-else-if

FIX in nodes.js: Removed those aforementioned four conditions, and put one if-else condition which checks if node_id or nodeId is not null.

Issues Resolved

Following issue is resolved: #240

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[dblock]

Thanks!

I am concerned that no tests broke given what I think is a bug below. Can you please beef up the putMapping tests to make sure we cover every path of the changed if?

Delete commented out code, and any unrelated changes.

[dblock]

Are these changes related to this fix? Rebase your branch.

[frost017]

Are these changes related to this fix? Rebase your branch.

No, they aren't. It was intended to fix deprecation mapping error when using NodeJS 16 and TypeScript #365. I am removing this fix for now, leaving it as it was previously.

[dblock]

I think you didn't want this change.

[dblock]

This will make /_maping/_mapping in the last else case at least.

[frost017]

/_mapping/_mapping shouldn't be the case in any way, right?

[frost017]

I am concerned that no tests broke given what I think is a bug below. Can you please beef up the putMapping tests to make sure we cover every path of the changed if?

Yes, of course, I will do my best to cover it with tests. The thing is that I am not yet familiar with the structure of unit tests in the project, so I may put getMapping tests in the wrong place:)