Skip to content

Fix skydDataId on TargetedMSRun #526

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

Merged
vagisha merged 3 commits into from
Mar 22, 2025
Merged

Fix skydDataId on TargetedMSRun #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
08dc319
Fix incorrect skydDataId
vagisha Mar 16, 2025
e45b860
Updated comments
vagisha Mar 19, 2025
6247f93
More comment updates
vagisha Mar 19, 2025
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
75 changes: 75 additions & 0 deletions panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicFileImporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.labkey.api.pipeline.PipelineService;
import org.labkey.api.query.BatchValidationException;
import org.labkey.api.security.User;
import org.labkey.api.targetedms.TargetedMSService;
import org.labkey.api.writer.VirtualFile;
import org.labkey.panoramapublic.pipeline.CopyExperimentPipelineJob;

Expand All @@ -25,6 +26,7 @@
import java.nio.file.Paths;
import java.util.List;
import java.util.Objects;
import java.util.Optional;

/**
* This importer does a file move instead of copy to the temp directory and creates a symlink in place of the original
Expand Down Expand Up @@ -90,6 +92,7 @@ public void process(@Nullable PipelineJob job, FolderImportContext ctx, VirtualF
PanoramaPublicSymlinkManager.get().moveAndSymLinkDirectory(expJob, ctx.getContainer(), sourceFiles, targetFiles, log);

alignDataFileUrls(expJob.getUser(), ctx.getContainer(), log);
updateSkydDataIds(expJob.getUser(), ctx.getContainer(), log);
}
}

Expand Down Expand Up @@ -154,6 +157,78 @@ private void alignDataFileUrls(User user, Container targetContainer, Logger log)
}
}

/**
* Fixes incorrect skydDataId reference in TargetedMSRun. This happens when the relative locations of the sky.zip
* and .skyd file are non-standard in the folder being copied.
*
* When a sky.zip file or its exploded folder are moved, post-import, so that the relative locations of sky.zip and
* its corresponding .skyd file are non-standard, two ExpData rows are created for the skyd file in the Panorama Public
* copy pipeline job.
* The first ExpData (linked to the ExpRun) is created during XAR import.
* The second ExpData (not linked to the ExpRun) is created in the SkylineDocumentParser.parseChromatograms() method.
* Normally, while running the copy pipeline job, SkylineDocumentParser.parseChromatograms() does not have to create
* a new ExpData, since an ExpData with the expected path already exists.
* Having 2 ExpDatas causes:
* 1. The skydDataId in TargetedMSRun references an ExpData not linked to the ExpRun. It refers to a file in the
* 'export' directory which gets deleted after folder import.
* 2. FK violations during cleanup (CopyExperimentFinalTask.cleanupExportDirectory()) prevents deletion of ExpData
* corresponding to the skydDataId
*
* This method finds a match and updates skydDataId in TargetedMSRun in the case where the skyDataId is not linked
* to the ExpRun.
*/
private void updateSkydDataIds(User user, Container targetContainer, Logger log) throws BatchValidationException, ImportException
{
log.info("Updating skydDataIds in folder: " + targetContainer.getPath());

boolean errors = false;
ExperimentService expService = ExperimentService.get();
List<? extends ExpRun> runs = expService.getExpRuns(targetContainer, null, null);

TargetedMSService tmsService = TargetedMSService.get();
for (ExpRun run : runs)
{
var targetedmsRun = tmsService.getRunByLsid(run.getLSID(), targetContainer);
if (targetedmsRun == null) continue;

var skydDataId = targetedmsRun.getSkydDataId();
if (skydDataId == null) continue;

var skydData = expService.getExpData(skydDataId);
if (skydData == null)
{
log.error("Could not find a row for skydDataId " + skydDataId + " for run " + targetedmsRun.getFileName());
errors = true;
}
else if (skydData.getRun() == null)
{
// skydData is not associated with an ExpRun. Find an ExpData associated with the ExpRun that matches
// the skydName and update the skydDataId on the run.
String skydName = skydData.getName();
Optional<? extends ExpData> matchingData = run.getAllDataUsedByRun().stream()
.filter(data -> Objects.equals(skydName, data.getName()))
.findFirst();

if (matchingData.isPresent())
{
ExpData data = matchingData.get();
log.debug("Updating skydDataId for run " + targetedmsRun.getFileName() + " to " + data.getRowId());
tmsService.updateSkydDataId(targetedmsRun, data, user);
}
else
{
log.error("Could not find matching skyData for run " + targetedmsRun.getFileName());
errors = true;
}
}
}

if (errors)
{
throw new ImportException("Could not update skydDataIds");
}
}

