Issue 33828: NPE when importing a new Skyline document into QC folder

issues
Status:closed
Assigned To:Guest
Type:Defect
Area:TargetedMS
Priority:2
Milestone:18.1
Opened:2018-04-04 10:50 by jeckels
Changed:2018-04-11 10:22 by jeckels
Resolved:2018-04-09 12:05 by Dave Bradlee
Resolution:Fixed
Support Ticket: 
Triage:Approved and Code Reviewed
Closed:2018-04-11 10:22 by jeckels
2018-04-04 10:50 jeckels
Title»NPE when importing a new Skyline document into QC folder
Assigned ToGuest»martyp
Notify»Dave Bradlee
Type»Defect
Area»TargetedMS
Priority»2
Milestone»18.1
We have a user who is hitting a NPE when trying to import a new file into an existing QC folder. It looks like there's a discrepancy between what's being tracked in the targetedms schema in terms of files, and the references in the exp.data table.

I think that Dave may have some insights here, as there may have been some subtle changes to how we create file URIs in 18.1.

I'd like to either get them a workaround (by manually deleting some previously imported data, for example) or give them an 18.1 patch.

java.lang.NullPointerException
    at org.labkey.targetedms.TargetedMSManager.deleteRunForFile(TargetedMSManager.java:1168)
    at org.labkey.targetedms.TargetedMSManager.purgeUnreferencedFiles(TargetedMSManager.java:1185)
    at org.labkey.targetedms.SkylineDocImporter.importSkylineDoc(SkylineDocImporter.java:618)
    at org.labkey.targetedms.SkylineDocImporter.importRun(SkylineDocImporter.java:173)
    at org.labkey.targetedms.pipeline.TargetedMSImportTask.run(TargetedMSImportTask.java:63)
    at org.labkey.api.pipeline.PipelineJob.runActiveTask(PipelineJob.java:754)
    at org.labkey.api.pipeline.PipelineJob.run(PipelineJob.java:985)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)

2018-04-04 11:09 Dave Bradlee
Assigned Tomartyp»Dave Bradlee
I'll take this. I'm a bit surprised I didn't already hit this, looking at the code. QC folder is the way this is getting hit.

2018-04-09 08:08 Dave Bradlee
NotifyDave Bradlee»Dave Bradlee;triage
Triage»Review Requested
Because of the change in file path format, the code wouldn't find the expected row in exp.Data. We'll now look for the legacy as well as current format. FileQueryUpdateService was already doing this, but the specific code in TargetedMS was not. I've noted to refactor in trunk to share the code that handles this.
 
 Issue_33828__TargetedMS_NPE.patch

2018-04-09 09:25 jeckels
Should we change the call to TableSelector to get a List of Maps instead of expecting there to be a single match? I'm worried about having a mix of URI styles in the table.

Not for 18.1, but should we update all of the existing rows in the DB to store in the new preferred format? I don't like having to check for multiple styles in lots of different places.

2018-04-09 10:11 Dave Bradlee
I can certainly change to get a list of maps. Probably should then make a set of runIds and delete for all runIds. As I noted in comments, the existing code (and new code) does not add container as a filter. In the normal case, this works fine since the path usually would include the folder name. I'd like to that as is, so as not to add another difference. Thanks.

For 18.2, we could run an upgrade script to change all of the URIs; shouldn't be too difficult.

2018-04-09 10:59 Dave Bradlee
Updated patch handling multiple rows in exp.data.
 
 Issue_33828_2.patch

2018-04-09 11:53 jeckels
TriageReview Requested»Approved and Code Reviewed

2018-04-09 12:05 Dave Bradlee
resolve as Fixed
Statusopen»resolved
Assigned ToDave Bradlee»jeckels
r57508 has the fix.

2018-04-11 10:22 jeckels
close
Statusresolved»closed
Assigned Tojeckels»Guest
Verified on customer server.