From 8aa31bf843aebe010df8daadf75c5fc0fd127474 Mon Sep 17 00:00:00 2001 From: Marco Brandizi Date: Tue, 11 Jul 2023 13:46:54 +0100 Subject: [PATCH] Reviewing Js code. --- .../webapp/html/GeneView/summary-legend.js | 13 ++- .../main/webapp/html/javascript/data-utils.js | 73 ++++++++---- .../webapp/html/javascript/evidence-table.js | 13 ++- .../webapp/html/javascript/web-cache-new.js | 105 ++++++++++++++++++ .../main/webapp/html/javascript/web-cache.js | 14 ++- 5 files changed, 189 insertions(+), 29 deletions(-) create mode 100644 client-base/src/main/webapp/html/javascript/web-cache-new.js diff --git a/client-base/src/main/webapp/html/GeneView/summary-legend.js b/client-base/src/main/webapp/html/GeneView/summary-legend.js index ed969b1eb..6bcbfcdeb 100644 --- a/client-base/src/main/webapp/html/GeneView/summary-legend.js +++ b/client-base/src/main/webapp/html/GeneView/summary-legend.js @@ -58,11 +58,15 @@ function filterGeneTableByType ( event, conceptType ) if ( !conceptEvidences ) return false // just in case // Splits the gene evidences string in the gene table into an array of evidences. // See the API for details about this format + + /* TODO: remove, use the available facilities for these + operations let evidences = [] for(let evidence in conceptEvidences){ evidences.push(evidence) - } - return selectedTypes.every ( t => evidences.includes ( t ) ) + } */ + const rowEvidences = Object.keys ( conceptEvidences ) + return selectedTypes.every ( t => rowEvidences.includes ( t ) ) } _filterKnetTableByType ( @@ -74,8 +78,7 @@ function filterGeneTableByType ( event, conceptType ) function filterEvidenceTableByType ( event, conceptType ) { const rowFilterPred = ( selectedTypes, tableRow ) => { - const rowEvidencesString = tableRow.conceptType - return selectedTypes.some ( t => t == rowEvidencesString ) + return selectedTypes.some ( t => t == tableRow.conceptType ) } _filterKnetTableByType ( @@ -84,7 +87,7 @@ function filterEvidenceTableByType ( event, conceptType ) ) } -// function updates, store and checks for non-active legend keys +// function updates, store and checks for non-active legend keys function updateLegendsKeys(key, location, event) { const currentTable = $(`#${location}`) diff --git a/client-base/src/main/webapp/html/javascript/data-utils.js b/client-base/src/main/webapp/html/javascript/data-utils.js index 151397b3d..32a8aeca5 100644 --- a/client-base/src/main/webapp/html/javascript/data-utils.js +++ b/client-base/src/main/webapp/html/javascript/data-utils.js @@ -55,7 +55,7 @@ function searchKeyword() { // api request var request = "/" + searchMode; - + // TODO: possibly related to #768 //if(geneList_size > freegenelist_limit) { // check if user logged in and if yes, get user_id @@ -81,6 +81,31 @@ function searchKeyword() { * checks user login status and, in case of success, calls the API specified in searchMode and * requestParams. * + * TODO: (when we have time, don't remove this commnt until completed, possubly move it to a GH issue) + * This is a callback hell (google for it), checkUserPlan() receives + * parameters it shouldn't deal with at all, just to pass them along. + * + * The modern, cleaner way to do the same is to use promises, althogh in this case, checkUserPlan() + * needs to do some wrapping around the call back to be chained after success. So, it needs to become + * like: + * + * function checkUserPlan ( onComplete ) // pass the callback, not its details or parameters + * { + * $.ajax({ + * ... + * complete: function () { + * ... // the same current pre-processing + * onComplete() // in place of the current requestGenomeData() + * ... // the same current post-procesding + * } + * }) + * } + * + * // This would be the invocation in searchKeyword() above + * // don't give the requestGenomeData() params to checkUserPlan(), it doesn't really need + * // to know them. + * checkUserPlan( () => requestGenomeData(...) ) + * */ function checkUserPlan() { var login_check_url = knetspace_api_host + "/api/v1/me"; @@ -934,18 +959,25 @@ class GenesListManager { * Converts genetable JSON format data to TSV format removing conceptEvidences and qtlEvidence properties. */ function geneTableToTsv(data) { + + /* TODO: remove. Stop coding these transformations this way. const genesArrayExclEvidences = [] data.forEach(genes => { const { conceptEvidences, qtlEvidences, ...genesWithoutEvidence } = genes genesArrayExclEvidences.push(genesWithoutEvidence) }) + */ + const genesArrayExclEvidences = data.map ( geneTableRow => { + // TODO: remove WHAT SORT OF NAME IS genesWithoutEvidence?!? + // const { conceptEvidences, qtlEvidences, ...genesWithoutEvidence } = genes + const { conceptEvidences, qtlEvidences, ...filteredFields} = geneTableRow + return filteredFields + }) const tsvFormat = formatJsonToTsv(genesArrayExclEvidences); return tsvFormat; - - } /** @@ -972,9 +1004,25 @@ function formatJsonToTsv(data) { } +// function replace gene and evidence genome data ondexId key with nodeId +function replaceOndexId(tableData) { -// TODO: see init-utils.js -// + const refinedTableData = tableData.map(({ + ondexId: nodeId, + ...data + }) => ({ + nodeId, + ...data + })) + + return refinedTableData +} + + +/* + * TODO: see init-utils.js + * Keep this test/provisional code ad the end of files. + */ if (TEST_MODE) { function testGeneTable2OldString() { let testTableJs = [ @@ -1120,18 +1168,3 @@ if (TEST_MODE) { testEvidenceTable2OldString() } // if TEST_MODE - - -// function replace gene and evidence genome data ondexId key with nodeId -function replaceOndexId(tableData) { - - const refinedTableData = tableData.map(({ - ondexId: nodeId, - ...data - }) => ({ - nodeId, - ...data - })) - - return refinedTableData -} \ No newline at end of file diff --git a/client-base/src/main/webapp/html/javascript/evidence-table.js b/client-base/src/main/webapp/html/javascript/evidence-table.js index 80360ddc8..d914f7051 100644 --- a/client-base/src/main/webapp/html/javascript/evidence-table.js +++ b/client-base/src/main/webapp/html/javascript/evidence-table.js @@ -343,7 +343,7 @@ function triggerAccessionToolTips() { var downloadToolTip = createAccessionToolTips('.accession-downloadicon', 'Download full table.', 'download'); - // TODO: too much repition + // TODO: too much repeatition $(".accession-clipboard").mouseover(function (e) { e.preventDefault(); @@ -417,7 +417,16 @@ function createEvidenceTableBody ( tableData, doAppend = false ) const fromRow = evidenceTableScroller.getPageStart () const toRow = evidenceTableScroller.getPageEnd () - tableData.forEach( (evidence,index) => + /** + * TODO: how can this work, if before we were looping on the fromRow/toRow window? + * + * Is it because it now gets the rendered window only? If yes, remove fromRow, toRow declarations, + * but then what's evidenceTableScroller for? + * + * if it's a bug, fix it with a loop over the right window + */ + + tableData.forEach( (evidence,index) => { let {conceptType, name, pvalue, totalGenesSize, geneList, nodeId, userGenesSize, userGeneAccessions } = evidence diff --git a/client-base/src/main/webapp/html/javascript/web-cache-new.js b/client-base/src/main/webapp/html/javascript/web-cache-new.js new file mode 100644 index 000000000..881fb477e --- /dev/null +++ b/client-base/src/main/webapp/html/javascript/web-cache-new.js @@ -0,0 +1,105 @@ +/** + * Manages the caching for web requests, with a specific new entry handler that + * make URL calls and deals with possible errors. + * + * Currently, we're only using the subclass EvidenceAccessionCache in openGeneListPopup()/evidence-table.js + * + */ +class WebCacheWrapper +{ + #cacheName = null + + constructor ( cacheName ) { + this.#cacheName = cacheName + } + + + /** + * Entry getter that uses the cache-through approach, ie, if the entry is already cached, + * returns it, if not, uses #apiHandler() to get data, caches the result and then return them. + * + * This is the main method to access the cache, it is used like: + * + * let data = cacheWrapper.get ( "http://foo.url?param=" + param ) // options can be omitted + * let data = cacheWrapper.get ( "http://foo.url?param=" + param, { timeout: 1000 } ) + * + * + * + * ==> Note that, due to the cache-through behaviour, you DO NOT need to call the URL yourself, + * cause the method already takes care of that if the call output isn't already in the cache, + * and you should assume some data always come out of this method (else, there's an error, or + * there might be an empty result). + * + */ + async get ( requestUrl, options = { data: '', timeout: 100000 } ) + { + let result = await caches.match ( request, options ) + if ( result ) return result.json () + + result = await this._apiHandler ( requestUrl, options ); + + caches.open ( this.#cacheName ).then ( (cache) => + { + // TODO: do we need to serialize/unserialize JSON, can't we just store and return 'result'? + // + cache.put ( request, new Response ( JSON.stringify ( result ), options ) ) + }) + + return result + } + + + /** + * Calls our API with the given request and options. It takes care of options and possible API call + * errors. + * + * Used by get(), when an entry isn't already cached + * + * Note that this IS NOT USED to save data in the cache, this is the job of get(). + * It's named with '_', cause it is a PROTECTED method and MUST NOT call it outside of + * its class or its extensions. + * + */ + async _apiHandler ( requestUrl, options ) + { + try { + const result = await $.get( { url:requestUrl, ...options } ) + return result + } + catch ( err ) + { + // TODO: report the error + jboxNotice ( 'An error occured, kindly try again', 'red', 300, 2000 ); + return null + } + } + +} + +/** + * Extended class caters solely for AccessionTable Popup, called in evidence-table(openGeneListPopup()) + * + */ + +class EvidenceAccessionCache extends WebCacheWrapper +{ + async get ( conceptId ) + { + return super.get ( + api_url + '/genome?keyword=ConceptID:' + conceptId, + { data: '', timeout: 100000, headers: { 'Content-Type': 'text/plain' } } + ) + } + + async _apiHandler ( requestUrl, options ) + { + let result = await super._apiHandler ( requestUrl, options ) + if ( !result ) return result + + // TODO: WILL BE REMOVED IN COMING DAYS + let geneTable = formatJsonToTsv(response.geneTable) + geneTable = geneTable.split("\n") + + return geneTable + } +} diff --git a/client-base/src/main/webapp/html/javascript/web-cache.js b/client-base/src/main/webapp/html/javascript/web-cache.js index 838fee13a..1cc36c272 100644 --- a/client-base/src/main/webapp/html/javascript/web-cache.js +++ b/client-base/src/main/webapp/html/javascript/web-cache.js @@ -1,3 +1,15 @@ +/** + * TODO: THIS HAS BECOME WRONG IN UNINMAGINABLE WAYS!!! + * + * See web-cache-new.js, study that code, replace this mess here with that (ie, copy-paste here), use it for the + * evidence table (ie, use get() and assume it always return a result), test it, and eventually commit + * (and also remove the -new file). + * + * ==> I'm not 100% sure that code is correct and corresponds to what you're trying to do, please DO + * understand it before use and come back to me if you need clarifications. + * + */ + /** * Manages the caching for web requests, with a specific new entry handler that * make URL calls and deals with possible errors. @@ -169,5 +181,3 @@ } } - -