Open Stack

TL;DR: we need to fix workflows :-)

On 15 July 2016 at 16:32, Tripp, Travis S <travis.tripp at hpe.com> wrote:

> If we find that there is still a need to send the scope in that can’t be
> reasonably worked around, then the action services could be changed to be a
> factory.


I think this is a bad idea because in the angular world "factory" means
singleton (which is totally opposite to what everyone else in the
programming world [and the rest] thinks "factory" means but hey, angular
gonna angular) and we'd be changing that, and the potential for confusion
would be very high.


The injected action factory should have a factory function called by the
> controller which creates an instance of the action in each controller.


Controllers are already not singletons - I see no reason why mutable state
shouldn't be contained in *them*. We don't need to invent something new to
hold that mutable state.



> Then the init scope could be called on the instance of that action. This
> also would then allow actions to have state that is retained for the
> lifetime of the controller, while not worrying about sharing action state
> with other controllers.
>

I don't believe retaining "init scope" is necessary at all.

As far as I can tell we're already creating an independent context for the
actual meat of the action, we're just not keeping it independent enough. To
demonstrate, I'm going to pull apart one example: images create-volume.

The files for create volume consist of (under
openstack_dashboard/static/app/core/images):

actions/create-volume.service.js
  Is the action fired off when a user hits a "create volume" button.
workflows/create-volume.service.js
  Is the workflow service used in the modal dialog that the above action
service creates.
steps/create-volume/create-volume.html
  Is the single HTML step in the above workflow service.
steps/create-volume/create-volume.controller.js
  Is the controller used in the above HTML step.

Note that the workflow only has one step. I think that's done so plugins
can extend create-volume with additional steps. Regardless, it's the
workflow that's the root of our problems here. Read on... ;-)

The flow through the above code is basically:

1. allow an interface to invoke an action perform() to create a volume
2. open a modal with a form in it to capture the volume details
3. have the service from step 1 invoke an API with the data captured in the
form
4. resolve the action service perform() promise with an ActionResult

The scope usage is needed because the data transfer between the step
controller and the action service is done through an event, VOLUME_CHANGED,
and event passing is done through a scope. Thus the controller and action
service, which are quite independent pieces of code, must have intimate
knowledge of each other's internal operation. I'm not so keen on that ;-)

The event is currently necessary because of the workflow sitting between
the action service and controller. Without it, the controller submit()
handler could resolve the $modal promise with the captured data, which the
action service could consume. What we should be seeing is the workflow
service resolving that $modal promise with the data captured from the
workflow, and that promise resolution going back to the action service.

But the workflow implementation has no concept of an over-arching model for
the workflow. If that was changed, I believe all the current $scope
shenanigans (which are basically about short-circuiting the workflow not
doing its job ;-) would go away.

So, here's my rough thought: workflow.model is an object with properties
named for each of the workflow steps - using the step formName as the name
(hell, schema form could probably make this a doddle). The workflow model
is passed to the controller for each step, which uses its own named model
to store the data captured by the step - and as a side effect it can poke
at (and watch) the data captured by other steps, which is often useful.
Workflow $modal resolution supplies the workflow model for the consumer of
the workflow to then to something with all that data.


     Richard



> From: Richard Jones <r1chardj0n3s at gmail.com>
> Reply-To: OpenStack List <openstack-dev at lists.openstack.org>
> Date: Friday, July 15, 2016 at 2:28 PM
> To: OpenStack List <openstack-dev at lists.openstack.org>
> Subject: [openstack-dev] [Horizon] Angular action services and initScope
>
> Hi folks,
>
> Something that's been bothering me for a while is that the action services
> break the encapsulation model of Angular Services - that they are
> singletons, and consumable by multiple simultaneous consumers without those
> consumers affecting each other through their use of the Service.
>
> At the moment the initScope() functionality we've included in the action
> services breaks that model - at a minimum it is possible for multiple
> consumers to initScope() with different scopes simultaneously. This is the
> reason why I've been arguing (ok, "debating" :-) for the cessation of using
> scopes in this way.
>
> I think we need to do two things reasonably soon (before patterns become
> more ingrained):
>
> 1. Stop passing in $scope to initScope in all cases - the new
> ActionResult-enabled pattern should hopefully replace all those
> 2. Remove all initScope methods altogether. The only other use of
> initScope that I see is the pre-loading of data used during the execution
> of action allowed() methods. We should move that preloading/caching either
> into the creation of the Service object itself, or into the allowed method.
>
> If there is a use-case of initScope that I've missed (something that needs
> to be execute *after* the Service is created, not something that needs to
> tie the Service to a particular consumer of the service) then please let me
> know :-)
>
>
>      Richard
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20160717/7fb9fb83/attachment.html>