public static class Factory extends AbstractFolderImportFactory
{
@Override
Expand Down
60 changes: 59 additions & 1 deletion ...amapublic/test/src/org/labkey/test/tests/panoramapublic/PanoramaPublicMoveSkyDocTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,46 @@ public void testExperimentCopy()
moveDocument(SKY_FILE_2, targetFolder, 2);

goToProjectFolder(projectName, targetFolder);
log("Importing " + SKY_FILE_3 + " in folder " + skyDocSourceFolder);
log("Importing " + SKY_FILE_3 + " in folder " + targetFolder);
importData(SKY_FILE_3, 3);

// Test moving the sky.zip to a subdirectory in the file root, while the .skyd remains in the original location.
//
// If the sky.zip file and the .skyd file are not in their typical relative locations, the skydDataId in
// TargetedMSRun has to be updated after the folder is copied to Panorama Public. Without the update the
// copy pipeline job fails.
// Example:
// -------------------------
// BEFORE MOVE:
// SmallMolLibA.sky.zip
// SmallMolLibA
// - SmallMolLibA.sky
// - SmallMolLibA.skyd
// -------------------------
// AFTER MOVE:
// - SkylineFiles
// - SmallMolLibA.sky.zip (new location after move)
// SmallMolLibA
// - SmallMolLibA.sky
// - SmallMolLibA.skyd
// This results in:
// Two ExpData rows created for the .skyd file in folder copy on Panorama Public.
// 1. @files/export/.../Run<id>/SkylineFiles/SmallMolLibA/SmallMolLibA.skyd
// 2. @files/SmallMolLibA/SmallMolLibA.skyd
// #1 is set as the skydDataId in TargetedMSRuns, but it is not linked to the ExpRun (runId is null)
// #2 is linked to the ExpRun. This is the ExpData that skydDataId in TargetedMSRun *should* refer to.
// This situation causes two problems
// 1. ExpData cleanup in CopyExperimentFinalTask fails due to FK violation - cannot delete ExpData #1 since
// skydDataId in TargetedMSRun points to it.
// 2. Even if we were not cleaning up ExpData referring to files in the 'export' directory, chromatogram data
// would become unavailable since the "export" directory gets deleted after folder import.
//
// PanoramaPublicFileImporter.updateSkydDataId() fixes the skydDataId, if required.
String subDir = "SkylineFiles";
log("Moving " + SKY_FILE_3 + " to sub directory " + subDir + " in the Files browser");
// Move the .sky.zip file to a subdirectory
moveSkyZipToSubDir(SKY_FILE_3, subDir);

log("Creating and submitting an experiment");
String experimentTitle = "Experiment to test moving Skyline documents from other folders";
var expWebPart = createExperimentCompleteMetadata(experimentTitle);
Expand All @@ -75,6 +112,15 @@ public void testExperimentCopy()
verifyRunFilePathRoot(SKY_FILE_1, PANORAMA_PUBLIC, panoramaCopyFolder);
verifyRunFilePathRoot(SKY_FILE_2, PANORAMA_PUBLIC, panoramaCopyFolder);
verifyRunFilePathRoot(SKY_FILE_3, PANORAMA_PUBLIC, panoramaCopyFolder);

// Verify that we can view chromatograms for the Skyline document that was moved to a subdirectory.
goToDashboard();
clickAndWait(Locator.linkContainingText(SKY_FILE_3));
clickAndWait(Locator.linkContainingText("2 replicates"));
clickAndWait(Locator.linkContainingText("FU2_2017_0915_RJ_05_1ab_30").index(0));
assertTextPresent("Sample File Summary");
assertTextPresent("Total Ion Chromatogram");
assertTextNotPresent("Unable to load chromatogram");
}

private void moveDocument(String skylineDocName, String targetFolder, int jobCount)
Expand All @@ -96,6 +142,18 @@ private void moveDocument(String skylineDocName, String targetFolder, int jobCou
verifyRunFilePathRoot(skylineDocName, getProjectName(), targetFolder);
}

private void moveSkyZipToSubDir(String documentName, String subDir)
{
portalHelper.goToModule("FileContent");
waitForText(documentName);
if (!_fileBrowserHelper.fileIsPresent(subDir))
{
_fileBrowserHelper.createFolder(subDir);
}
_fileBrowserHelper.moveFile(documentName, subDir);
}


private void verifyRunFilePathRoot(String skylineDocName, String projectName, String targetFolder)
{
// Verify that exp.run filePathRoot is set to the target folder
Expand Down