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-5542: Queued DataSource #566

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
8a8e11c
FLUID-5542: Added requestQueue
jobara Oct 30, 2014
fe6a81e
FLUID-5542: Added more comments to requestQueues
jobara Oct 30, 2014
a29872f
FLUID-5542: Added the queuedDataSource grade
jobara Oct 30, 2014
983ff2f
FLUID-5542: Update all tests, renamed files
jobara Oct 30, 2014
09b61af
FLUID-5542: Corrected typos
jobara Oct 31, 2014
1bdbf25
FLUID-5542: Renaming
jobara Oct 31, 2014
fed923a
FLUID-5542: Corrected signature for the DataSource
jobara Oct 31, 2014
53d355b
FLUID-5542: Created a new grade for fifo queue
jobara Oct 31, 2014
0550ea1
FLUID-5542: Removed throttle request queue
jobara Oct 31, 2014
0613710
FLUID-5542: Removed the asyncloop from the tests
jobara Oct 31, 2014
a124dce
FLUID-5442: Re-implemented based on review.
jobara Nov 5, 2014
6b19c80
FLUID-5542: Fixed typo
jobara Nov 6, 2014
edacdcf
FLUID-5542: Updated comments
jobara Nov 6, 2014
80c1e6e
FLUID-5542: Interim commit before adding promises
jobara Nov 7, 2014
b1c3544
Merge remote-tracking branch 'antranig/FLUID-5513' into FLUID-5542
jobara Nov 7, 2014
27532db
FLUID-5542: refactored to use promises
jobara Nov 10, 2014
f07ec54
FLUID-5542: Linting
jobara Nov 10, 2014
ac6d291
FLUID-5542: Changed base grade
jobara Nov 10, 2014
bb4fab5
FLUID-5542: Fixed returning promises + unit tests
jobara Nov 14, 2014
05a5a22
Merge branch 'master' into FLUID-5542
jobara Nov 18, 2014
f8865db
FLUID-5542: Corrected typos
jobara Nov 19, 2014
942976a
FLUID-5542: removed quotes around some keys
jobara Nov 19, 2014
c62458d
FLUID-5542: configured extra delay buffer
jobara Nov 19, 2014
a4440df
FLUID-5542: Added extra examples to test
jobara Nov 19, 2014
5020fb2
Merge branch 'master' into FLUID-5542
jobara Nov 20, 2014
9a857bf
Merge branch 'master' into FLUID-5542
jobara Dec 12, 2014
454ea1c
FLUID-5542: expanded test namespace
jobara Dec 12, 2014
c5ae8ec
Merge branch 'master' into FLUID-5542
jobara Jun 16, 2015
fc2a1d6
FLUID-5542: Reducing duplicate code in tests
jobara Jun 16, 2015
21803c5
FLUID-5542: Split base grade from implantation
jobara Jun 22, 2015
b07f492
FLUID-5542: refactored to simplify integration
jobara Jun 22, 2015
88c0e26
Merge branch 'master' into FLUID-5542
jobara Jun 29, 2015
b395ca8
FLUID-5542: Testing consistency of fluid.toHashKey
jobara Jun 29, 2015
0e5fc60
FLUID-5542: removed commented out code.
jobara Jun 29, 2015
550f145
FLUID-5542: Call that.addToQueue in enqueueImpl
jobara Jun 29, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
247 changes: 239 additions & 8 deletions src/framework/core/js/FluidRequests.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@ var fluid_2_0 = fluid_2_0 || {};

