From fbef3d33645e7bb1c501b74d24bd5a255b9ee259 Mon Sep 17 00:00:00 2001 From: Lum Date: Tue, 9 Sep 2025 10:46:08 -0700 Subject: [PATCH 1/7] do a better job of showing errors that might occur during run import --- .../web/signaldata/UploadView/UploadLog.js | 60 +++++++++++++++---- .../UploadView/uploadResultFiles.js | 6 +- 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/signalData/resources/web/signaldata/UploadView/UploadLog.js b/signalData/resources/web/signaldata/UploadView/UploadLog.js index 643372aa8..de4d1a7c6 100644 --- a/signalData/resources/web/signaldata/UploadView/UploadLog.js +++ b/signalData/resources/web/signaldata/UploadView/UploadLog.js @@ -244,17 +244,56 @@ Ext4.define('LABKEY.SignalData.UploadLog', { if (Ext4.isFunction(callback)) { var me = this; var destination = this.fileSystem.concatPaths(this.fileSystem.getBaseURL(), targetDirectory); - this.fileSystem.renamePath({ - source: this.getWorkingPath(), - destination: destination, - isFile: false, - success: function () { - me.resolveFileResources(targetDirectory, callback, scope, runProperties); + + const getResourcesCallback = function(files) { + if (files && files.length > 0) { + // files currently exist, validate and resolve before creating the run + me.resolveDataFileURL(files, callback, scope, runProperties); } - }); + else { + // files need to be moved to the target directory + this.fileSystem.renamePath({ + source: this.getWorkingPath(), + destination: destination, + isFile: false, + success: function () { + me.resolveFileResources(targetDirectory, callback, scope, runProperties); + } + }); + } + }; + + // get the list of files in the target directory (if any) + this.getTargetDirResources(targetDirectory, getResourcesCallback, this); } }, + /** + * Returns the list of data files in the target directory. Uploaded run data files are moved from + * a temporary location to the target directory when the run is created. + */ + getTargetDirResources : function (targetDirectory, callback, callbackScope) { + var fileUri = this.fileSystem.concatPaths(this.fileSystem.getAbsoluteURL(), targetDirectory); + LABKEY.Ajax.request({ + url: fileUri, + method: 'GET', + params: {method: 'JSON'}, + success: function (response) { + var json = Ext4.decode(response.responseText); + var files = []; + if (Ext4.isDefined(json) && Ext4.isArray(json.files)) + files = json.files; + + callback.call(callbackScope, files); + }, + failure: function() { + // this is normal in the case where the target directory does not yet exist or + // the files have not yet been moved there + callback.call(callbackScope, []); + } + }); + }, + resolveFileResources: function (targetDirectory, callback, callbackScope, runProperties) { var fileUri = this.fileSystem.concatPaths(this.fileSystem.getAbsoluteURL(), targetDirectory); LABKEY.Ajax.request({ @@ -269,9 +308,10 @@ Ext4.define('LABKEY.SignalData.UploadLog', { } } }, - failure: function () { - } - , scope: this + failure: LABKEY.Utils.getCallbackWrapper(function(json, response, opts) { + LABKEY.Utils.alert('Error', 'Unable to resolve file resources : ' + response.exception); + }, this, true), + scope: this }, this); }, diff --git a/signalData/resources/web/signaldata/UploadView/uploadResultFiles.js b/signalData/resources/web/signaldata/UploadView/uploadResultFiles.js index f51a34f37..37445cd71 100644 --- a/signalData/resources/web/signaldata/UploadView/uploadResultFiles.js +++ b/signalData/resources/web/signaldata/UploadView/uploadResultFiles.js @@ -366,9 +366,9 @@ LABKEY.SignalData.initializeDataFileUploadForm = function (metadataFormId, eleme window.location = returnUrl; },this); }, - failure: function(response){ - //TODO: Should probably do something here... - } + failure: LABKEY.Utils.getCallbackWrapper(function(json, response, opts) { + LABKEY.Utils.alert('Error', 'Unable to save run : ' + response.exception); + }, this, true) }, this); } From 913c2f093252137710d8e506427b43a9dc15094a Mon Sep 17 00:00:00 2001 From: Lum Date: Wed, 10 Sep 2025 15:12:30 -0700 Subject: [PATCH 2/7] refactor getSignalDataResourcesAction to handle multiple file paths at once --- .../resources/views/mockSignalDataWatch.html | 58 ++++----- .../signaldata/ResultUpdate/resultupdate.js | 18 ++- .../web/signaldata/UploadView/UploadLog.js | 59 ++++----- .../UploadView/uploadResultFiles.js | 2 +- .../signaldata/SignalDataController.java | 121 ++++++++---------- 5 files changed, 120 insertions(+), 138 deletions(-) diff --git a/signalData/resources/views/mockSignalDataWatch.html b/signalData/resources/views/mockSignalDataWatch.html index 0d3f9681e..819a2cbd4 100644 --- a/signalData/resources/views/mockSignalDataWatch.html +++ b/signalData/resources/views/mockSignalDataWatch.html @@ -61,23 +61,23 @@ } }); - var dataRows = [], - dataInputs = []; + var dataRows = [], dataInputs = []; Ext4.each(fileSet, function(file) { + var fileName = file['FileName']; dataRows.push({ - Name: file.text, - DataFile: SIGNAL_DATA_FILE_ROOT + file.text, + Name: fileName, + DataFile: SIGNAL_DATA_FILE_ROOT + fileName, TestType: 'SMP' }); dataInputs.push({ - name: file.text, - dataFileURL: file.dataFileURL + name: fileName, + dataFileURL: file['DataFileUrl'] }); }); - run.dataRows = dataRows; run.dataInputs = dataInputs; + return run; } @@ -197,33 +197,25 @@ { if (Ext4.isFunction(callback)) { - var received = 0; - var newFiles = []; + var paths = []; + var fileNames = []; + files.forEach(function (file) { + paths.push(decodeURIComponent(file.id)); + fileNames.push(file.text); + }, this); - function done(file, results) - { - Ext4.each(files, function(f) { - if (f.text === file.text) { - f['dataFileURL'] = results['DataFileUrl']; - newFiles.push(f); - } - }); - received++; - - if (received == files.length) { - callback.call(scope || this, newFiles); - } - } - - Ext4.each(files, function(file) { - LABKEY.Ajax.request({ - url: LABKEY.ActionURL.buildURL('signaldata', 'getSignalDataResource.api'), - method: 'POST', - params: { path: decodeURIComponent(file.id), test: true }, - success: function(response) { - done(file, Ext4.decode(response.responseText)); - } - }); + LABKEY.Ajax.request({ + url: LABKEY.ActionURL.buildURL('SignalData', 'getSignalDataResource.api'), + method: 'POST', + jsonData: { + paths : paths, + files : fileNames + }, + success: function (response) { + let result = Ext4.decode(response.responseText); + callback.call(scope || this, result.files); + }, + scope: this }); } } diff --git a/signalData/resources/web/signaldata/ResultUpdate/resultupdate.js b/signalData/resources/web/signaldata/ResultUpdate/resultupdate.js index 2c4f95710..e1e1a9dc6 100644 --- a/signalData/resources/web/signaldata/ResultUpdate/resultupdate.js +++ b/signalData/resources/web/signaldata/ResultUpdate/resultupdate.js @@ -34,8 +34,8 @@ var getResultData = function(assay) { requiredVersion: 13.2, filterArray: [LABKEY.Filter.create('RowId', LABKEY.ActionURL.getParameter('rowId'))], success: function(results){ - if (results.length > 0) { - init(assay, results.getRow(0)); + if (results.rowCount > 0) { + init(assay, results.rows[0]); } else { // Use an ExtJS alert instead of a raw browser alert to avoid alarming the crawler @@ -64,11 +64,17 @@ var init = function(assay, row){ LABKEY.Ajax.request({ url: LABKEY.ActionURL.buildURL('SignalData', 'getSignalDataResource.api'), method: 'POST', - params: {path: decodeURIComponent(file.internalId), test: true}, + params: { + paths: [decodeURIComponent(file.internalId)], + files: [file.name] + }, success: function (response) { - var fileResource = Ext4.decode(response.responseText); - var updatedRow = setRunFields(form, fileResource); - updateRunResult(updatedRow, fileResource); + var result = Ext4.decode(response.responseText); + Ext4.each(result.files, function(file) { + + var updatedRow = setRunFields(form, file); + updateRunResult(updatedRow, file); + }, this); }, scope: this }); diff --git a/signalData/resources/web/signaldata/UploadView/UploadLog.js b/signalData/resources/web/signaldata/UploadView/UploadLog.js index de4d1a7c6..81e77cfa5 100644 --- a/signalData/resources/web/signaldata/UploadView/UploadLog.js +++ b/signalData/resources/web/signaldata/UploadView/UploadLog.js @@ -321,41 +321,42 @@ Ext4.define('LABKEY.SignalData.UploadLog', { resolveDataFileURL: function (files, callback, scope, runProperties) { if (Ext4.isFunction(callback)) { - var received = 0; - var newFiles = []; - var me = this; + var paths = []; + var fileNames = []; + files.forEach(function (file) { + paths.push(decodeURIComponent(file.id)); + fileNames.push(file.text); + }, this); - function done(file, results) { + LABKEY.Ajax.request({ + url: LABKEY.ActionURL.buildURL('SignalData', 'getSignalDataResource.api'), + method: 'POST', + jsonData: { + paths : paths, + files : fileNames + }, + success: function (response) { + let result = Ext4.decode(response.responseText); + let store = this.getStore(); - var store = me.getStore(); - var idx = store.find(me.DATA_FILE, file.text); - var process = store.getAt(idx); + Ext4.each(result.files, function(file) { - //Set upload time - process.set(me.FILENAME, results[me.DATA_FILE]); - process.set(me.FILE_URL, results['DataFileUrl']); - process.set('file', file); - newFiles.push(file); - received++; + var idx = store.find(this.DATA_FILE, file["FileName"]); + var rec = store.getAt(idx); - if (received == files.length) { - callback.call(scope || me, newFiles, runProperties); - } - } + if (rec) { + //Set upload time in the store record + rec.set(me.FILENAME, file[this.DATA_FILE]); + rec.set(me.FILE_URL, file['DataFileUrl']); + rec.set('file', true); + } + }, this); - //TODO: This should be refactored to use a single ajax call for the array - files.forEach(function (file) { - LABKEY.Ajax.request({ - url: LABKEY.ActionURL.buildURL('SignalData', 'getSignalDataResource.api'), - method: 'POST', - params: {path: decodeURIComponent(file.id), test: true}, - success: function (response) { - done(file, Ext4.decode(response.responseText)); - }, - scope: this - }); - }, this); + callback.call(scope || me, runProperties); + }, + scope: this + }); } }, diff --git a/signalData/resources/web/signaldata/UploadView/uploadResultFiles.js b/signalData/resources/web/signaldata/UploadView/uploadResultFiles.js index 37445cd71..dc0ca0f48 100644 --- a/signalData/resources/web/signaldata/UploadView/uploadResultFiles.js +++ b/signalData/resources/web/signaldata/UploadView/uploadResultFiles.js @@ -372,7 +372,7 @@ LABKEY.SignalData.initializeDataFileUploadForm = function (metadataFormId, eleme }, this); } - var generateAndSaveRun = function(files, fieldValues) { + var generateAndSaveRun = function(fieldValues) { var dataRows = []; var dataInputs = []; diff --git a/signalData/src/org/labkey/signaldata/SignalDataController.java b/signalData/src/org/labkey/signaldata/SignalDataController.java index b065d3301..f3afb6245 100644 --- a/signalData/src/org/labkey/signaldata/SignalDataController.java +++ b/signalData/src/org/labkey/signaldata/SignalDataController.java @@ -22,7 +22,6 @@ import org.labkey.api.action.ReadOnlyApiAction; import org.labkey.api.action.SpringActionController; import org.labkey.api.data.Container; -import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.TableInfo; import org.labkey.api.exp.api.ExpData; import org.labkey.api.exp.api.ExperimentService; @@ -32,10 +31,11 @@ import org.labkey.api.pipeline.PipeRoot; import org.labkey.api.pipeline.PipelineService; import org.labkey.api.query.QueryUpdateService; +import org.labkey.api.query.ValidationException; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.permissions.ReadPermission; +import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.util.FileUtil; -import org.labkey.api.util.UnexpectedException; import org.labkey.api.webdav.WebdavResource; import org.labkey.api.webdav.WebdavService; import org.labkey.signaldata.assay.SignalDataAssayDataHandler; @@ -43,9 +43,8 @@ import org.springframework.validation.BindException; import java.io.File; -import java.net.MalformedURLException; import java.net.URI; -import java.sql.SQLException; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -101,115 +100,99 @@ public ApiResponse execute(Object form, BindException errors) throws Exception public static class SignalDataResourceForm { - public String _path; - public boolean _test = false; + private List _paths; + private List _files; - public String getPath() + public List getFiles() { - return _path; + return _files; } - public void setPath(String path) + public void setFiles(List files) { - _path = path; + _files = files; } - public boolean isTest() + public List getPaths() { - return _test; + return _paths; } - public void setTest(boolean test) + public void setPaths(List paths) { - _test = test; + _paths = paths; } } - - @RequiresPermission(ReadPermission.class) + @RequiresPermission(UpdatePermission.class) public static class getSignalDataResourceAction extends MutatingApiAction { @Override - public ApiResponse execute(SignalDataResourceForm form, BindException errors) + public ApiResponse execute(SignalDataResourceForm form, BindException errors) throws Exception { - String path = form.getPath(); - WebdavResource resource = WebdavService.get().lookup(path); - Map props = new HashMap<>(); + List> results = new ArrayList<>(); Container c = getContainer(); + FileContentService svc = FileContentService.get(); + TableInfo ti = ExpSchema.TableType.Data.createTable(new ExpSchema(getUser(), c), ExpSchema.TableType.Data.toString(), null); + QueryUpdateService qus = ti.getUpdateService(); + int maxUrlSize = ExperimentService.get().getTinfoData().getColumn("DataFileURL").getScale(); + int idx = 0; - if (null != resource) + for (String path : form.getPaths()) { - FileContentService svc = FileContentService.get(); - ExpData data = svc.getDataObject(resource, c); + WebdavResource resource = WebdavService.get().lookup(path); + String fileName = form.getFiles().get(idx++); - if (form.isTest() && data == null) + if (null != resource) { - // Generate the data to help the test, replicating what DavController would do for us - File file = resource.getFile(); - if (null != file) + ExpData data = svc.getDataObject(resource, c); + if (data == null) { - data = ExperimentService.get().createData(c, UPLOADED_FILE); - data.setName(file.getName()); - data.setDataFileURI(file.toURI()); - - int scale = ExperimentService.get().getTinfoData().getColumn("DataFileURL").getScale(); - String dataFileURL = data.getDataFileUrl(); - if (dataFileURL == null || dataFileURL.length() <= scale) + // create the ExpData object if it doesn't already exist + File file = resource.getFile(); + if (null != file) { - data.save(getUser()); - } - else - { - // If the path is too long to store, bail out without creating an exp.data row + data = ExperimentService.get().createData(c, UPLOADED_FILE); + data.setName(file.getName()); + data.setDataFileURI(file.toURI()); + + String dataFileURL = data.getDataFileUrl(); + if (dataFileURL == null || dataFileURL.length() <= maxUrlSize) + { + data.save(getUser()); + } + else + { + throw new ValidationException(String.format("The data file URL is too long to store in the database (max %d).", maxUrlSize)); + } } } - } - if (null != data) - { - TableInfo ti = ExpSchema.TableType.Data.createTable(new ExpSchema(getUser(), c), ExpSchema.TableType.Data.toString(), null); - QueryUpdateService qus = ti.getUpdateService(); - - try + if (null != data) { File canonicalFile = FileUtil.getAbsoluteCaseSensitiveFile(resource.getFile()); String url = canonicalFile.toURI().toURL().toString(); - Map keys = Collections.singletonMap(ExpDataTable.Column.DataFileUrl.name(), url); - List> rows = qus.getRows(getUser(), c, Collections.singletonList(keys)); + List> rows = qus.getRows(getUser(), c, Collections.singletonList(Map.of(ExpDataTable.Column.DataFileUrl.name(), url))); if (rows.size() == 1) { + Map props = new HashMap<>(); + props.put("FilePath", path); + props.put("FileName", fileName); + results.add(props); for (Map.Entry entry : rows.get(0).entrySet()) { Object value = entry.getValue(); - if (null != value) - { props.put(entry.getKey(), String.valueOf(value)); - } } } - } - catch (MalformedURLException e) - { - throw UnexpectedException.wrap(e); - } - catch (SQLException e) - { - throw new RuntimeSQLException(e); - } - catch (RuntimeException re) - { - throw re; - } - catch (Exception e) - { - throw new RuntimeException(e); + else + throw new RuntimeException(String.format("Unexpected number of rows returned for DataFileUrl '%s': %d", url, rows.size())); } } } - - return new ApiSimpleResponse(props); + return new ApiSimpleResponse(Map.of("files", results)); } } } \ No newline at end of file From 090f2e10dbb7149b6850e0f24d69803434048e24 Mon Sep 17 00:00:00 2001 From: Lum Date: Thu, 11 Sep 2025 11:17:59 -0700 Subject: [PATCH 3/7] update comment --- signalData/resources/web/signaldata/UploadView/UploadLog.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/signalData/resources/web/signaldata/UploadView/UploadLog.js b/signalData/resources/web/signaldata/UploadView/UploadLog.js index 81e77cfa5..895c7b36b 100644 --- a/signalData/resources/web/signaldata/UploadView/UploadLog.js +++ b/signalData/resources/web/signaldata/UploadView/UploadLog.js @@ -270,7 +270,7 @@ Ext4.define('LABKEY.SignalData.UploadLog', { /** * Returns the list of data files in the target directory. Uploaded run data files are moved from - * a temporary location to the target directory when the run is created. + * a temporary location to the target directory before the run is created. */ getTargetDirResources : function (targetDirectory, callback, callbackScope) { var fileUri = this.fileSystem.concatPaths(this.fileSystem.getAbsoluteURL(), targetDirectory); From c18b8b8e57fd2f03f8b06f359c6ccd171608925f Mon Sep 17 00:00:00 2001 From: Lum Date: Thu, 11 Sep 2025 16:24:43 -0700 Subject: [PATCH 4/7] code review feedback --- .../resources/views/mockSignalDataWatch.html | 7 +- .../signaldata/ResultUpdate/resultupdate.js | 8 +- .../signaldata/SignalDataController.java | 87 ++++++++++--------- 3 files changed, 52 insertions(+), 50 deletions(-) diff --git a/signalData/resources/views/mockSignalDataWatch.html b/signalData/resources/views/mockSignalDataWatch.html index 819a2cbd4..9cb8e711f 100644 --- a/signalData/resources/views/mockSignalDataWatch.html +++ b/signalData/resources/views/mockSignalDataWatch.html @@ -211,10 +211,9 @@ paths : paths, files : fileNames }, - success: function (response) { - let result = Ext4.decode(response.responseText); - callback.call(scope || this, result.files); - }, + success: LABKEY.Utils.getCallbackWrapper(function(response) { + callback.call(scope || this, response.files); + }, this, true), scope: this }); } diff --git a/signalData/resources/web/signaldata/ResultUpdate/resultupdate.js b/signalData/resources/web/signaldata/ResultUpdate/resultupdate.js index e1e1a9dc6..1d6eb50e8 100644 --- a/signalData/resources/web/signaldata/ResultUpdate/resultupdate.js +++ b/signalData/resources/web/signaldata/ResultUpdate/resultupdate.js @@ -68,14 +68,12 @@ var init = function(assay, row){ paths: [decodeURIComponent(file.internalId)], files: [file.name] }, - success: function (response) { - var result = Ext4.decode(response.responseText); - Ext4.each(result.files, function(file) { - + success: LABKEY.Utils.getCallbackWrapper(function(response) { + Ext4.each(response.files, function(file) { var updatedRow = setRunFields(form, file); updateRunResult(updatedRow, file); }, this); - }, + }, this, true), scope: this }); } diff --git a/signalData/src/org/labkey/signaldata/SignalDataController.java b/signalData/src/org/labkey/signaldata/SignalDataController.java index f3afb6245..5fbc76a2d 100644 --- a/signalData/src/org/labkey/signaldata/SignalDataController.java +++ b/signalData/src/org/labkey/signaldata/SignalDataController.java @@ -22,6 +22,7 @@ import org.labkey.api.action.ReadOnlyApiAction; import org.labkey.api.action.SpringActionController; import org.labkey.api.data.Container; +import org.labkey.api.data.DbScope; import org.labkey.api.data.TableInfo; import org.labkey.api.exp.api.ExpData; import org.labkey.api.exp.api.ExperimentService; @@ -133,64 +134,68 @@ public ApiResponse execute(SignalDataResourceForm form, BindException errors) th List> results = new ArrayList<>(); Container c = getContainer(); FileContentService svc = FileContentService.get(); - TableInfo ti = ExpSchema.TableType.Data.createTable(new ExpSchema(getUser(), c), ExpSchema.TableType.Data.toString(), null); + TableInfo ti = new ExpSchema(getUser(), getContainer()).getDatasTable(); QueryUpdateService qus = ti.getUpdateService(); - int maxUrlSize = ExperimentService.get().getTinfoData().getColumn("DataFileURL").getScale(); + int maxUrlSize = ExperimentService.get().getTinfoData().getColumn(ExpDataTable.Column.DataFileUrl.name()).getScale(); int idx = 0; - for (String path : form.getPaths()) + try (DbScope.Transaction transaction = DbScope.getLabKeyScope().ensureTransaction()) { - WebdavResource resource = WebdavService.get().lookup(path); - String fileName = form.getFiles().get(idx++); - - if (null != resource) + for (String path : form.getPaths()) { - ExpData data = svc.getDataObject(resource, c); - if (data == null) + WebdavResource resource = WebdavService.get().lookup(path); + String fileName = form.getFiles().get(idx++); + + if (null != resource) { - // create the ExpData object if it doesn't already exist - File file = resource.getFile(); - if (null != file) + ExpData data = svc.getDataObject(resource, c); + if (data == null) { - data = ExperimentService.get().createData(c, UPLOADED_FILE); - data.setName(file.getName()); - data.setDataFileURI(file.toURI()); - - String dataFileURL = data.getDataFileUrl(); - if (dataFileURL == null || dataFileURL.length() <= maxUrlSize) - { - data.save(getUser()); - } - else + // create the ExpData object if it doesn't already exist + File file = resource.getFile(); + if (null != file) { - throw new ValidationException(String.format("The data file URL is too long to store in the database (max %d).", maxUrlSize)); + data = ExperimentService.get().createData(c, UPLOADED_FILE); + data.setName(file.getName()); + data.setDataFileURI(file.toURI()); + + String dataFileURL = data.getDataFileUrl(); + if (dataFileURL == null || dataFileURL.length() <= maxUrlSize) + { + data.save(getUser()); + } + else + { + throw new ValidationException(String.format("The data file URL is too long to store in the database (max %d).", maxUrlSize)); + } } } - } - - if (null != data) - { - File canonicalFile = FileUtil.getAbsoluteCaseSensitiveFile(resource.getFile()); - String url = canonicalFile.toURI().toURL().toString(); - List> rows = qus.getRows(getUser(), c, Collections.singletonList(Map.of(ExpDataTable.Column.DataFileUrl.name(), url))); - if (rows.size() == 1) + if (null != data) { - Map props = new HashMap<>(); - props.put("FilePath", path); - props.put("FileName", fileName); - results.add(props); - for (Map.Entry entry : rows.get(0).entrySet()) + File canonicalFile = FileUtil.getAbsoluteCaseSensitiveFile(resource.getFile()); + String url = canonicalFile.toURI().toURL().toString(); + List> rows = qus.getRows(getUser(), c, Collections.singletonList(Map.of(ExpDataTable.Column.DataFileUrl.name(), url))); + + if (rows.size() == 1) { - Object value = entry.getValue(); - if (null != value) - props.put(entry.getKey(), String.valueOf(value)); + Map props = new HashMap<>(); + props.put("FilePath", path); + props.put("FileName", fileName); + results.add(props); + for (Map.Entry entry : rows.get(0).entrySet()) + { + Object value = entry.getValue(); + if (null != value) + props.put(entry.getKey(), String.valueOf(value)); + } } + else + throw new RuntimeException(String.format("Unexpected number of rows returned for DataFileUrl '%s': %d", url, rows.size())); } - else - throw new RuntimeException(String.format("Unexpected number of rows returned for DataFileUrl '%s': %d", url, rows.size())); } } + transaction.commit(); } return new ApiSimpleResponse(Map.of("files", results)); } From 03cf08ae07748330e942cbd4b1611b4e62f7b31a Mon Sep 17 00:00:00 2001 From: Lum Date: Tue, 16 Sep 2025 09:54:15 -0700 Subject: [PATCH 5/7] code review feedback --- signalData/resources/views/mockSignalDataWatch.html | 2 +- .../resources/web/signaldata/ResultUpdate/resultupdate.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/signalData/resources/views/mockSignalDataWatch.html b/signalData/resources/views/mockSignalDataWatch.html index 9cb8e711f..efa0c1d15 100644 --- a/signalData/resources/views/mockSignalDataWatch.html +++ b/signalData/resources/views/mockSignalDataWatch.html @@ -213,7 +213,7 @@ }, success: LABKEY.Utils.getCallbackWrapper(function(response) { callback.call(scope || this, response.files); - }, this, true), + }, this), scope: this }); } diff --git a/signalData/resources/web/signaldata/ResultUpdate/resultupdate.js b/signalData/resources/web/signaldata/ResultUpdate/resultupdate.js index 1d6eb50e8..6cd9fb9f0 100644 --- a/signalData/resources/web/signaldata/ResultUpdate/resultupdate.js +++ b/signalData/resources/web/signaldata/ResultUpdate/resultupdate.js @@ -73,7 +73,7 @@ var init = function(assay, row){ var updatedRow = setRunFields(form, file); updateRunResult(updatedRow, file); }, this); - }, this, true), + }, this), scope: this }); } From bca56c1f6d066f176dd6e1be11f03996a75eced3 Mon Sep 17 00:00:00 2001 From: labkey-danield Date: Fri, 19 Sep 2025 13:49:49 -0700 Subject: [PATCH 6/7] Add error tests for Signal Data tests. Cover Issue 53786 --- .../signaldata/SignalDataUploadPage.java | 11 ++ .../tests/signaldata/SignalDataRawTest.java | 153 +++++++++++++++--- 2 files changed, 146 insertions(+), 18 deletions(-) diff --git a/signalData/test/src/org/labkey/test/pages/signaldata/SignalDataUploadPage.java b/signalData/test/src/org/labkey/test/pages/signaldata/SignalDataUploadPage.java index fe1e6a427..ad9a313d1 100644 --- a/signalData/test/src/org/labkey/test/pages/signaldata/SignalDataUploadPage.java +++ b/signalData/test/src/org/labkey/test/pages/signaldata/SignalDataUploadPage.java @@ -18,9 +18,11 @@ import org.labkey.test.BaseWebDriverTest; import org.labkey.test.Locator; import org.labkey.test.WebDriverWrapper; +import org.labkey.test.components.bootstrap.ModalDialog; import org.labkey.test.components.ext4.Window; import org.labkey.test.util.Ext4Helper; import org.openqa.selenium.Keys; +import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.ui.ExpectedConditions; @@ -111,6 +113,15 @@ public void saveRun() _test.clickAndWait(Locators.saveButton); } + public Window saveRunExpectingError(WebDriver webDriver) + { + WebElement saveButton = Locators.saveButton.findElement(_test.getDriver()); + WebDriverWrapper.waitFor(() -> !saveButton.getAttribute("class").contains("disabled"), "Unable to save, button is disabled", 1000); + _test.click(Locators.saveButton); + + return Window(_test.getDriver()).withTitle("Error").waitFor(); + } + private static class Locators { static final Locator.XPathLocator runIdentifier = Locator.input("RunIdentifier").notHidden(); diff --git a/signalData/test/src/org/labkey/test/tests/signaldata/SignalDataRawTest.java b/signalData/test/src/org/labkey/test/tests/signaldata/SignalDataRawTest.java index fd8f92cf3..29e346467 100644 --- a/signalData/test/src/org/labkey/test/tests/signaldata/SignalDataRawTest.java +++ b/signalData/test/src/org/labkey/test/tests/signaldata/SignalDataRawTest.java @@ -23,16 +23,22 @@ import org.labkey.test.BaseWebDriverTest; import org.labkey.test.Locator; import org.labkey.test.categories.Daily; +import org.labkey.test.pages.ReactAssayDesignerPage; import org.labkey.test.pages.signaldata.SignalDataAssayBeginPage; import org.labkey.test.pages.signaldata.SignalDataRunViewerPage; import org.labkey.test.pages.signaldata.SignalDataUploadPage; +import org.labkey.test.params.FieldDefinition; import org.labkey.test.util.DataRegionTable; import org.labkey.test.util.Ext4Helper; +import org.labkey.test.components.ext4.Window; import org.labkey.test.util.PostgresOnlyTest; +import org.labkey.test.util.data.TestDataUtils; import org.labkey.test.util.signaldata.SignalDataInitializer; import org.openqa.selenium.WebElement; import java.io.File; +import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; @@ -84,14 +90,14 @@ public static void doSetup() throws Exception @Before public void preTest() { - // Reset to the original run/data file set created in the initialize - navigateToAssayLandingPage().resetUploadedData(DEFAULT_RUN); + // Reset to the original run/data file set created in the initializing + navigateToAssayLandingPage(SignalDataInitializer.RAW_SignalData_ASSAY).resetUploadedData(DEFAULT_RUN); } @Test public void testRunsSearch() { - SignalDataAssayBeginPage beginPage = navigateToAssayLandingPage(); + SignalDataAssayBeginPage beginPage = navigateToAssayLandingPage(SignalDataInitializer.RAW_SignalData_ASSAY); //Test search by file beginPage.setSearchBox(RESULT_FILENAME_1); @@ -107,7 +113,7 @@ public void testRunsSearch() public void testRunViewer() { // TODO: Test the run viewer. See FormulationsTest.qualityControlHPLCData for guidance. - SignalDataAssayBeginPage beginPage = navigateToAssayLandingPage(); + SignalDataAssayBeginPage beginPage = navigateToAssayLandingPage(SignalDataInitializer.RAW_SignalData_ASSAY); beginPage.selectData(RESULT_FILENAME_1, DEFAULT_RUN); beginPage.selectData(RESULT_FILENAME_2, DEFAULT_RUN); @@ -132,7 +138,8 @@ public void testFileImport() File metadataFile = getFile("RunsMetadata/datafiles.tsv"); Map> expectedData = Map.of("StringValue", List.of("StringOne", "StringTwo", "StringThree"), "IntegerValue", List.of("1", "2", "3")); - SignalDataAssayBeginPage beginPage = importRun("importTest1", + SignalDataAssayBeginPage beginPage = importRun(SignalDataInitializer.RAW_SignalData_ASSAY, + "importTest1", metadataFile, List.of(getFile(String.join("/", ASSAY_DATA_LOC, "BLANK235.TXT"))), List.of( @@ -150,10 +157,11 @@ public void testFileImport() uploadPage.setRunIDField("cleared run"); assertElementPresent(Ext4Helper.Locators.getGridRow()); //Check grid has elements uploadPage.clearRun(); - navigateToAssayLandingPage(); //Should not cause unload warning + navigateToAssayLandingPage(SignalDataInitializer.RAW_SignalData_ASSAY); //Should not cause unload warning // test upload of metadata file with full data file paths - importRun("importTest2", + importRun(SignalDataInitializer.RAW_SignalData_ASSAY, + "importTest2", getFile("RunsMetadata/datafiles2.tsv"), Collections.emptyList(), List.of( @@ -164,7 +172,8 @@ public void testFileImport() ), Collections.EMPTY_MAP, 4); // test import of files with a subset of the metadata files - importRun("importTest3", + importRun(SignalDataInitializer.RAW_SignalData_ASSAY, + "importTest3", getFile("RunsMetadata/datafiles.tsv"), List.of(getFile(String.join("/", ASSAY_DATA_LOC, "BLANK235.TXT"))), List.of( @@ -173,7 +182,8 @@ public void testFileImport() ), expectedData, 3); - importRun("importTest4", + importRun(SignalDataInitializer.RAW_SignalData_ASSAY, + "importTest4", getFile("RunsMetadata/datafiles2.tsv"), Collections.emptyList(), List.of( @@ -182,16 +192,123 @@ public void testFileImport() ), Collections.EMPTY_MAP, 4); } + @Test + public void testErrorConditions() throws IOException + { + + String errorAssay = "Test Errors Conditions"; + + goToProjectHome(); + + log("Defining Error Assay"); + goToManageAssays(); + + // We don't handle tricky characters well. + // Uncomment these lines once Issue 53965 is fixed. +// String strFieldName = TestDataGenerator.randomFieldName("Str"); +// String intFieldName = TestDataGenerator.randomFieldName("Int"); + String strFieldName = "Str"; + String intFieldName = "Int"; + + FieldDefinition strField = new FieldDefinition(strFieldName, FieldDefinition.ColumnType.String) + .setRequired(true); + + FieldDefinition intField = new FieldDefinition(intFieldName, FieldDefinition.ColumnType.Integer) + .setValidators(List.of(new FieldDefinition.RangeValidator("Large", "Must be greater than 5.", + "Value must be greater than 5.", + FieldDefinition.RangeType.GT, "5"))); + + ReactAssayDesignerPage assayDesigner = _assayHelper.createAssayDesign("Signal Data", errorAssay); + assayDesigner.setDescription("Testing error condition are handled correctly."); + assayDesigner.setEditableRuns(true); + assayDesigner.setEditableResults(true); + assayDesigner.goToResultsFields() + .addField(strField) + .addField(intField); + assayDesigner.clickFinish(); + + List> fileData = new ArrayList<>(); + fileData.add(List.of("Name", "DataFile", strField.getName(), intField.getName())); + fileData.add(List.of("Missing Required", RESULT_FILENAME_1, "", "123")); + fileData.add(List.of("Has All", RESULT_FILENAME_2, "DEF", "456")); + File metadataFile = TestDataUtils.writeRowsToTsv("Missing Require Result Field.tsv", fileData); + + uploadWithErrorAction(errorAssay, + metadataFile, + "Missing Required Run", + String.format("Missing value for required property: %s", strField.getName())); + + fileData = new ArrayList<>(); + fileData.add(List.of("Name", "DataFile", strField.getName(), intField.getName())); + fileData.add(List.of("Valid Entry", RESULT_FILENAME_1, "ABC", "123")); + fileData.add(List.of("Incompatible Data Type", RESULT_FILENAME_2, "DEF", "GHI")); + metadataFile = TestDataUtils.writeRowsToTsv("Invalid Data Type.tsv", fileData); + + uploadWithErrorAction(errorAssay, + metadataFile, + "Invalid Data Type", + String.format("Int: Value 'GHI' for field '%s' is invalid. Value must be greater than 5.", intField.getName())); + + fileData = new ArrayList<>(); + fileData.add(List.of("Name", "DataFile", strField.getName(), intField.getName())); + fileData.add(List.of("Valid Entry Again", RESULT_FILENAME_1, "ABC", "123")); + fileData.add(List.of("Range Validation Error", RESULT_FILENAME_2, "DEF", "2")); + metadataFile = TestDataUtils.writeRowsToTsv("Range Validation Error.tsv", fileData); + + uploadWithErrorAction(errorAssay, + metadataFile, + "Invalid Data Type", + String.format("Int: Value '2' for field '%s' is invalid. Value must be greater than 5.", intField.getName())); + + } + + private void uploadWithErrorAction(String assayName, + File metadataFile, + String runId, String expectedMsg) + { + + // If there is ever a desire to expand the error testing to include errors in the data files, then this list of + // data files should be identified in the test and passed in as a parameter. + List dataFiles = List.of( + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_1)), + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_2))); + + SignalDataUploadPage uploadPage = navigateToAssayLandingPage(assayName).navigateToImportPage(); + + log(String.format("Uploading metadata file: %s", metadataFile.getName())); + uploadPage.uploadMetadataFile(metadataFile); + + log("Uploading data files."); + int uploadCount = dataFiles.size(); + uploadPage.uploadFile(dataFiles); + uploadPage.waitForProgressBars(uploadCount); + + uploadPage.setRunIDField(runId); + Window dialog = uploadPage.saveRunExpectingError(getDriver()); + + String actualMsg = dialog.getBody(); + + if (checker().withScreenshot() + .verifyTrue(String.format("Error dialog message '%s' does not contain expected message: %s", actualMsg, expectedMsg), + actualMsg.contains(expectedMsg))) + { + dialog.clickButton("OK", true); + uploadPage.clearRun(); + } + + } + private SignalDataAssayBeginPage importRun( - String runName, - File metadataFile, - List unspecifiedDataFiles, - List dataFiles, - Map> expectedData, - int expectedResultRows + String assayName, + String runName, + File metadataFile, + List unspecifiedDataFiles, + List dataFiles, + Map> expectedData, + int expectedResultRows ) { - SignalDataAssayBeginPage beginPage = navigateToAssayLandingPage(); + SignalDataAssayBeginPage beginPage = navigateToAssayLandingPage(assayName); SignalDataUploadPage uploadPage = beginPage.navigateToImportPage(); log("Uploading metadata file"); @@ -236,11 +353,11 @@ private File getFile(String relativePath) return file; } - private SignalDataAssayBeginPage navigateToAssayLandingPage() + private SignalDataAssayBeginPage navigateToAssayLandingPage(String assayName) { //Navigate to Landing Page goToProjectHome(); - clickAndWait(Locator.linkWithText(SignalDataInitializer.RAW_SignalData_ASSAY)); + clickAndWait(Locator.linkWithText(assayName)); SignalDataAssayBeginPage page = new SignalDataAssayBeginPage(this); page.waitForPageLoad(); return page; From e36c4f031c86a79362261cc39b0a73ca6e54a87d Mon Sep 17 00:00:00 2001 From: labkey-danield Date: Fri, 19 Sep 2025 13:53:23 -0700 Subject: [PATCH 7/7] Add some comments. --- .../org/labkey/test/tests/signaldata/SignalDataRawTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/signalData/test/src/org/labkey/test/tests/signaldata/SignalDataRawTest.java b/signalData/test/src/org/labkey/test/tests/signaldata/SignalDataRawTest.java index 29e346467..8b6d9ec9f 100644 --- a/signalData/test/src/org/labkey/test/tests/signaldata/SignalDataRawTest.java +++ b/signalData/test/src/org/labkey/test/tests/signaldata/SignalDataRawTest.java @@ -233,6 +233,7 @@ public void testErrorConditions() throws IOException fileData.add(List.of("Has All", RESULT_FILENAME_2, "DEF", "456")); File metadataFile = TestDataUtils.writeRowsToTsv("Missing Require Result Field.tsv", fileData); + log("Validate error condition of a required field is missing."); uploadWithErrorAction(errorAssay, metadataFile, "Missing Required Run", @@ -244,6 +245,7 @@ public void testErrorConditions() throws IOException fileData.add(List.of("Incompatible Data Type", RESULT_FILENAME_2, "DEF", "GHI")); metadataFile = TestDataUtils.writeRowsToTsv("Invalid Data Type.tsv", fileData); + log("Validate error condition when there is an invalid value (string for an int)."); uploadWithErrorAction(errorAssay, metadataFile, "Invalid Data Type", @@ -255,9 +257,10 @@ public void testErrorConditions() throws IOException fileData.add(List.of("Range Validation Error", RESULT_FILENAME_2, "DEF", "2")); metadataFile = TestDataUtils.writeRowsToTsv("Range Validation Error.tsv", fileData); + log("Validate error condition when field value fails range validation."); uploadWithErrorAction(errorAssay, metadataFile, - "Invalid Data Type", + "Invalid Range", String.format("Int: Value '2' for field '%s' is invalid. Value must be greater than 5.", intField.getName())); }