diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f361607ef407170396c244a3fa98730fed4c46d..18052e718ec41d79b7d95c71ce296d83d50856f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed (for now removed features) +### Fixed + +* #101 - loading of A LOT of references in `ext_references` slows down the + webui or even freezes the brower. + ### Security (in case of vulnerabilities) ## v0.2-rc.1 (2020-04-10) diff --git a/build.properties.d/00_default.properties b/build.properties.d/00_default.properties index 046375fa9718c1acdc0a30cf46d488a8beaf3a2b..6185725aefd266b45ef5024bd3e47a78801fef1f 100644 --- a/build.properties.d/00_default.properties +++ b/build.properties.d/00_default.properties @@ -38,6 +38,12 @@ # Note: The variable BUILD_NUMBER is special and should not be used. It will be # overridden in the makefile in any case. +############################################################################## +# Modules enabled by default +############################################################################## +BUILD_MODULE_EXT_PREVIEW=ENABLED +BUILD_MODULE_EXT_RESOLVE_REFERENCES=ENABLED + ############################################################################## # Navbar properties ############################################################################## diff --git a/misc/list_references_test_data.py b/misc/list_references_test_data.py index 68371df2f3a5db4d0da5e26abba432ef846b5898..806f4222230200778da415e38a2186ca7e1d0107 100755 --- a/misc/list_references_test_data.py +++ b/misc/list_references_test_data.py @@ -1,21 +1,26 @@ #!/usr/bin/env python3 # -*- coding: utf-8 -*- """ -(C) Copyright IndiScale GmbH 2019 +(C) Copyright 2019-2020 IndiScale GmbH <info@indiscale.com> +(C) Copyright 2019-2020 Timm Fitschen <t.fitschen@indiscale.com> + +Test entities for the ext_references module. """ import caosdb import random -caosdb.execute_query("FIND Referenc*").delete() +d = caosdb.execute_query("FIND TestReferenc*") +if len(d) > 0: + d.delete() # data model datamodel = caosdb.Container() datamodel.extend([ - caosdb.RecordType("Referenced"), + caosdb.RecordType("TestReferenced"), caosdb.RecordType( - "Referencing" - ).add_property("Referenced", datatype=caosdb.LIST("Referenced")), + "TestReferencing" + ).add_property("TestReferenced", datatype=caosdb.LIST("TestReferenced")), ]) datamodel.insert() @@ -26,15 +31,15 @@ testdata = caosdb.Container() for i in range(100): testdata.append( - caosdb.Record("ReferenceObject-{}".format(i) - ).add_parent("Referenced") + caosdb.Record("TestReferenceObject-{}".format(i) + ).add_parent("TestReferenced") ) testdata.insert(); caosdb.Record().add_parent( - "Referencing" - ).add_property("Referenced", - datatype=caosdb.LIST("Referenced"), + "TestReferencing" + ).add_property("TestReferenced", + datatype=caosdb.LIST("TestReferenced"), value=testdata ).insert() diff --git a/src/core/css/webcaosdb.css b/src/core/css/webcaosdb.css index 0c8a1a5eb7b01efa947bafd7d17955582eceec44..d598e8175c249fc1457e04498ffa1af6c3f0654d 100644 --- a/src/core/css/webcaosdb.css +++ b/src/core/css/webcaosdb.css @@ -337,13 +337,6 @@ h5 { background-color: #4E5752; } -.caosdb-id { - margin:5px; - background-color:orange; - display:none; -} - - .caosdb-properties-heading { padding-top: 2px; padding-bottom: 2px; diff --git a/src/core/js/ext_references.js b/src/core/js/ext_references.js index 9e11ac70642fee2b1f3371bebb21c68f0ab949da..8676bed637fdf8c8134cc2a3a2aba1e914e2807b 100644 --- a/src/core/js/ext_references.js +++ b/src/core/js/ext_references.js @@ -328,6 +328,12 @@ var resolve_references = new function () { var _scroll_timeout = undefined; + /** + * This event is dispatched on the summary container after the summary has + * been generated and appended to the container. + */ + this.summary_ready_event = new Event("resolve_references.summary.ready"); + /** * Scroll listener calls {@link resolve_references.update_visible_references} 500 milliseconds after the * last scroll event. @@ -348,14 +354,15 @@ var resolve_references = new function () { * visible references. */ this.init = function () { - scroll_listener(); + if ("${BUILD_MODULE_EXT_RESOLVE_REFERENCES}" === "ENABLED") { + scroll_listener(); - // mainly for vertical scrolling - $(window).scroll(scroll_listener); + // mainly for vertical scrolling + $(window).scroll(scroll_listener); - // for horizontal scrolling. is this still necessary when lists are - // loaded in one go? - $(".caosdb-value-list").scroll(scroll_listener); + // for horizontal scrolling. + $(".caosdb-value-list").scroll(scroll_listener); + } } /** @@ -433,35 +440,6 @@ var resolve_references = new function () { } - /** - * Example implementation of a function which returns a reference_info for - * referenced `ReferenceObject` entities. `ReferenceObject` is a mock-up - * entity which is used to test this module. - */ - this.get_referenced = function (entity) { - return { - "text": getEntityName(entity), - "data": { - "name": getEntityName(entity), - "number": getEntityName(entity).replace( - "ReferenceObject-", "") - }, - "callback": function (ref_infos) { - var ret = $('<div/>').append("Summary: "); - var - array = [] - for (const ref_info of ref_infos) { - array.push(parseInt(ref_info["data"]["number"], - 10)); - } - ret.append(reference_list_summary - .simplify_integer_numbers(array)); - return ret[0]; - } - }; - } - - this.retrieve = retrieve; /** @@ -515,8 +493,10 @@ var resolve_references = new function () { } else if (par.name === "Box") { ret["text"] = getProperty(entity, "Number"); } else if (par.name === "Palette") { - ret["text"] = getProperty(entity, "Number"); } else if (par.name === "Referenced") { - ret = this.get_referenced(entity); + ret["text"] = getProperty(entity, "Number"); + } else if (par.name === "TestReferenced" && typeof resolve_references.test_resolver === "function") { + // this is a test case, initialized by the test suite. + ret = resolve_references.test_resolver(entity); } else { var name = getEntityName(entity); if (typeof name !== "undefined" && name.length > 0) { @@ -555,10 +535,12 @@ var resolve_references = new function () { * @return {reference_info} the resolved reference information */ this.update_single_resolvable_reference = async function (rs) { - const target = this.add_target(rs); - const id = getIDfromHREF(rs); + $(rs).find(".caosdb-id-button").hide(); + const target = resolve_references.add_target(rs); + const id = getEntityID(rs); target.textContent = id; - const resolved_entity_info = await this.resolve_reference(id); + const resolved_entity_info = await resolve_references + .resolve_reference(id); target.textContent = resolved_entity_info.text; return resolved_entity_info; } @@ -577,21 +559,27 @@ var resolve_references = new function () { */ this.add_summary_field = function (list_values) { const summary = $( - '<div class="caosdb-resolve-reference-summary"/>'); + `<div class="${resolve_references._summary_class}"/>`); $(list_values).prepend(summary); return summary[0]; } + this._summary_class = "caosdb-resolve-reference-summary"; + + /** + * All references which have not yet been resolved are contained in an HTML + * Element with this css class. + */ this._unresolved_class_name = "caosdb-resolvable-reference"; this.get_resolvable_properties = function (container) { - const _magic_class_name = this._unresolved_class_name; + const _unresolved_class_name = this._unresolved_class_name; return $(container).find(".caosdb-f-property-value").has( - `.${_magic_class_name}`).toArray(); + `.${_unresolved_class_name}`).toArray(); } - /* + /** * This function updates all references in the body which are inside of the * current viewport. * @@ -599,74 +587,113 @@ var resolve_references = new function () { * container are being processed. * * @param {HTMLElement} container - * @return {Promise[]} array of promises for ref_infos. */ - this.update_visible_references = function (container) { + this.update_visible_references = async function (container) { const property_values = resolve_references .get_resolvable_properties(container || document.body); - const _magic_class_name = resolve_references + const _unresolved_class_name = resolve_references ._unresolved_class_name; - var all_ref_infos = []; + // Loop over all property values in the container element. Note: each property_value can be a single reference or a list of references. for (const property_value of property_values) { const lists = $(property_value).find( ".caosdb-value-list").has( - `.${_magic_class_name}`); + `.${_unresolved_class_name}`); if (lists.length > 0) { - const summary_field = resolve_references - .add_summary_field(property_value); + logger.debug("processing list of references", lists); - logger.debug("processing lists of references", lists); for (var i = 0; i < lists.length; i++) { const list = lists[i]; if (resolve_references .is_in_viewport_vertically(list)) { const rs = $(list).find( - `.${_magic_class_name}`) - .toggleClass(_magic_class_name, false); - const ref_infos = []; - for (var j = 0; j < rs.length; j++) { - const ref_info = resolve_references - .update_single_resolvable_reference(rs[j]); - ref_info["index"] = j; - ref_infos.push(ref_info); + `.${_unresolved_class_name}`) + .toggleClass(_unresolved_class_name, false); + + // First resolve only one reference. If the `ref_info` + // indicates that a summary is to be generated from the + // list of references, retrieve all other other + // references. Otherwise retrieve only those which are + // visible in the viewport horizontally and trigger the + // retrieval of the others when they are scrolled into + // the view port. + const first_ref_info = await resolve_references + .update_single_resolvable_reference(rs[0]); + + first_ref_info["index"] = 0; + + if (typeof first_ref_info.callback === "function") { + // there is a callback function, hence we need to + // generate a summary. + logger.debug("loading all references for summary", + rs); + const summary_field = resolve_references + .add_summary_field(property_value); + + // collect ref infos for the summary + const ref_infos = [first_ref_info]; + for (var j = 1; j < rs.length; j++) { + const ref_info = resolve_references + .update_single_resolvable_reference(rs[j]); + ref_info["index"] = j; + ref_infos.push(ref_info); + } + + // wait for resolution of references, + // then generate the summary, + // dispatch event when ready. + Promise.all(ref_infos) + .then(_ref_infos => {reference_list_summary + .generate(_ref_infos, summary_field);}) + .then(() => { + summary_field.dispatchEvent( + resolve_references + .summary_ready_event + ); logger.error("here");}) + .catch((err) => { + logger.error(err); + }) + + } else { + // no summary to be generated + + logger.debug("lazy loading references", rs); + for (var j = 1; j < rs.length; j++) { + // mark others to be loaded later and only if + // visible + $(rs[j]).toggleClass(_unresolved_class_name, true); + } } - - // wait for resolution of references, - // then generate the summary. - Promise.all(ref_infos) - .then(_ref_infos => reference_list_summary - .generate(_ref_infos, summary_field)) - .catch(logger.error); - - all_ref_infos = all_ref_infos.concat(ref_infos); } } - } else { - // TODO merge code with the above? - const rs = $(property_value).find( - `.${_magic_class_name}`); - logger.debug("processing single references", rs); - for (var i = 0; i < rs.length; i++) { - if (resolve_references.is_in_viewport_vertically( - rs[i]) && - resolve_references.is_in_viewport_horizontally( - rs[i])) { - $(rs).toggleClass(_magic_class_name, false); - all_ref_infos.push(resolve_references - .update_single_resolvable_reference(rs[i])); - } + } + + // Load all remaining references. These are single reference values + // and those references from lists which are left for lazy loading. + const rs = $(property_value).find( + `.${_unresolved_class_name}`); + for (var i = 0; i < rs.length; i++) { + if (resolve_references.is_in_viewport_vertically( + rs[i]) && + resolve_references.is_in_viewport_horizontally( + rs[i])) { + logger.debug("processing single references", rs); + $(rs[i]).toggleClass(_unresolved_class_name, false); + + // discard return value as it is not needed for any summary + // generation as above. + resolve_references.update_single_resolvable_reference(rs[i]); } } } - - return all_ref_infos; } } $(document).ready(function () { - caosdb_modules.register(resolve_references); + if ("${BUILD_MODULE_EXT_RESOLVE_REFERENCES}" === "ENABLED") { + caosdb_modules.register(resolve_references); + } }); diff --git a/src/core/js/preview.js b/src/core/js/preview.js index 04a7b699b2d1adae5143f7362e965828c3d61531..9af9bb0366097f6d1017a46ff077874ab34bf773 100644 --- a/src/core/js/preview.js +++ b/src/core/js/preview.js @@ -489,6 +489,9 @@ var preview = new function() { nextButton.innerHTML = preview.carouselNextButtonInnerHTML; let nav = $('<div class="' + preview.classNamePreviewCarouselNav + '"></div>')[0]; let selectors = refLinksContainer.cloneNode(true); + // TODO handle case when ext_references already removed the + // resolvable-reference class but did not resolve it yet. + $(selectors).show(); $(selectors).find('a,button,.btn').each((index, button) => { $(button).toggleClass("active", index === 0); @@ -548,17 +551,6 @@ var preview = new function() { return null; } - /** - * Create a div with class `item` which wraps the first argument. - * @param {HTMLElement} entity - * @returns {HTMLElement} - */ - this.createSlideItem = function(entity) { - let item = $('<div class="item"></div>')[0]; - item.appendChild(entity); - return item; - } - /** * Find the index of the div.item.active element withing the div.carouse-inner element. * @@ -726,4 +718,8 @@ var preview = new function() { }; -$(document).ready(preview.init); +$(document).ready(function () { + if ("${BUILD_MODULE_EXT_PREVIEW}" === "ENABLED") { + caosdb_modules.register(preview); + } +}); diff --git a/src/core/js/webcaosdb.js b/src/core/js/webcaosdb.js index 87b9e744c5e9d876347e0764c567df1a02f978bd..b496c3b7a176828cf99a3efd017d591dde2e937b 100644 --- a/src/core/js/webcaosdb.js +++ b/src/core/js/webcaosdb.js @@ -150,6 +150,9 @@ this.connection = new function() { * Send a get request. */ this.get = async function _get(uri) { + if (typeof uri === "undefined" || uri === "undefined") { + throw new Error("uri was undefined"); + } try { return await $.ajax({ url: connection.getBasePath() + uri, diff --git a/src/core/xsl/entity.xsl b/src/core/xsl/entity.xsl index 7187c6b7d183b340a8eaee4cbdcead7ada44d433..6b709d0e25647807d852bf9d6b778b54ff543d08 100644 --- a/src/core/xsl/entity.xsl +++ b/src/core/xsl/entity.xsl @@ -146,7 +146,7 @@ </xsl:attribute> </span> <xsl:apply-templates mode="backreference-link" select="@id"/> - <span class="label caosdb-id caosdb-id-button"> + <span class="label caosdb-id caosdb-id-button hidden"> <xsl:value-of select="@id"/> </span> </h5> diff --git a/src/core/xsl/filesystem.xsl b/src/core/xsl/filesystem.xsl index 93028ed2f9667f5f7b673483cb87dacb2591d5ac..38a772209fb1e199cee8a81a3f02ac1cf7a5da44 100644 --- a/src/core/xsl/filesystem.xsl +++ b/src/core/xsl/filesystem.xsl @@ -93,7 +93,7 @@ <xsl:value-of select="concat($entitypath, @id)"/> </xsl:attribute> <span class="label caosdb-label-file">F</span> - <span class="label caosdb-id"> + <span class="label caosdb-id hidden"> <xsl:value-of select="@id"/> </span> </a> diff --git a/test/core/js/modules/ext_references.js.js b/test/core/js/modules/ext_references.js.js index 9c64a78826f033378cc92f79510399f44023d8b8..38d61a9ac18d1bccaddd5cc7cdad1d1c9d3b5498 100644 --- a/test/core/js/modules/ext_references.js.js +++ b/test/core/js/modules/ext_references.js.js @@ -25,21 +25,55 @@ /* SETUP ext_references module */ QUnit.module("ext_references.js", { before: function(assert) { - // dummy test functions - var logger = log.getLogger("ext_references.js") - logger.setLevel("debug"); + // dummy test functions resolve_references.retrieve = async function (id) { - logger.debug("retrieve", id); return transformation.transformEntities(str2xml( `<Response> - <Record id="${id}" name="ReferenceObject-${id}"> - <Parent name="Referenced"/> + <Record id="${id}" name="TestReferenceObject-${id}"> + <Parent name="TestReferenced"/> </Record> </Response>` )); + }; + + /** + * Example implementation of a function which returns a reference_info + * for referenced `TestReferenceObject` entities. + * `TestReferenceObject` is a mock-up entity which is used to test this + * module. + */ + this.test_resolver = function (with_cb) { + return function (entity) { + callback = !with_cb || function (ref_infos) { + console.log(ref_infos); + var ret = $('<div/>').append("Summary: "); + var array = []; + for (const ref_info of ref_infos) { + array.push(parseInt(ref_info["data"]["number"], + 10)); + } + ret.append(reference_list_summary + .simplify_integer_numbers(array)); + return ret[0]; + } + return { + "text": getEntityName(entity), + "data": { + "id": getEntityID(entity), + "name": getEntityName(entity), + "number": getEntityName(entity).replace( + "TestReferenceObject-", "") + }, + "callback": callback + }; + }; } + + }, + beforeEach: function(assert) { + resolve_references.test_resolver = this.test_resolver(true); resolve_references.is_in_viewport_horizontally = () => true; resolve_references.is_in_viewport_vertically = () => true; } @@ -57,42 +91,111 @@ QUnit.test("get_person_str", function(assert){ assert.ok(resolve_references.get_person_str); }); -QUnit.test("update_visible_references", async function(assert){ - const f = resolve_references.update_visible_references; +QUnit.test("update_visible_references_without_summary", async function(assert){ + resolve_references.test_resolver = this.test_resolver(false); + resolve_references.is_in_viewport_horizontally = () => false; + const update_visible_refs = resolve_references.update_visible_references; const test_property = $(`<div class="caosdb-f-property-value"> <div data-entity-id="15" class="${resolve_references._unresolved_class_name}"> - <span class="${resolve_references._target_class}"/></span> </div> </div><div class="caosdb-f-property-value"> <div class="caosdb-value-list"> <div data-entity-id="16" class="${resolve_references._unresolved_class_name}"> - <span class="${resolve_references._target_class}"/></span> </div> <div data-entity-id="17" class="${resolve_references._unresolved_class_name}"> - <span class="${resolve_references._target_class}"/></span> </div> </div> </div>`); assert.equal(test_property.find(`.${resolve_references._unresolved_class_name}`).length, 3, "is unresolved"); - const ref_infos = f($("<div/>").append(test_property)); + await update_visible_refs($("<div/>").append(test_property)); - assert.equal(test_property.find(`.${resolve_references._unresolved_class_name}`).length, 0, "is resolved"); + assert.equal(test_property.find(`.${resolve_references._unresolved_class_name}`).length, 2, "only the first of the list is resolved"); + assert.ok(test_property.find(`[data-entity-id='15']`).is(`.${resolve_references._unresolved_class_name}`), "single is not resolve (not visible horizontally)."); + assert.notOk(test_property.find(`[data-entity-id='16']`).is(`.${resolve_references._unresolved_class_name}`), "first of list is resolved"); + assert.ok(test_property.find(`[data-entity-id='17']`).is(`.${resolve_references._unresolved_class_name}`), "second of list is not resolved"); + + // target element was generated + const target_elem = test_property + .find(`.${resolve_references._target_class}`); + assert.equal(target_elem.length, 1, "one target is there"); + // no summary has been generated + const summary_elem = test_property + .find(`.${resolve_references._summary_class}`); + assert.equal(summary_elem.length, 0, "no summary is there"); - assert.equal(ref_infos.length, 3, "three ref_infos"); +}); + +QUnit.test("update_visible_references_with_summary", async function(assert){ + const update_visible_refs = resolve_references.update_visible_references; + + const test_property = $(`<div class="caosdb-f-property-value"> + <div data-entity-id="15" + class="${resolve_references._unresolved_class_name}"> + </div> + </div><div class="caosdb-f-property-value"> + <div class="caosdb-value-list"> + <div data-entity-id="16" + class="${resolve_references._unresolved_class_name}"> + </div> + <div data-entity-id="17" + class="${resolve_references._unresolved_class_name}"> + </div> + </div> + </div>`); + + assert.equal(test_property.find(`.${resolve_references._unresolved_class_name}`).length, 3, "is unresolved"); + + // the summary is created asyncronously and dispatches an event when it's + // ready. var done = assert.async(); - Promise.all(ref_infos).then(ref_infos => { - assert.equal(ref_infos[1].data.name.replace(/[0-9]*/g, ""), "ReferenceObject-", "has data"); - assert.ok(typeof ref_infos[1].callback === "function", - "has callback"); + test_property[1].addEventListener(resolve_references.summary_ready_event.type, (e) => { + assert.equal(e.target.textContent, "Summary: 16, 17", "summary text is correct"); + test_property.remove(); done(); - }); + }, true); + + await update_visible_refs($(document.body).append(test_property)); + + assert.equal(test_property.find(`.${resolve_references._unresolved_class_name}`).length, 0, "is resolved"); + + + // target element was generated + const target_elem = test_property + .find(`.${resolve_references._target_class}`); + assert.equal(target_elem.length, 3, "three targets are there"); + + // summary container is there, this is the case even before the summary + // itself is ready. + const summary_elem = test_property + .find(`.${resolve_references._summary_class}`); + assert.equal(summary_elem.length, 1, "one summary is there"); + + +}); + + +/** + * Test the generation of ref_info objects from resolvable references. + */ +QUnit.test("update_single_resolvable_reference", async function(assert) { + const update_single = resolve_references.update_single_resolvable_reference; + + const test_property = $(`<div class="caosdb-f-property-value"> + <div class="caosdb-id ${resolve_references._unresolved_class_name}">15</div></div>`); + + const ref_info = await update_single(test_property[0]); + + // ref_info has expected fields, data and callback + assert.equal(ref_info.data.name, "TestReferenceObject-15", "has data"); + assert.equal(ref_info.data.id, "15", "id was parsed correctly"); + assert.equal(typeof ref_info.callback, "function", "has callback"); }); diff --git a/test/core/js/modules/webcaosdb.js.js b/test/core/js/modules/webcaosdb.js.js index 9ab5c5a3f5997d4dc9165466ed117fafaa25b7fc..3b615e11e4bd96c26ecc0d8e1cb9d8220a4cba8e 100644 --- a/test/core/js/modules/webcaosdb.js.js +++ b/test/core/js/modules/webcaosdb.js.js @@ -741,22 +741,6 @@ QUnit.test("removeAllWaitingNotifications", function(assert) { assert.equal(emptyElem, removeAllWaitingNotifications(emptyElem), "empty elem works"); }); -QUnit.test("createSlideItem", function(assert) { - assert.ok(preview.createSlideItem, "function available"); - let p_elem = $('<div>preview</div>')[0]; - - assert.throws(() => { - preview.createSlideItem(); - }, "no parameter throws"); - assert.throws(() => { - preview.createSlideItem(null); - }, "null parameter throws"); - - let item = preview.createSlideItem(p_elem); - assert.ok($(item).hasClass("item"), "has class item"); - assert.equal(p_elem, item.firstChild, "is wrapped"); -}); - QUnit.test("getActiveSlideItemIndex", function(assert) { assert.ok(preview.getActiveSlideItemIndex, "function available"); let okElem0 = $('<div><div class="carousel-inner">' +