(function ($, fluid) {
"use strict";

/** NOTE: All contents of this file are DEPRECATED and no entry point should be considered a supported API **/

fluid.explodeLocalisedName = function (fileName, locale, defaultLocale) {
var lastDot = fileName.lastIndexOf(".");
if (lastDot === -1 || lastDot === 0) {
lastDot = fileName.length;
}
var baseName = fileName.substring(0, lastDot);
var extension = fileName.substring(lastDot);

var segs = locale.split("_");

var exploded = fluid.transform(segs, function (seg, index) {
var shortSegs = segs.slice(0, index + 1);
return baseName + "_" + shortSegs.join("_") + extension;
Expand Down Expand Up @@ -77,7 +77,7 @@ var fluid_2_0 = fluid_2_0 || {};
that.operate();
return that;
};

fluid.fetchResources.explodeForLocales = function (resourceSpecs) {
fluid.each(resourceSpecs, function (resourceSpec, key) {
if (resourceSpec.locale) {
Expand All @@ -95,7 +95,7 @@ var fluid_2_0 = fluid_2_0 || {};
});
return resourceSpecs;
};

fluid.fetchResources.condenseOneResource = function (resourceSpecs, resourceSpec, key, localeCount) {
var localeSpecs = [resourceSpec];
for (var i = 0; i < localeCount; ++ i) {
Expand All @@ -110,15 +110,15 @@ var fluid_2_0 = fluid_2_0 || {};
resourceSpecs[key] = lastNonError;
}
};

fluid.fetchResources.condenseForLocales = function (resourceSpecs) {
fluid.each(resourceSpecs, function (resourceSpec, key) {
if (typeof(resourceSpec.localeExploded) === "number") {
fluid.fetchResources.condenseOneResource(resourceSpecs, resourceSpec, key, resourceSpec.localeExploded);
}
});
};

fluid.fetchResources.notifyResources = function (that, resourceSpecs, callback) {
fluid.fetchResources.condenseForLocales(resourceSpecs);
callback(resourceSpecs);
Expand Down Expand Up @@ -431,5 +431,236 @@ var fluid_2_0 = fluid_2_0 || {};
return fluid.NO_VALUE;
};

/** Start dataSource **/

/*
* A grade defining a DataSource.
* This grade illustrates the expected structure a dataSource, as well as
* providing a means for identifying dataSources in a component tree by type.
*
* The purpose of the "dataSource" abstraction is to express indexed access
* to state. A REST/CRUD implementation is an example of a DataSource; however,
* it is not limited to this method of interaction.
*/
fluid.defaults("fluid.dataSource", {
gradeNames: ["fluid.eventedComponent", "autoInit"]
// Invokers should be defined for the typical CRUD functions and return
// a promise.
//
// The "get" and "delete" methods require the signature (directModel).
// The "set" method requires the signature (directModel, model)
//
// directModel: An object expressing an "index" into some set of
// state which can be read or written.
//
// model: The payload sent to the storage.
//
// options: An object expressing implementation specific details
// regarding the handling of a request. Note: this does not
// include details for identifying the resource. Those should be
// placed in the directModel.
//
// invokers: {
// "get": {}, // handles the Read function
// "set": {}, // handles the Create and Update functions
// "delete": {} // handles the Delete function
// }
});

/*
* Converts an object or array to string for use as a key.
* The objects are sorted alphabetically to insure that they
* result in the same string across executions.
*/
fluid.objectToHashKey = function (obj) {
var str = fluid.isArrayable(obj) ? "array " : "object ";
var keys = fluid.keys(obj).sort();

fluid.each(keys, function (key) {
var val = obj[key];
str += key + ":" + fluid.toHashKey(val);
});

return str;
};

/*
* Generates a string for use as a key.
* They typeof of the value passed in will be prepended
* to ensure that (strings vs numbers) and (arrays vs objects)
* are distinguishable.
*/
fluid.toHashKey = function (val) {
var str;
if(fluid.isPlainObject(val)){
str = "<" + fluid.objectToHashKey(val) + ">";
} else {
str = "|" + JSON.stringify(val) + "|";
}
return str;
};

/*
* A dataSource wrapper providing a queuing mechanism for requests.
* Requests are queued based on type (read/write) and resource (directModel).
*
* A fully implemented dataSource, following the structure outlined by fluid.dataSource,
* must be provided in the wrappedDataSource subcomponent. The get, set, and delete methods
* found on the queuedDataSource will call their counterparts in the wrappedDataSource, after
* filtering through the appropriate queue.
*
* TODO: A fully realized implementation should provide a mechanism for working with a local
* cache. For example pending write requests could be used to service get requests directly.
*/
fluid.defaults("fluid.queuedDataSource", {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming is still confusing here. The comment above suggests that this is some kind of generic component that permits different queue policies - but in fact this grade is already bound to an implementation that operates a particular hardcoded policy that enforces a queue size of 1 (via "fluid.queuedDataSource.enqueueImpl"). This top-level grade should be better factored into a generic (possibly abstract) grade that doesn't commit the user to anything, with functions like enqueueImpl more clearly named (bound in a different grade/namespace) to emphasize that they are part of a particular concrete realisation of the queuedDataSource.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comment

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

created a new grade called debouncedDatasource that attaches the enqueue methods.

gradeNames: ["fluid.eventedComponent", "autoInit"],
members: {
requests: {
read: {},
write: {}
}
},
components: {
wrappedDataSource: {
// requires a dataSource that implements the standard set, get, and delete methods.
type: "fluid.dataSource"
}
},
events: {
enqueued: null,
afterRequestComplete: null
},
listeners: {
"enqueued.start": {
listener: "{that}.start",
args: ["{arguments}.1"]
},
"afterRequestComplete.start": {
listener: "{that}.start",
args: ["{arguments}.1"]
}
},
invokers: {
// The add to queue method needs to be provided by the integrator
// addToQueue: "",
// addToQueue: {funcName: "fluid.identity"},
set: {
funcName: "fluid.queuedDataSource.enqueueWithModel",
args: ["{that}", "{that}.requests.write", "{wrappedDataSource}.set", "{arguments}.0", "{arguments}.1", "{arguments}.2"]
},
get: {
funcName: "fluid.queuedDataSource.enqueue",
args: ["{that}", "{that}.requests.read", "{wrappedDataSource}.get", "{arguments}.0", "{arguments}.1"]
},
"delete": {
funcName: "fluid.queuedDataSource.enqueue",
args: ["{that}", "{that}.requests.write", "{wrappedDataSource}.delete", "{arguments}.0", "{arguments}.1"]
},
start: {
funcName: "fluid.queuedDataSource.start",
args: ["{that}", "{arguments}.0"]
}
}
});

fluid.queuedDataSource.start = function (that, queue) {
if (!queue.isActive && queue.requests.length) {
var request = queue.requests.shift();

var requestComplete = function () {
queue.isActive = false;
that.events.afterRequestComplete.fire(request, queue);
};

queue.isActive = true;
var args = "model" in request ? [request.directModel, request.model, request.options] : [request.directModel, request.options];
var response = request.method.apply(null, args);

response.then(requestComplete, requestComplete);
fluid.promise.follow(response, request.promise);
}
};

/*
* Adds only one item to the queue at a time, new requests replace older ones
*
* The request object contains the request function and arguments.
* In the form {method: requestFn, directModel: {}, model: {}, callback: callbackFn}
*/
fluid.queuedDataSource.enqueueImpl = function (that, addToQueueFn, requestsQueue, request) {
var promise = fluid.promise();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From this line (500) to 516, the behaviour is generic - as is the behaviour after 519. That is, the implementation is broadly what could be expected to be generic amongst all queue variants. Although we are only providing one queue variant in this current implementation, we should make sure that the component easily allows extension. It seems like the only line which represents a concrete implementation choice is line 518 - so this line should be factored out into a dedicated function (probably an invoker) attached to the component which is easily configurable and suitably named.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comment - we need to ensure that the two "enqueue" functions do not have a hard binding to enqueueImpl.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added an addToQueue invoker

var key = fluid.toHashKey(request.directModel);
var queue = requestsQueue[key];

// add promise to the request object
// to be resolved in the start method
// when the wrapped dataSource's request returns.
request.promise = promise;

// create a queue if one doesn't already exist
if (!queue) {
queue = {
isActive: false,
requests: []
};
requestsQueue[key] = queue;
}

// queue.requests[0] = request;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray commented line here


if (typeof(addToQueueFn) === "string") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addToQueueFn could never be a string, since it is sourced from an invoker target - to be clearer, you can just invoke that.addToQueue(queue, request)

fluid.invokeGlobalFunction(addToQueueFn, [queue, request]);
} else {
addToQueueFn(queue, request);
}

that.events.enqueued.fire(request, queue);

return promise;
};

fluid.queuedDataSource.enqueue = function (that, requestsQueue, method, directModel, options) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is mostly identical to enqueueWithModel - and this duplication also creates a hazard for people wanting to customise the queue policy since they will need to provide implementations of both of these. We should i) consolidate these into 1 function as well as ii) provide a clear means for the user to just be able to replace enqueueImpl without necessarily having to replace enqueue/enqueueWithModel.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for fluid.queuedDataSource.enqueue and fluid.queuedDataSource.enqueueWithModel being separate functions is that in fluid.queuedDataSource.start we need to check for the presence of model in the request object to determine the correct set of arguments to use.

https://github.com/jobara/infusion/blob/FLUID-5542/src/framework/core/js/FluidRequests.js#L485

var request = {
method: method,
directModel: directModel,
options: options
};

return fluid.queuedDataSource.enqueueImpl(that, that.addToQueue, requestsQueue, request);
};

fluid.queuedDataSource.enqueueWithModel = function (that, requestsQueue, method, directModel, model, options) {
var request = {
method: method,
directModel: directModel,
model: model,
options: options
};

return fluid.queuedDataSource.enqueueImpl(that, that.addToQueue, requestsQueue, request);
};

fluid.queuedDataSource.replaceRequest = function (queue, request) {
queue.requests[0] = request;
};

/*
* A dataSource wrapper providing a debounce queuing mechanism for requests.
* Requests are queued based on type (read/write) and resource (directModel).
* Only 1 requested is queued at a time. New requests replace older ones.
*
* A fully implemented dataSource, following the structure outlined by fluid.dataSource,
* must be provided in the wrappedDataSource subcomponent. The get, set, and delete methods
* found on the queuedDataSource will call their counterparts in the wrappedDataSource, after
* filtering through the appropriate queue.
*/
fluid.defaults("fluid.debouncedDataSource", {
gradeNames: ["fluid.queuedDataSource", "autoInit"],
invokers: {
addToQueue: "fluid.queuedDataSource.replaceRequest"
}
});

/** End dataSource **/

})(jQuery, fluid_2_0);
1 change: 1 addition & 0 deletions tests/all-tests.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"./framework-tests/core/html/FluidIoC-test.html",
"./framework-tests/core/html/FluidIoCStandalone-test.html",
"./framework-tests/core/html/FluidIoCView-test.html",
"./framework-tests/core/html/DataSource-test.html",
"./framework-tests/enhancement/html/ProgressiveEnhancement-test.html",
"./framework-tests/renderer/html/RendererUtilities-test.html",
"./framework-tests/preferences/html/AuxBuilder-test.html",
Expand Down
33 changes: 33 additions & 0 deletions tests/framework-tests/core/html/DataSource-test.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>DataSource Tests</title>

