add setters and getters to notes

[msimerson]

Background: over the years we've seen many dozens of crashing bugs related to attempts to set or access object properties that don't exist. This has led to a lot of defensive code whereby plugins write code like this:

if (connection.transaction.notes.some && connection.transaction.notes.some.path && connection.transaction.notes.some.path.property) {
     // do the needful
}

Much of this was mitigated with haraka-results as it provided standard locations for various types of data as well as setters and getters. We should do the same for notes, implemented such that a pleasant and easy syntax such as this works:

connection.transaction.notes.get('queue.wants');
connection.transaction.notes.set('queue.wants', 'qmail');
if (connection.transaction.notes.get('queue.wants') === 'smtp_forward') {...
connection.notes.get('foo.bar.baz');

TODO

• [x] create haraka-notes repo
• [x] export a notes object with .get() and .set() properties
  • [x] prevents get/set from getting reassigned.
• create instances of notes on:
  • [x] new connections #2082
  • [x] new transactions
  • [x] haraka-test-fixtures connections haraka/haraka-test-fixtures#24
  • [x] haraka-test-fixtures transactions

Stretch Goals

• [ ] retrofit existing plugins to use the get/set functions.
• [ ] retrofit existing tests to use the get/set functions.

[smfreegard]

I agree with all of this except for creating it as a separate repository. This code should be in core IMO.

[smfreegard]

And I think it goes without saying that connection.notes.foo = 'bar'; etc. must continue to work as they do now. We just make it preferable to do it via get/set and then for v3 we mandate get/set only.

[msimerson]

If it's only in core, then all the advantages of having smarter notes aren't available in the test suite without duplicating the set/get code into haraka-test-fixtures. And then keeping it in sync in the future when we extend notes further. That's less than idea.

I'm envisioning the implementation of notes.js as being not greatly more complicated than:

const Notes = {};
module.exports = Notes;

Notes.prototype.get(path) {
    // code
}

Notes.prototype.set(path, value) {
    // code
}

There's no reason it wouldn't continue to work just fine with legacy usage.

[smfreegard]

Ok - yeah, I forgot that haraka-test-fixtures was separate now to handle testing of the plugins as modules.