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.

Sign up for GitHub

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

Jump to bottom

Move all URLs to a separate file & improve getbin method #762

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Move all URLs to a separate file & improve getbin method #762

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ccc8a03
Add throttling to download network call and move all URLs to separate…
prajwalkulkarni Jan 28, 2024
f149c1f
Update import path
prajwalkulkarni Jan 28, 2024
fbd3ce7
Minor changes to utilities.ts
prajwalkulkarni Feb 6, 2024
705d8b1
Merge branch 'master' of https://github.com/prajwalkulkarni/3Dmol.js …
prajwalkulkarni Feb 28, 2024
3dbd73d
Remove throttling
prajwalkulkarni Feb 28, 2024
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
3 changes: 3 additions & 0 deletions src/URLs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const RCSB_FILES_URL = "https://files.rcsb.org/view/";
export const NCBI_PUBCHEM_URL = "https://pubchem.ncbi.nlm.nih.gov/rest/pug/compound/cid/";
export const RCSB_MMTF_URL = "https://mmtf.rcsb.org/v1.0/full/";
9 changes: 5 additions & 4 deletions src/autoload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { GLViewer, createViewer } from "./GLViewer";
import { SurfaceType } from "./ProteinSurface4";
import { get, specStringToObject } from "./utilities";
import { CC } from "./colors";
import { NCBI_PUBCHEM_URL, RCSB_FILES_URL } from "./URLs";