<!-- This is the jqUnit test css file -->
<link rel="stylesheet" media="screen" href="../../../lib/qunit/css/qunit.css" />

<script type="text/javascript" src="../../../../src/lib/jquery/core/js/jquery.js"></script>
<script type="text/javascript" src="../../../../src/framework/core/js/Fluid.js"></script>
<script type="text/javascript" src="../../../../src/framework/core/js/FluidPromises.js"></script>
<script type="text/javascript" src="../../../../src/framework/core/js/FluidIoC.js"></script>
<script type="text/javascript" src="../../../../src/framework/core/js/DataBinding.js"></script>
<script type="text/javascript" src="../../../../src/framework/core/js/FluidRequests.js"></script>

<!-- These are the jqUnit test js files -->
<script type="text/javascript" src="../../../lib/qunit/js/qunit.js"></script>
<script type="text/javascript" src="../../../test-core/jqUnit/js/jqUnit.js"></script>

<!-- These are tests that have been written using this page as data -->
<script type="text/javascript" src="../js/DataSourceTests.js"></script>

</head>
<body>
<h1 id="qunit-header">DataSource Test Suite</h1>
<h2 id="qunit-banner"></h2>
<div id="qunit-testrunner-toolbar"></div>
<h2 id="qunit-userAgent"></h2>
<ol id="qunit-tests"></ol>

</body>
</html>
Loading