TheProlog / prolog-use_cases

Use-case layer for Meldd/Prolog application.
0 stars 0 forks source link

Create and persist a new `User` entity in response to a registration request. #5

Closed jdickey closed 8 years ago

jdickey commented 8 years ago

As presented in Feature Strategy and Planning (Issue #1), the second use case, from the user perspective, is "Sign up as a new member". Following the pattern established by Prolog::UseCases::SummariseContent, this new use-case implementation, tentatively named Prolog::UseCases::RegisterNewMember, will implement the domain logic required to accomplish that task.

Continuing with our spin on hexagonal architecture, this will involve Wisper conversations with external subsystems ("ports", in hexagonal terminology) that accomplish the following tasks in response to a call to RegisterNewMember#call, supplying the proposed new-member details as parameters:

  1. Verify that no member is presently logged in, using an "authentication listener" as developed for SummariseContent;
  2. Verify that the requested member name is available, using a "persistence listener" for Prolog::Core::User entities similar to the Article persistence listener developed for SummariseContent;
  3. After validating the proposed member details as appropriate, persisting them as a new User entity;
  4. A user-interface (UI) listener that, as with the listener defined for SummariseContent provides a facility to report errors, provides an additional capability to report successful registration.

If this sounds more complicated than SummariseContent, that's because, at a feature level it is. We expect, however, to be able to proceed relatively more quickly as many of the basic process questions involved in spinning up a use-case class and then updating the Gem have now been answered.

jdickey commented 8 years ago

To restate what likely "ought" to be obvious from the four-step description in the opening comment:

There are four conversations with either three or perhaps four "port"-level Wisper listeners that this service engages in; other than those, it is a black box with no caller-accessible attributes. These ports are as follows:

  1. An authentication port determines the current user name and communicate it back to the service. How the remote end of the conversation actually accomplishes that determination is beyond the scope of the service.
  2. A persistence port for User entities is used to query whether the requested user name already exists in the system;
  3. A persistence port for User entities (perhaps the same as that in Item 2) is used to persist the newly-validated and -created User entity based on the parameters specified to #call; and, finally
  4. A UI port is notified of successful completion or of one or more errors which caused the service to fail. The request parameters are not echoed as part of the error notifications; the remote UI service is presumed to already have the relevant information or be able to reacquire it.

How these "listeners" or remote ends of the various message conversations accomplish their tasks is beyond the scope of this class. All we're concerned with here is the domain logic of how a new-user registration request is processed and the interactions it participates in with outside services. Otherwise, we'll never finish.

jdickey commented 8 years ago

One final comment for the night: we're mulling over the idea of evolving our use-case development process, currently in its second use, to require some form of documentation for each use case. This would describe how it was used; what interactions (Wisper broadcast-message conversations and others as appropriate) are engaged in; and what data is made accessible to code outside the service object after completion. YAGNI and "Use the tests, Luke" are arguing against that in its entirety; concern over ability to recall or rediscover required information efficiently when use case objects are tied into an application argues for some form of it; and we're standing in the middle wondering if the risk truly justifies the time spent, or even time spent thinking through such a process.

mitpaladin commented 8 years ago

should it be documentation at a Use Case level then or at a service object level? implementation of a later use case would in some cases overwrite or expand upon the data/messages exposed previously right?

jdickey commented 8 years ago

use case == service in this discussion; a use case is a specific type/context of service.

I think I’ll solve, or at least punt, the problem by having the complete golden path be the first thing in the test spec for the use-case class, instead of buried idiosyncratically farther down. I've also been reading a good book by Alistair Cockburn on use cases, and it makes a few useful points (buried in among the 2000-era last gasps of the UML-oriented and CASE-oriented sideshows).

jdickey commented 8 years ago

Commit 22fbca6 reworked the test specs for SummariseContent in what is intended, anyway, to be a more descriptive, documentary style:

  1. setup of data and fixtures to be used across individual tests;
  2. documentation of the interface of the class;
  3. documenting details of the return value from #call after correct setup;
  4. documentation of the result of calling #call without having done the necessary setup.

Or, in more general terms:

  1. Data/fixture setup;
  2. Public API of use-case class;
  3. "Golden path" results when required external setup correctly done and valid parameters, if any, passed in;
  4. Documentation of results and effects of any possible (read: known and supported) failure mode.

These failure modes might result from improper external setup (e.g., not setting up a correct Wisper listener) or detection of an internal error within the API-method black box. What happens in SummariseContent#call or, rather its persistence interface, fails to retrieve article data because the database server crashes? We don't have anything now that explicitly speaks to that. Is that something we should worry about right now? In the rush to ship 0.5, arguably not; more rigorous functional tests for the delivery-system implementation may well suffice.

jdickey commented 8 years ago

And now we remind ourselves of some of the towering warts on the newest way of doing things.

Repeat after me:

Use cases (and the form objects they depend on) are stateful, even though the value objects they take as input and output aren't.

Forgetting that bit of hard-earned wisdom is an excellent way to fritter away half of what passes for a "working day" here. A five-minute review of an earlier demo project, model_form_demo, turns up this form object which is used by this controller action responder class. (Controller action responders were our home-baked version of what the industry calls form objects.)

Note how the form object has mutable internal state, since it includes Virtus.model rather than Virtus.value_object, as well as including ActiveModel::Validations, (which adds an errors instance variable to the class, among other things). We can (and may well, use different validation classes/logic, but the template still holds..

In the responder/use case, an instance of the form object (which wraps an entity-like object, remember) is created and then validated using the form object's defined validations. Our use-case usage will be very similar.

One difference between our new use-case usage of form objects and what we'd done previously is that form objects are expected to be unique to each use case. When an application only used a handful, adding their classes to a top-level namespace such as FormObjects was a reasonable organisation strategy; given that the new prolog-use_cases Gem essentially packages each use case, and thus each client of a particular form object, separately, that style no longer seems appropriate. Rather than, say, FormObjects::RegisterNewMember, this use case's form-object class would be Prolog::UseCases::RegisterNewMember::FormObject. In the presently-viewed-as-unlikely event that a single use case would employ multiple form objects, names can be selected to suit.

Onward!

jdickey commented 8 years ago

Using form objects (rather than a "raw" Virtus value object) would also move the Wisper conversations from the use case to the form object, further apparently simplifying the implementation code (if not the tests, which need to talk to Wisper as before). Yes, we're just reshuffling deck chairs, but we're not asking the entire football side to sit in one chair.

And, in theory, given sufficient quantities of that mythical resource known as time, we could DRY that test setup up nicely. In theory, there's no difference here between theory and practice; in practice, a calendar is a thing.

jdickey commented 8 years ago

Well, dry-validation had been on our "check this out when you find yourself reaching for the equivalent" list and, after an hour and a half of poking, the "equivalent" that we'd previously used for form objects, ActiveModel::Validations, is a lot less work (even at the cost of dragging tonnes more code into the Gem dependency list). :disappointed:

jdickey commented 8 years ago

Right. So we want to add a form object to our use-case class, to handle the data validation and mastication. The problem is that, to do that presently, Wisper listeners are being attached to the use case object, to provide services such as current user name, repository search, and UI notifications. The Wisper conversations, in the current use case, are associated with the validation (and eventually with saving the created User entity). Objects enclosed within the use case must not be Publishers in their own right, but must be delegated the #broadcast (alias #publish) method from the containing class (or its containing class, etc.) We are clearly and drastically exceeding Wisper's own target usage scenario. This is rapidly becoming unwieldy to and beyond the point of farce.

We have three alternatives presently visible:

  1. We forget about breaking out commonly-used Publishers such as anything that queries remote services (asking for the current user, getting user details from persistent storage, etc), include single-level code within use cases (delegating only to form objects, which cannot be avoided), and tell static-analysis tools like RuboCop and Reek to STFU and ignore everything they don't like, at the risk of introducing speed bugs.
  2. We resurrect at least aspects of a design pattern we'd previously abandoned in Prolog::Core, passing objects into use-case initialisers that respond to messages corresponding to our existing/envisioned Wisper usage, with these in turn being passed into any child classes (such as the previously-mentioned current-user checker). There's a voice in the back of our heads screaming No, don't do that, but the long explanation of why is mumbled to the point of unintelligibility. Anybody who sees any problems with this, now is the time; we're not talking about core entities, after all.
  3. We follow the instinct we've had for a couple of weeks now, ripping out Wisper and replacing it with a more "traditional" message queue, where listeners simply look for messages that they "know" how to deal with and ignore everything else. This is the preferred solution in a post-0.5 world.

Thoughts?

mitpaladin commented 8 years ago

both 1 and 2 are inelegant at best, door #3

jdickey commented 8 years ago

Per our discussion on Slack last night, proceeding with Option 2. It's going to involve more total work doing that and then ripping it out in favour of an Option-3-like solution after 0.5 ships, but I believe it adds less risk for additional delay to 0.5 itself. We're already in dangerous waters here.