export var autoinit = false;
export var processing_autoinit = false;
Expand Down Expand Up @@ -40,12 +41,12 @@ export function autoload(viewer?: any, callback?: (arg0: any) => void) {

type = null;
if (viewerdiv.dataset.pdb) {
datauri.push("https://files.rcsb.org/view/" + viewerdiv.dataset.pdb + ".pdb");
datauri.push(RCSB_FILES_URL + viewerdiv.dataset.pdb + ".pdb");
datatypes.push("pdb");
} else if (viewerdiv.dataset.cid) {
//this doesn't actually work since pubchem does have CORS enabled
datatypes.push("sdf");
datauri.push("https://pubchem.ncbi.nlm.nih.gov/rest/pug/compound/cid/" + viewerdiv.dataset.cid +
datauri.push(NCBI_PUBCHEM_URL + viewerdiv.dataset.cid +
"/SDF?record_type=3d");
}
else if (viewerdiv.dataset.href || viewerdiv.dataset.url) {
Expand All @@ -67,15 +68,15 @@ export function autoload(viewer?: any, callback?: (arg0: any) => void) {
var divdata = viewerdiv.dataset;
for (i in divdata) {
if ((i.substring(0, 3) === "pdb" && (i !== "pdb"))) {
datauri.push("https://files.rcsb.org/view/" + divdata[i] + ".pdb");
datauri.push(RCSB_FILES_URL + divdata[i] + ".pdb");
datatypes.push('pdb');

} else if (i.substring(0, 4) === "href" && (i !== "href")) {
uri = divdata[i];
datauri.push(uri);
datatypes.push(uri.substring(uri.lastIndexOf('.') + 1));
} else if (i.substring(0, 3) === "cid" && (i !== "cid")) {
datauri.push("https://pubchem.ncbi.nlm.nih.gov/rest/pug/compound/cid/" + divdata[i] + "/SDF?record_type=3d");
datauri.push(NCBI_PUBCHEM_URL + divdata[i] + "/SDF?record_type=3d");
datatypes.push('sdf');
}
}
Expand Down
54 changes: 31 additions & 23 deletions src/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//a collection of miscellaneous utility functions

import { NCBI_PUBCHEM_URL, RCSB_FILES_URL, RCSB_MMTF_URL } from "./URLs";
import { builtinGradients, Gradient } from "./Gradient";
import { VolumeData } from "./VolumeData";
import { builtinColorSchemes, CC, elementColors, htmlColors, Color } from "./colors";
Expand Down Expand Up @@ -344,20 +345,22 @@ function checkStatus(response) {
return response;
}

let isRequestProcessing = false;
/**
* Fetch data from URL
*
* @param uri URL
* @param callback Function to call with data
*/
export function get(uri, callback?) {
var promise = fetch(uri).then(checkStatus).then((response) => response.text());
export function get(uri:string, callback?) {
const promise = fetch(uri).then(checkStatus).then((response) => response.text()).finally(()=>isRequestProcessing = false);
if (callback)
return promise.then(callback);
else
return promise;
return promise;
}


type RequestMethod = "GET"|"POST"|"PUT"|"DELETE"|"HEAD"|"OPTIONS"|"PATCH";
/**
* Download binary data (e.g. a gzipped file) into an array buffer and provide
* arraybuffer to callback.
Expand All @@ -367,19 +370,16 @@ export function get(uri, callback?) {
* @param {string} [postdata] - data for POST request
* @return {Promise}
*/
export function getbin(uri, callback?, request?, postdata?) {
var promise;
if (request == "POST") {
promise = fetch(uri, { method: 'POST', body: postdata })
.then((response) => checkStatus(response))
.then((response) => response.arrayBuffer());
} else {
promise = fetch(uri).then((response) => checkStatus(response))
.then((response) => response.arrayBuffer());
}

export function getbin(uri:string, callback?, request?: RequestMethod, postdata?) {
const promise = fetch(uri, { method: request || "GET", body: postdata })
.then((response) => checkStatus(response))
.then((response) => response.arrayBuffer())
.finally(()=>isRequestProcessing = false);


if (callback) return promise.then(callback);
else return promise;
return promise;
};


Expand Down Expand Up @@ -409,6 +409,11 @@ export function download(query, viewer, options, callback?) {
var promise = null;
var m = viewer.addModel();

if(isRequestProcessing){
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not see why you wouldn't want to allow multi-threaded downloads. Serializing them is not a feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ummm...I'm not sure of the use case for downloading the same data multiple times, and users might unintentionally end up performing the same action multiple times due to a lack of visual indication and hence I feel we should throttle the action. Kindly let me know if I'm missing something here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you assume it is the same data? getbin is a general utility. It is not acceptable for someone to try to fetch two different files at the same time and get an error on the second one.

Copy link
Contributor Author

@prajwalkulkarni prajwalkulkarni Feb 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

getbin is a general utility. It is not acceptable for someone to try to fetch two different files at the same time and get an error on the second one.

I 100% agree with this. However, the throttling functionality was added only to the download method and not getbin so fetching multiple files from getbin shouldn't block multiple downloads at once. Anyway, I further double-checked to see if the download method is being used elsewhere and I could see that it is used in example.html of tests and in several html files of tests/webpages. And, it turns out that in webpages/splitviewer.html the download method is invoked in a loop and throttling would cause a bottleneck here. I had overseen this earlier and hence I was not aware. I shall remove the throttling changes and push a commit. Thank you for bringing this to my attention :)

Copy link
Contributor

Choose a reason for hiding this comment

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

More to the point, it is part of the public API so anyone can use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. Going ahead, I'll keep a broader view when proposing changes.

alert("Request already processing, please wait");
return;
}

if (query.indexOf(':') < 0) {
//no type specifier, guess
if (query.length == 4) {
Expand All @@ -420,13 +425,14 @@ export function download(query, viewer, options, callback?) {
}
}
if (query.substring(0, 5) === 'mmtf:') {
pdbUri = options && options.pdbUri ? options.pdbUri : "https://mmtf.rcsb.org/v1.0/full/";
pdbUri = options && options.pdbUri ? options.pdbUri : RCSB_MMTF_URL;
query = query.substring(5).toUpperCase();
uri = pdbUri + query;
if (options && typeof options.noComputeSecondaryStructure === 'undefined') {
//when fetch directly from pdb, trust structure annotations
options.noComputeSecondaryStructure = true;
}
isRequestProcessing = true;
promise = new Promise(function (resolve) {
getbin(uri)
.then(function (ret) {
Expand Down Expand Up @@ -454,21 +460,21 @@ export function download(query, viewer, options, callback?) {
return;
}
if (type == 'mmtf') {
mmtfUri = options && options.mmtfUri ? options.mmtfUri : 'https://mmtf.rcsb.org/v1.0/full/';
mmtfUri = options && options.mmtfUri ? options.mmtfUri : RCSB_MMTF_URL;
uri = mmtfUri + query.toUpperCase();
}
else {
pdbUri = options && options.pdbUri ? options.pdbUri : "https://files.rcsb.org/view/";
pdbUri = options && options.pdbUri ? options.pdbUri : RCSB_FILES_URL;
uri = pdbUri + query + "." + type;
}

} else if (query.substring(0, 4) == 'cid:') {
type = "sdf";
query = query.substring(4);
if (!query.match(/^[0-9]+$/)) {
alert("Wrong Compound ID"); return;
return alert("Wrong Compound ID");
}
uri = "https://pubchem.ncbi.nlm.nih.gov/rest/pug/compound/cid/" + query +
uri = NCBI_PUBCHEM_URL + query +
"/SDF?record_type=3d";
} else if (query.substring(0, 4) == 'url:') {
uri = query.substring(4);
Expand All @@ -480,15 +486,16 @@ export function download(query, viewer, options, callback?) {
viewer.zoomTo();
viewer.render();
};
isRequestProcessing = true;
promise = new Promise(function (resolve) {
if (type == 'mmtf') { //binary data
if (type === 'mmtf') { //binary data
getbin(uri)
.then(function (ret) {
handler(ret);
resolve(m);
}).catch(function () {
//if mmtf server is being annoying, fallback to text
pdbUri = options && options.pdbUri ? options.pdbUri : "https://files.rcsb.org/view/";
pdbUri = options && options.pdbUri ? options.pdbUri : RCSB_FILES_URL;
uri = pdbUri + query + ".pdb";
type = "pdb";
console.log("falling back to pdb format");
Expand Down Expand Up @@ -520,7 +527,8 @@ export function download(query, viewer, options, callback?) {
});
return m;
}
else return promise;

return promise;
};


Expand Down
Loading