Fix nodesGetAll handler

[addievo]

Description

nodesGetAll handler was obsolete and was breaking operation of nodes getAll on Polykey-CLI.

This simple PR just fixes the handler.

Handler previously was:

for await (const bucket of nodeGraph.getBuckets()) {
      let index;
      for (const id of Object.keys(bucket)) {
        const encodedId = nodesUtils.encodeNodeId(
          IdInternal.fromString<NodeId>(id),
        );
        // For every node in every bucket, add it to our message
        if (!index) {
          index = nodesUtils.bucketIndex(
            keyRing.getNodeId(),
            IdInternal.fromString<NodeId>(id),
          );
        }
        if (ctx.signal.aborted) throw ctx.signal.reason;
        yield {
          bucketIndex: index,
          nodeIdEncoded: encodedId,
          host: bucket[id].address.host,
          port: bucket[id].address.port,
        };
      }
    }

And now is changed to:

  for await (const [index, bucket] of nodeGraph.getBuckets()) {
      for (const [id, info] of bucket) {
        const encodedId = nodesUtils.encodeNodeId(keyRing.getNodeId());
        // For every node in every bucket, add it to our message
        if (ctx.signal.aborted) {
          throw ctx.signal.reason;
        }
        yield {
          bucketIndex: index,
          nodeIdEncoded: encodedId,
          host: info.address.host,
          port: info.address.port,
        };
      }
    }

Issues Fixed

• Related CLI polish and fixes for deployment Polykey-CLI#44
• Fixes getAll handler is currently broken and needs fixing.  #610

Tasks

• 1. Update handler.
• 2. Verify it works on Polykey-CLI.

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[ghost]

👇 Click on the image for a new way to code review

Legend

[tegefaulkes]

FYI, you don't need to code dump in the issue spec. Implementation detail is less relevant than requirements.

You want clear specification of what needs to be done, How the code looks like isn't that use. Focus on what needs to be done and what is required of the code. Not how it will look.

[tegefaulkes]

You also need to review the tests for this. no tests caught the problem so either they are insufficient or non-existent. Look into it.

[addievo]

You also need to review the tests for this. no tests caught the problem so either they are insufficient or non-existent. Look into it.

You are right, currently no tests exist for getAll. I will make one to test the same.

[tegefaulkes]

Likely there is no test for the Nodes Connections as well.

[addievo]

Likely there is no test for the Nodes Connections as well.

Yep, there are currently only tests for add, claim, find, and ping.

[tegefaulkes]

Just a little more polish, Once that is done, so long as all the test pass, you can proceed to merge.

[CMCDragonkai]

Good work!