Skip to content

Commit

Permalink
FLUID-5082: Correction to "soft namespaces" algorithm, both to correc…
Browse files Browse the repository at this point in the history
…t dispatching bug as well as lengthening namespaces to include source component where registered. This requires upgrade in the base framework to insert a source component record into every listener record - can't be done with mergePolicies since they are stateless. Note that this now changes the record stored in "rawDefaults" to be a copy of the original options. This will head off some bugs in general but also will need a cleanup at some point to catch several sites where options are now being duplicated unnecessarily. After 4 years we now also have a test that the rendering version of the pager actually renders anything ...
  • Loading branch information
amb26 committed Jul 21, 2013
1 parent 8c24d6b commit d740ca7
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 45 deletions.
2 changes: 1 addition & 1 deletion src/webapp/components/pager/js/Pager.js
Expand Up @@ -402,7 +402,7 @@ var fluid_1_5 = fluid_1_5 || {};
};


fluid.defaults("fluid.pager.summary", {
fluid.defaults("fluid.pager.summary", {
gradeNames: ["fluid.viewComponent", "autoInit"],
listeners: {
onCreate: {
Expand Down
30 changes: 24 additions & 6 deletions src/webapp/framework/core/js/Fluid.js
Expand Up @@ -1117,11 +1117,13 @@ var fluid = fluid || fluid_1_5;
if (!record) {
return;
}
listener = record.listener;
listener = record.length !== undefined ? record : record.listener;
}
var id = identify(listener);
if (!id) {
fluid.fail("Cannot remove unregistered listener function ", listener, " from event " + that.name);
if (typeof(listener) === "function") {
var id = identify(listener);
if (!id) {
fluid.fail("Cannot remove unregistered listener function ", listener, " from event " + that.name);
}
}
namespace = namespace || (byId[id] && byId[id].namespace) || id;
delete byId[id];
Expand Down Expand Up @@ -1299,7 +1301,7 @@ var fluid = fluid || fluid_1_5;
};
gradeStruct.gradeHash[defaultName] = true;
// TODO: this algorithm will fail if extra grades are mutually redundant and supplied out of dependency order
// expectation is that stronger grades appear to the left in defaults - dynamic grades are stronger still
// expectation is that stronger grades appear to the left in defaults - dynamic grades are stronger still - FLUID-5085
return resolveGradesImpl(gradeStruct, (gradeNames || []).concat(fluid.makeArray(defaultGrades)));
};

Expand Down Expand Up @@ -1357,13 +1359,29 @@ var fluid = fluid || fluid_1_5;
}
return mergedDefaults.defaults;
};

// unsupported, NON-API function
// Modify supplied options record to include "componentSource" annotation required by FLUID-5082
fluid.annotateListeners = function (componentName, options) {
if (options.listeners) {
options.listeners = fluid.transform(options.listeners, function (record) {
var togo = fluid.makeArray(record);
return fluid.transform(togo, function (onerec) {
onerec.componentSource = componentName;
return onerec;
});
});
}
};

// unsupported, NON-API function
fluid.rawDefaults = function (componentName, options) {
if (options === undefined) {
return defaultsStore[componentName];
} else {
defaultsStore[componentName] = options;
var optionsCopy = fluid.copy(options);
fluid.annotateListeners(componentName, optionsCopy);
defaultsStore[componentName] = optionsCopy;
gradeTickStore[componentName] = gradeTick++;
}
};
Expand Down
10 changes: 6 additions & 4 deletions src/webapp/framework/core/js/FluidIoC.js
Expand Up @@ -1512,10 +1512,12 @@ outer: for (var i = 0; i < exist.length; ++i) {
if (!expanded.listener) {
badRec(record, " Listener record must contain a member named \"listener\", \"func\", \"funcName\" or \"method\"");
}
var softNamespace = fluid.event.resolveSoftNamespace(record.method ? record["this"] + "-" + record.method : expanded.listener);
var softNamespace = record.method ?
fluid.event.resolveSoftNamespace(record["this"]) + "." + record.method :
fluid.event.resolveSoftNamespace(expanded.listener);
if (!expanded.namespace && !namespace && softNamespace) {
expanded.softNamespace = true;
expanded.namespace = softNamespace;
expanded.namespace = (record.componentSource ? record.componentSource : that.typeName) + "." + softNamespace;
}
var listener = expanded.listener = fluid.expandOptions(expanded.listener, that);
if (!listener) {
Expand Down Expand Up @@ -1602,9 +1604,9 @@ outer: for (var i = 0; i < exist.length; ++i) {
fluid.popActivity();
return togo;
};
firer.addListener = function (listener, namespace, predicate, priority) {
firer.addListener = function (listener, namespace, predicate, priority, softNamespace) {
var dispatcher = fluid.event.dispatchListener(that, listener, eventName, eventSpec);
adder(origin).addListener(dispatcher, namespace, predicate, priority);
adder(origin).addListener(dispatcher, namespace, predicate, priority, softNamespace);
};
firer.removeListener = function (listener) {
origin.removeListener(listener);
Expand Down
Expand Up @@ -142,8 +142,11 @@ <h2 id="qunit-userAgent"></h2>
<li class="flc-pager-next">next &gt;</li>
</ul>
</div>
<!-- Presence of these two controls tests an obscure FLUID-5082 bug, do not remove -->
<span class="flc-pager-summary"></span>
<select class="flc-pager-page-size"></select>

<table summary="Table of all students and teachers." xmlns:rsf="http://ponder.org.uk" class="flc-pager-body-template">
<table summary="Table of all annimles and their breeds" xmlns:rsf="http://ponder.org.uk" class="flc-pager-body-template">
<thead>
<tr rsf:id="header:">
<th class="flc-pager-sort-header"><a rsf:id="category" href="#">Category</a></th>
Expand Down
72 changes: 52 additions & 20 deletions src/webapp/tests/component-tests/pager/js/PagedTableTests.js
Expand Up @@ -139,7 +139,7 @@ https://github.com/fluid-project/infusion/raw/master/Infusion-LICENSE.txt
/**
* Test gappedPageStrategy Strategy
*/
jqUnit.test("Pager gappedPageStrategy", function () {
jqUnit.asyncTest("Pager gappedPageStrategy", function () {
var pageSize = 3;
var pageList = 100;
var expectedPages = Math.ceil(pageList / pageSize);
Expand All @@ -149,6 +149,8 @@ https://github.com/fluid-project/infusion/raw/master/Infusion-LICENSE.txt
return {
type: "fluid.pager.renderedPageList",
options: {
dataModel: fluid.tests.pager.animalDataModel,
columnDefs: fluid.tests.pager.animalColumnDefs,
pageStrategy: fluid.pager.gappedPageStrategy(j, m)
}
};
Expand Down Expand Up @@ -180,22 +182,30 @@ https://github.com/fluid-project/infusion/raw/master/Infusion-LICENSE.txt
return false;
};

var i = 1;

var allPagesAfterClickedEachFn = function (index, element) {
if (!$(element).hasClass("flc-pager-pageLink-skip")) {
jqUnit.assertTrue("Clicked on [page " + i + "] and checking [" + $(element).find("a").attr("id") + "]", shouldExistInList(i, element));
}
};

// Go through all pages 1 by 1, and click each
for (var i = 1; i <= expectedPages; i++) {
function clickNext() {
var page = fluid.jById("page-link:link" + i);
page.click();
var allPagesAfterClicked = pager.pagerBar.pageList.locate("root").find("li");
allPagesAfterClicked.each(allPagesAfterClickedEachFn);
if (i === expectedPages - 1) {
pager.destroy();
jqUnit.start();
}
else { // Convert the test to async since on FF it is now expensive enough to generate painful "unresponsive script" warnings (mainly due to tooltip cost)
++i;
setTimeout(clickNext, 1);
}
}
pager.destroy();
clickNext();
});

fluid.tests.pager.animalDataModel = {
pets: [
{
Expand Down Expand Up @@ -248,13 +258,12 @@ https://github.com/fluid-project/infusion/raw/master/Infusion-LICENSE.txt
sortable: true
}
];


jqUnit.test("Page Table Header aria-sort, also checks if anchor titles changes accordingly ", function () {
jqUnit.test("Page Table Header aria-sort and title, and body rendering test", function () {
var strings = fluid.defaults("fluid.table").strings;

var opt = {
dataModel: fluid.tests.pager.animaldataModel,
dataModel: fluid.tests.pager.animalDataModel,
columnDefs: fluid.tests.pager.animalColumnDefs,
annotateColumnRange: "category",
model: {
Expand All @@ -263,6 +272,26 @@ https://github.com/fluid-project/infusion/raw/master/Infusion-LICENSE.txt
};
var pager = renderedPager("#rendered", opt);
var currentHeaders = pager.locate("headerSortStylisticOffset");

// Check that the table data was actually rendered into the markup
var trs = $("tbody tr", pager.container);
jqUnit.assertEquals("Correct number of rows rendered", opt.model.pageSize, trs.length);
function spanToObject(accum, span) {
var id = span.prop("id");
var member = id.substring(id.lastIndexOf(":") + 1);
accum[member] = span.text();
}

var recovered = fluid.transform(trs, function (tr) {
var togo = {};
var spans = $("span", tr);
fluid.each(spans, function (span) {
spanToObject(togo, $(span));
});
return togo;
});

jqUnit.assertDeepEq("All table data rendered", fluid.tests.pager.animalDataModel, {pets: recovered});

/**
* Get a string representation of the parameter based on the strings we have in Pager.js
Expand Down Expand Up @@ -325,13 +354,6 @@ https://github.com/fluid-project/infusion/raw/master/Infusion-LICENSE.txt
//second click is descending order
clickHeader(i);
testAriaOnAllHeaders(i, "2nd", "descending");

//third click is ascending order
//if rand(0,1)===0, then do a third click; this adds randomness
if (Math.floor(Math.random() * 2) === 0) {
clickHeader(i);
testAriaOnAllHeaders(i, "3rd", "ascending");
}
}
pager.destroy();
});
Expand All @@ -340,7 +362,7 @@ https://github.com/fluid-project/infusion/raw/master/Infusion-LICENSE.txt
/**
* Test consistentGappedPageStrategy Strategy
*/
jqUnit.test("Pager consistentGappedPageStrategy", function () {
jqUnit.asyncTest("Pager consistentGappedPageStrategy", function () {
/*
* Create n pages, check if number of pages = n
* consistentGappedPageStrategy(j, m) should look like this:
Expand Down Expand Up @@ -407,16 +429,26 @@ https://github.com/fluid-project/infusion/raw/master/Infusion-LICENSE.txt
}
};

//Go through all pages 1 by 1 , and click all page dynamically each time
for (var i = 1; i <= expectedPages; i++) {
var i = 1;

//Go through all pages 1 by 1 , and click all page dynamically each time
function clickNext() {
var page = fluid.jById("page-link:link" + i);
page.click();
jqUnit.assertEquals("Verify number of top page links", totalPages,
pager.pagerBar.locate("pageLinks").length + pager.pagerBar.locate("pageLinkSkip").length);
var allPagesAfterClicked = pager.pagerBar.pageList.locate("root").find("li");
allPagesAfterClicked.each(allPagesAfterClickedEachFn);
if (i === expectedPages - 1) {
pager.destroy();
jqUnit.start();
}
else {
++i;
setTimeout(clickNext, 1);
}
}
pager.destroy();
clickNext();
});


Expand Down
47 changes: 34 additions & 13 deletions src/webapp/tests/framework-tests/core/js/FluidIoCTests.js
Expand Up @@ -877,8 +877,11 @@ fluid.registerNamespace("fluid.tests");
fireRecord: [],
self: "{that}"
},
events: {
testEvent: null,
},
listeners: {
onCreate: [{
testEvent: [{
funcName: "fluid.tests.FLUID5082func",
args: ["{that}", 1]
}, {
Expand All @@ -904,33 +907,51 @@ fluid.registerNamespace("fluid.tests");
});

fluid.defaults("fluid.tests.FLUID5082Child", {
gradeNames: ["fluid.tests.FLUID5082Parent", "autoInit"],
gradeNames: ["fluid.eventedComponent", "autoInit"],
listeners: {
onCreate: [{
testEvent: [{
funcName: "fluid.tests.FLUID5082func",
namespace: "FLUID5082func", // will override
args: ["{that}", 5]
namespace: "fluid.tests.FLUID5082Parent.FLUID5082func", // will override
args: ["{FLUID5082Parent}", 5]
}, {
func: "{that}.FLUID5082invoker",
namespace: "FLUID5082invoker", // will override
args: ["{that}", 6]
func: "{FLUID5082Parent}.FLUID5082invoker",
namespace: "fluid.tests.FLUID5082Parent.FLUID5082invoker", // will override
args: ["{FLUID5082Parent}", 6]
}, {
"this": "{that}.self",
namespace: "self-FLUID5082invoker2", // will override
"this": "{FLUID5082Parent}.self",
namespace: "fluid.tests.FLUID5082Parent.self.FLUID5082invoker2", // will override
method: "FLUID5082invoker2",
args: ["{that}", 7]
args: ["{FLUID5082Parent}", 7]
}, {
funcName: "fluid.tests.FLUID5082func2", // will not override
args: ["{that}", 8]
args: ["{FLUID5082Parent}", 8]
}]
}
});

jqUnit.test("Listener Merging Tests: FLUID-5082", function () {
var that = fluid.tests.FLUID5082Parent();
that.events.testEvent.fire();
jqUnit.assertDeepEq("Base grade listeners fired", [1, 2, 3, 4], that.fireRecord);
var that2 = fluid.tests.FLUID5082Child();
// Test configuration with child superposed on parent
var that2 = fluid.tests.FLUID5082Child({gradeNames: "fluid.tests.FLUID5082Parent"});
that2.events.testEvent.fire();
jqUnit.assertDeepEq("Base grade listeners fired", [4, 5, 6, 7, 8], that2.fireRecord);
// Test configuration with child as child component - results should be identical
var that3 = fluid.tests.FLUID5082Parent( {
components: {
child: {
type: "fluid.tests.FLUID5082Child",
options: {
events: {
testEvent: "{FLUID5082Parent}.events.testEvent"
}
}
}
}
});
that3.events.testEvent.fire();
jqUnit.assertDeepEq("Base grade listeners fired", [4, 5, 6, 7, 8], that3.fireRecord);
});

/** withEnvironment tests - eventually to be deprecated **/
Expand Down

0 comments on commit d740ca7

Please sign in to comment.