Consider separating processMessage() into individual handling methods per DWN Interface Method

[thehenrytsai]

Background:

With the introduction and implementation of RecordsRead (and other Interface Methods to follow like RecordsCommit), it is becoming apparent that the return type cannot be represented by a universal type that covers all need, unless we hack our way by adding a bunch of optional properties or even just use any.

Task Details:

• Maybe the time has come for us to consider evolve processMessage() which has served well, and replace/augment it with individual handlers:

handleRecordsRead(...): RecordsReadResponse { ... } ✅
handleRecordsWrite(...): RecordsWriteResponse { ... }
...

Objective:

• create a handler for handleRecordsWrite(...): RecordsWriteResponse { ... }

handleRecordsRead(...): RecordsReadResponse { ... } was introduced in this pull request if you're interested in an example.

Picking Up This Issue:

• If you'd like to work on this, please comment "picking this up" below, and I'll assign the issue to you

Questions:

• If you need assistance or clarification, feel free to comment below.
• Feeling overwhelmed? Join our #hack-together channel and collaborate with a buddy.
• New to open-source? Check out our Hacktober #getting-started channel.
• Leaderboard: Check your ranking after finishing this task.

Resources:

• Creating a Pull Request: If you're new to GitHub and unsure how to create a pull request, follow this step-by-step guide.

Remember, communication is key! If you have any questions or face any challenges, we're here to help so please don't hesitate to reach out.

Good Luck! 🍁

[shobitb]

@thehenrytsai / @EbonyLouis — can I pick this up?

If yes, a few questions before I get started —

1. Do you think the response type for handleRecordsWrite can continue to be GenericMessageReply (like in the Handler)? Or introduce a new RecordsWriteReply (but that would essentially be a GenericMessageReply (hence, an alias))?
2. Is it okay to introduce handleRecordsDelete (and any subsequent refactor) in separate commits? I feel tiny commits are easier to review/merge.

Looking forward to hearing back and contributing. Thanks!

[EbonyLouis]

@shobitb yay, I assigned it to you!
I also reached out to Henry he'll be the best person to answer your question.

[thehenrytsai]

Hi @shobitb, thanks for considering to contribute!! Answering your questions:

Do you think the response type for handleRecordsWrite can continue to be GenericMessageReply

No, the response type should be as specific to the type of message as possible, which is really the intent and benefit of having dedicated handler methods. There is a couple of precedence (RecordsRead and RecordsQuery) that you can follow.

Is it okay to introduce handleRecordsDelete (and any subsequent refactor) in separate commits?

Totally. Agree with you that small commits are better for many reasons!

Also, when making sure you have test coverage on these new dedicated handlers, you have one of the two approaches: either add new tests in dwn.spec.ts, or modify one (or a couple) existing tests to call these dedicated handlers instead. Obviously adding tests to dwn.spec.ts is more intentional, but I am not too opinionated if you prefer the latter/lazier approach.

[shobitb]

Thanks for assigning and helping, @EbonyLouis and @thehenrytsai. I've created a PR. Would love your feedback. If this looks good, I'll use the same format to introduce handleRecordsDelete in a subsequent PR.

[diehuxx]

#554 addresses this by overloading the function signature of processMessage, so we no longer need separate methods for better typing