Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FLUID-6209: Documentation for Remote Model and DataSource components #142

Open
wants to merge 13 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@jobara
Copy link
Member

commented Sep 13, 2018

@jobara

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2018

@amb26 would you be able to review this PR?


## Grades and Linkage

There are two main grades `fluid.dataSource` and `fluid.dataSource.writable`. `fluid.dataSource` contains the base configuration and includes configuration for getting (reading) in from a data source. `fluid.dataSource.writable` adds in the configuration for setting (writing) to a data source. If you need to read and write to a data source, you'll need to add concrete implementations for both of these grades and make use of [grade linkage](IoCAPI.md#fluidmakegradelinkagelinkagename-inputnames-outputnames) to apply the concrete writable grade to the datasource configuration. In this way one would only need to add the `fluid.dataSource.writable` grade to gain the configuration from the concrete writable grade.

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

Should clarify "If you need to read and write to a data source " .... to make clear that these are instructions for people who need to implement a new kind of DataSource. Most users should be able to get what they need by supplying configuration to the existing ones. In general the docs should make a clearer distinction between the needs of users configuring existing sources and those writing new ones and/or the concrete implementation for one.


## Encoding

During the `get` (read) and `set` (write) workflows, listeners are provided as handles for encoding and transforming the payload. For example serializing and deserializing the payload on it's way to and from a server. The encoding is implemented by a supplied encoding component which includes `parse` and `render` methods. For `get` the data is run through `parse`. For `set`, the data is run through `render` when being sent to the server, and any returned values are sent through `parse`.

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

Link at this point to the docs for fluid.promise.fireTransformEvent since it will not be clear in what sense these things are really "listeners".
Typo it's -> its


During the `get` (read) and `set` (write) workflows, listeners are provided as handles for encoding and transforming the payload. For example serializing and deserializing the payload on it's way to and from a server. The encoding is implemented by a supplied encoding component which includes `parse` and `render` methods. For `get` the data is run through `parse`. For `set`, the data is run through `render` when being sent to the server, and any returned values are sent through `parse`.

Two encoding classes are provided for use, `fluid.dataSource.encoding.JSON` (serializes/deserializes JSON data) and `fluid.dataSource.encoding.none` (provides no encoding/decoding transformation). If these do not meet the requirements necessary for the concrete dataSource, a different encoder subcomponent may be provided to the concrete dataSource grade.

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

"If these do not meet the requirements necessary" is redundant. Also the requirements are not those of the concrete dataSource but those of the user.


Deserializes a JSON string into a proper JavaScript object and returns a promise for the result. If there is an error, the promise will be rejected and contain the error message. Otherwise the promise will be resolved with the JavaScript object or `undefined` if the `string` value is falsey.

* `string {String}` The JSON string to be deserialized

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

I think we have a "* Returns:" syntax for expressing return types and meaning?

`fluid.remoteModelComponent` builds on top of [`fluid.modelComponent`](ComponentGrades.md) with the purpose of providing a buffer between a local and
remote model that are attempting to stay in sync. For example a local model is being updated by user interaction, this is sent back
to a remote server, which in turn tries to update the local model. If additional user actions occur during the roundtrip, an
infinite loop of updates may occur. `fluid.remoteModelComponent` solves this by restricting reading and writing to a single request at a time,

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

Should say it actually does a little more than this, in that it will make some attempt to "rebase" local changes that have occurred with respect to remote ones.

waiting for one request to complete before operating the next.

## Supported Events

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

Typo Compnent


## Supported Events

In addition to the standard `fluid.modelCompnent` events, `fluid.remoteModelComponent` add the following:

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

typo add -> adds

* `onWriteError`,
* `afterWrite`

For information on the different types of events, see [Infusion Event System](InfusionEventSystem.md).

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

We don't really have different "types" of events any more - all the non-core varieties are deprecated. Should instead say something like, for information on how events work and how to configure listeners to them, see xxxxx

For information on the different types of events, see [Infusion Event System](InfusionEventSystem.md).

<div class="infusion-docs-note">
<strong>Note:</strong> The <code>onFetch</code>, <code>afterFetch</code>, <code>onWrite</code> and <code>afterWrite</code> events are synthetic events that fire as a sequence. As implemented in the <code>fluid.remoteModelComponent</code>, the listeners for the event will all fire in the specified order and be passed the same arguments. However, the next listener will not trigger till the previous one has completed. That is, either returned a value or resolved/rejected a returned promise. This is useful to prevent further actions from happening until all of the listeners have completed, but unlike other event sequences, it cannot be used to pass the payload through a series of transformations.

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

Again link to fluid.promise.fireTransformEvent

<tr>
<th>Description</th>
<td>
Fires an event sequence the executes before <code>fetchImpl</code> is called.

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

typo the -> that

</td>
</tr>
<tr>
<th>Type</th>

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

Need information on the argument payload

<tr>
<th>Parameters</th>
<td>
Any paramaters passed along to the rejected promise. Typically this is an <code>error</code> object.

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

Typo parameters. We need to give more guidance here on the expectations of the reject payload. Is it really typically an error object? Probably meant capital E Error. One convention we use a lot is for there to be an isError: true field if it is not an Error -

This comment has been minimized.

Copy link
@jobara

jobara Nov 30, 2018

Author Member

@amb26 it is whatever gets returned for the rejected promise reason. So in that case, I don't really know exactly what it will be because it is dependent on what happens in the onFetch promise sequence and the onFetchImpl. Any thoughts on how to better document this?

<tr>
<th>Description</th>
<td>
Fires an event sequence the executes before <code>writeImpl</code> is called.

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

the -> that


## Write Workflow

Executing the `write` invoker adds a write request and returns a promise for the result. Only one request can be in flight (processing) at a time. If a fetch or write request is in flight, the write will be queued. If a write request is already in queue, the result of that request will be passed along to the current write request. When a write request is in flight, it will trigger the `writeImpl` invoker to perform the actual request.

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

"the result of that request will be passed along to the current write request" is this really right? I assume you mean that the requests will be coalesced into a single write request sent to the server, and all representatives in the queue will receive the same response payload.

This comment has been minimized.

Copy link
@jobara

jobara Nov 30, 2018

Author Member

Yes, I believe that is what I meant. I'll just make use of your text for the explanation.


## Fetch Workflow

Executing the `fetch` invoker will add a fetch request and returns a promise for the result. Only one request can be in flight (processing) at a time. If a write request is in flight, the fetch will be queued. If a fetch request is already in queue/flight, the result of that request will be passed along to the current fetch request. When a fetch request is in flight , it will trigger the `fetchImpl` invoker to perform the actual request.

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

See comment below in the similar wording for "the result of that request" for WRite


Two synthetic events, `onFetch` and `afterFetch`, are fired during the processing of a fetch. `onFetch` can be used to perform any necessary actions before running `fetchImpl`. `afterFetch` can be used to perform any necessary actions after running `fetchImpl` (e.g. updating the model, unblocking the queue). If promises returned from `onFetch`, `afterFetch`, or `fetchImpl` are rejected, the `onFetchError` event will be fired.

![A flow diagram depicting the Fetch workflow](images/remoteModel_fetch_diagram.svg)

This comment has been minimized.

Copy link
@amb26

amb26 Nov 28, 2018

Member

Should try to say something in words explaining the "rebasing" workflow in fluid.remoteModelComponent.updateModelFromFetch

@jobara

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2018

@amb26 I believe I've addressed all of your comments. Ready for more review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.