Skip to content

Commit

Permalink
Panorama Public updates and fixes (#367)
Browse files Browse the repository at this point in the history 
- In the Panorama Public notification message, include text and link to add a catalog entry.
- Assign PXD accession and DOI at the end of CopyExperimentFinalTask so that we don't create these identifiers again in case the task has to be rerun due to a previous error. Remove DOI from old copy after assigning it to the new copy.
- Fix NPE in confirmPublish.jsp if data license is null.
- Fix NPE if the dataVersion of an experiment is null. This can happen if there is an error in the data copy pipeline job.
- Log names of users that were assigned Reader role in the Panorama Public copy.
- Set directory and file for audit event. Updated comment text.
- Added more file audit events.
- Fix container path in log messages.
- When a Panorama Public container is deleted (fireSymlinkCopiedExperimentDelete), look at all containers and sub-containers that could have a symlink target in the container being deleted.
  • Loading branch information
@vagisha
vagisha authored Jul 25, 2023
1 parent 1bd3975 commit 776f6a8
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 46 deletions.
2 changes: 1 addition & 1 deletion panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5673,7 +5673,7 @@ public ExperimentAnnotationsDetails(User user, ExperimentAnnotations exptAnnotat
else
{
// This is not the current version; Display a link to the current version
ExperimentAnnotations maxExpt = publishedVersions.stream().filter(e -> e.getDataVersion().equals(maxVersion)).findFirst().orElse(null);
ExperimentAnnotations maxExpt = publishedVersions.stream().filter(e -> e.getDataVersion() != null && e.getDataVersion().equals(maxVersion)).findFirst().orElse(null);
_versionsUrl = maxExpt != null ? PageFlowUtil.urlProvider(ProjectUrls.class).getBeginURL(maxExpt.getContainer()) : null;
_isCurrentVersion = false;
}
Expand Down
6 changes: 3 additions & 3 deletions panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicFileImporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,13 @@ public void process(@Nullable PipelineJob job, FolderImportContext ctx, VirtualF

if (expJob.isMoveAndSymlink())
{
log.info("Moving files to folder " + expJob.getContainer().getPath() + " and creating symlinks");
log.info("Moving files to folder " + ctx.getContainer().getPath() + " and creating symlinks");
}
else
{
log.info("Copying files to folder " + expJob.getContainer().getPath());
log.info("Copying files to folder " + ctx.getContainer().getPath());
}
PanoramaPublicSymlinkManager.get().moveAndSymLinkDirectory(expJob, sourceFiles, targetFiles, log);
PanoramaPublicSymlinkManager.get().moveAndSymLinkDirectory(expJob, ctx.getContainer(), sourceFiles, targetFiles, log);

alignDataFileUrls(expJob.getUser(), ctx.getContainer(), log);
}
Expand Down
8 changes: 8 additions & 0 deletions panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicNotification.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,14 @@ else if (addedPublication)
messageBody.append(madePublic ? " will be made public" : " will be updated");
messageBody.append(String.format(" on ProteomeXchange by a %s administrator.", journal.getName()));
}

messageBody.append(NL2).append("If you haven't already done so, you can provide a brief description and an image to showcase your work in the slideshow on PanoramaWeb.")
.append(" To add an entry for the slideshow, click the \"Panorama Public Catalog Entry\" icon located under the experiment title")
.append(" in the \"Targeted MS Experiment\" panel in your data folder at ")
.append(link(journalCopy.getShortUrl().renderShortURL())).append(".")
.append(" Or, you can click this link: ")
.append(bold(link("Add Catalog Entry", PanoramaPublicController.getAddCatalogEntryUrl(journalCopy).getURIString())));

if (doiError != null)
{
messageBody.append(NL2);
Expand Down
33 changes: 25 additions & 8 deletions panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicSymlinkManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ private void fireSymlinkCopiedExperimentDelete(ExperimentAnnotations expAnnot, C
if (nextHighestVersion != null)
{
Container versionContainer = nextHighestVersion.getContainer();
handleContainerSymlinks(versionContainer, user, (link, target, c, u) -> {
// Handle symlinks in the versionContainer as well as any of its sub-containers that target files in the container being deleted.
handleAllSymlinks(Set.of(versionContainer), user, (link, target, c, u) -> {
if (!target.startsWith(deletedContainerPath))
{
return;
Expand Down Expand Up @@ -185,7 +186,8 @@ private void fireSymlinkCopiedExperimentDelete(ExperimentAnnotations expAnnot, C
}
if (null != sourceContainer)
{
handleContainerSymlinks(sourceContainer, user, (link, target, c, u) -> {
// Handle symlinks in the sourceContainer as well as any of its sub-containers that target files in the container being deleted.
handleAllSymlinks(Set.of(sourceContainer), user, (link, target, c, u) -> {
if (!target.startsWith(deletedContainerPath))
{
return;
Expand Down Expand Up @@ -244,26 +246,37 @@ public void fireSymlinkUpdateContainer(String oldContainer, String newContainer,
}
}

private void addFileMovedAuditEvent(Path path, Path sourcePath, Container container, User user)
{
addFileAuditEvent(path, container, user, "Moved from " + sourcePath);
}

private void addLinkCreatedAuditEvent(Path link, Path target, Container linkContainer, User user)
{
addFileAuditEvent(link, linkContainer, user, "Symlink created. Target: " + target);
}

private void addLinkUpdatedAuditEvent(Path link, Path newTarget, Container linkContainer, User user)
{
addFileAuditEvent(link, linkContainer, user, "Updated symlink target to " + newTarget);
addFileAuditEvent(link, linkContainer, user, "Symlink updated. Target: " + newTarget);
}

private void addReplaceSymlinkWithTargetAuditEvent(Path link, Path target, Container linkContainer, User user)
{
addFileAuditEvent(link, linkContainer, user, "Replaced symlink with target file " + target);
addFileAuditEvent(link, linkContainer, user, "Symlink replaced with target file " + target);
}

private void addReplaceTargetWithSymlinkAuditEvent(Path link, Path target, Container container, User user)
{
addFileAuditEvent(link, container, user, "Replaced target file with symlink to " + target);
addFileAuditEvent(link, container, user, "Target file replaced with symlink. Target: " + target);
}

private void addFileAuditEvent(Path link, Container container, User user, String comment)
{
FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(
container != null ? container.getId() : null, comment);
event.setFile(link.toString());
event.setFile(link.getFileName().toString());
event.setDirectory(link.getParent().toString());
AuditLogService.get().addEvent(user, event);
}

Expand Down Expand Up @@ -300,7 +313,7 @@ public void fireSymlinkUpdate(Path oldTarget, Path newTarget, Container containe
}
}

public void moveAndSymLinkDirectory(CopyExperimentPipelineJob job, File source, File target, @Nullable Logger log) throws IOException
public void moveAndSymLinkDirectory(CopyExperimentPipelineJob job, Container targetContainer, File source, File target, @Nullable Logger log) throws IOException
{
if (null == log)
{
Expand All @@ -311,6 +324,7 @@ public void moveAndSymLinkDirectory(CopyExperimentPipelineJob job, File source,
throw new RuntimeException("Unable to access FileContentService");

File[] files = source.listFiles();
Container sourceContainer = getContainerForFilePath(source.toPath());
if (null != files)
{
for (File file : files)
Expand All @@ -324,7 +338,7 @@ public void moveAndSymLinkDirectory(CopyExperimentPipelineJob job, File source,
log.debug("Directory created: " + targetPath);
}

moveAndSymLinkDirectory(job, file, targetPath.toFile(), log);
moveAndSymLinkDirectory(job, targetContainer, file, targetPath.toFile(), log);
}
else
{
Expand Down Expand Up @@ -369,6 +383,7 @@ public void moveAndSymLinkDirectory(CopyExperimentPipelineJob job, File source,
Files.move(oldPath, targetPath, REPLACE_EXISTING);
log.debug("Moved symlink target " + oldPath + " to " + targetPath);
fcs.fireFileCreateEvent(targetPath, job.getUser(), job.getContainer());
addFileMovedAuditEvent(targetPath, oldPath, targetContainer, job.getUser());

fireSymlinkUpdate(oldPath, targetPath, job.getContainer(), // job container is the target container on Panorama Public
job.getUser(), log);
Expand All @@ -389,9 +404,11 @@ public void moveAndSymLinkDirectory(CopyExperimentPipelineJob job, File source,
Files.move(filePath, targetPath, REPLACE_EXISTING);
log.debug("Moved file " + filePath + " to " + targetPath);
fcs.fireFileCreateEvent(targetPath, job.getUser(), job.getContainer());
addFileMovedAuditEvent(targetPath, filePath, targetContainer, job.getUser());

Files.createSymbolicLink(filePath, targetPath);
log.debug("Created symlink " + filePath + " targeting " + targetPath);
addLinkCreatedAuditEvent(filePath, targetPath, sourceContainer, job.getUser());
// We don't need to update any symlinks here since the source container should not have any symlink targets.
}
}
Expand Down
76 changes: 47 additions & 29 deletions panoramapublic/src/org/labkey/panoramapublic/pipeline/CopyExperimentFinalTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
Expand Down Expand Up @@ -146,7 +147,6 @@ private void finishUp(PipelineJob job, CopyExperimentJobSupport jobSupport) thro
// Update the row in panoramapublic.ExperimentAnnotations - set the shortURL and version
ExperimentAnnotations targetExperiment = updateExperimentAnnotations(container, sourceExperiment, js, previousCopy, jobSupport, user, log);


// If there is a Panorama Public data catalog entry associated with the previous copy of the experiment, move it to the
// new container.
moveCatalogEntry(previousCopy, targetExperiment, user);
Expand Down Expand Up @@ -183,6 +183,14 @@ private void finishUp(PipelineJob job, CopyExperimentJobSupport jobSupport) thro
verifySymlinks(previousCopy.getContainer(), container, false);
}

// Assign the ProteomeXchange ID and DOI at the end so that we don't we don't create these identifiers again in case the task has to be rerun due a previous error.
assignExternalIdentifiers(targetExperiment, previousCopy, jobSupport, log);
targetExperiment = ExperimentAnnotationsManager.save(targetExperiment, user);
if (previousCopy != null)
{
ExperimentAnnotationsManager.save(previousCopy, user);
}

// Create notifications. Do this at the end after everything else is done.
PanoramaPublicNotification.notifyCopied(sourceExperiment, targetExperiment, jobSupport.getJournal(), js.getJournalExperiment(), currentSubmission,
reviewer.first, reviewer.second, user, previousCopy != null /*This is a re-copy if previousCopy exists*/);
Expand Down Expand Up @@ -389,6 +397,8 @@ private void assignDoi(CopyExperimentJobSupport jobSupport, Logger log, Experime
{
log.info("Copying DOI from the previous copy of the data.");
targetExperiment.setDoi(previousCopy.getDoi());
log.info("Removing DOI from the previous copy");
previousCopy.setDoi(null);
}
else
{
Expand Down Expand Up @@ -425,6 +435,7 @@ private void assignPxId(CopyExperimentJobSupport jobSupport, Logger log, Experim
try
{
assignPxId(targetExperiment, jobSupport.usePxTestDb());
log.info("Assigned ProteomeXchange ID: " + targetExperiment.getPxid());
}
catch(ProteomeXchangeServiceException e)
{
Expand Down Expand Up @@ -462,15 +473,13 @@ private ExperimentAnnotations updateExperimentAnnotations(Container targetContai
{
throw new PipelineJobException("ExperimentAnnotations row does not exist in target folder: '" + targetContainer.getPath() + "'");
}

targetExperiment.setShortUrl(js.getShortAccessUrl());
Integer currentVersion = ExperimentAnnotationsManager.getMaxVersionForExperiment(sourceExperiment.getId());
int version = currentVersion == null ? 1 : currentVersion + 1;
log.info("Setting version on new experiment to " + version);
targetExperiment.setDataVersion(version);

assignPxId(jobSupport, log, targetExperiment, previousCopy);
assignDoi(jobSupport, log, targetExperiment, previousCopy);

targetExperiment = ExperimentAnnotationsManager.save(targetExperiment, user);

// Update the target of the short access URL to the journal's copy of the experiment.
Expand All @@ -488,6 +497,14 @@ private ExperimentAnnotations updateExperimentAnnotations(Container targetContai
return targetExperiment;
}


private void assignExternalIdentifiers(ExperimentAnnotations targetExperiment, ExperimentAnnotations previousCopy, CopyExperimentJobSupport jobSupport,
Logger log) throws PipelineJobException
{
assignPxId(jobSupport, log, targetExperiment, previousCopy);
assignDoi(jobSupport, log, targetExperiment, previousCopy);
}

private void updatePermissions(User formSubmitter, ExperimentAnnotations targetExperiment, ExperimentAnnotations sourceExperiment, ExperimentAnnotations previousCopy,
Journal journal, User pipelineJobUser, Logger log)
{
Expand All @@ -498,37 +515,43 @@ private void updatePermissions(User formSubmitter, ExperimentAnnotations targetE

// Give read permissions to the authors (all users that are folder admins)
log.info("Adding read permissions to all users that are folder admins in the source folder.");
List<User> sourceFolderAdmins = getUsersWithRole(sourceExperiment.getContainer(), RoleManager.getRole(FolderAdminRole.class));

Container target = targetExperiment.getContainer();
MutableSecurityPolicy newPolicy = new MutableSecurityPolicy(target, target.getPolicy());
for (User folderAdmin: sourceFolderAdmins)
{
newPolicy.addRoleAssignment(folderAdmin, ReaderRole.class);
}

// Assign the PanoramaPublicSubmitterRole so that the submitter or lab head is able to make the copied folder public, and add publication information.
assignPanoramaPublicSubmitterRole(newPolicy, log, targetExperiment.getSubmitterUser(), targetExperiment.getLabHeadUser(),
formSubmitter); // User that submitted the form. Can be different from the user selected as the data submitter

addToSubmittersGroup(target.getProject(), log, targetExperiment.getSubmitterUser(), targetExperiment.getLabHeadUser(), formSubmitter);
Set<User> allReaders = new HashSet<>(getUsersWithRole(sourceExperiment.getContainer(), RoleManager.getRole(FolderAdminRole.class)));

// If the submitter and lab head user are members of a permission group in the source folder, they will not
// be included in the set of users returned by getUsersWithRole(). So add them here
allReaders.add(formSubmitter); // User that submitted the form. Can be different from the user selected as the data submitter
allReaders.add(targetExperiment.getSubmitterUser());
allReaders.add(targetExperiment.getLabHeadUser());

if (previousCopy != null)
{
// Users that had read access to the previous copy should be given read access to the new copy. This will include the reviewer
// account if one was created for the previous copy.
log.info("Adding read permissions to all users that had read access to previous copy.");
List<User> previousCopyReaders = getUsersWithRole(previousCopy.getContainer(), RoleManager.getRole(ReaderRole.class));
previousCopyReaders.forEach(u -> newPolicy.addRoleAssignment(u, ReaderRole.class));
allReaders.addAll(getUsersWithRole(previousCopy.getContainer(), RoleManager.getRole(ReaderRole.class)));
}

Container target = targetExperiment.getContainer();
MutableSecurityPolicy newPolicy = new MutableSecurityPolicy(target, target.getPolicy());
allReaders.stream().filter(Objects::nonNull).forEach(u ->
{
log.info("Assigning " + ReaderRole.class.getSimpleName() + " to " + u.getEmail());
newPolicy.addRoleAssignment(u, ReaderRole.class);
});


// Assign the PanoramaPublicSubmitterRole so that the submitter or lab head is able to make the copied folder public, and add publication information.
assignPanoramaPublicSubmitterRole(newPolicy, log, targetExperiment.getSubmitterUser(), targetExperiment.getLabHeadUser(),
formSubmitter); // User that submitted the form. Can be different from the user selected as the data submitter

SecurityPolicyManager.savePolicy(newPolicy);

addToSubmittersGroup(target.getProject(), log, targetExperiment.getSubmitterUser(), targetExperiment.getLabHeadUser(), formSubmitter);
}

private void assignPanoramaPublicSubmitterRole(MutableSecurityPolicy policy, Logger log, User... users)
{
Arrays.stream(users).filter(Objects::nonNull).forEach(user -> {
Arrays.stream(users).filter(Objects::nonNull).collect(Collectors.toSet()).forEach(user -> {
log.info("Assigning " + PanoramaPublicSubmitterRole.class.getSimpleName() + " to " + user.getEmail());
policy.addRoleAssignment(user, PanoramaPublicSubmitterRole.class, false);
});
Expand All @@ -540,7 +563,7 @@ private void addToSubmittersGroup(Container project, Logger log, User... users)
Group group = getGroup(groupName, project, log);
if (group != null)
{
Arrays.stream(users).filter(Objects::nonNull).forEach(user -> {
Arrays.stream(users).filter(Objects::nonNull).collect(Collectors.toSet()).forEach(user -> {
// This group already exists in the Panorama Public project on panoramaweb.org.
// CONSIDER: Make this configurable through the Panorama Public admin console.
addToGroup(user, group, log);
Expand Down Expand Up @@ -592,13 +615,13 @@ private Group getGroup(String groupName, Container project, Logger log)
return group;
}

private List<User> getUsersWithRole(Container container, Role role)
private Set<User> getUsersWithRole(Container container, Role role)
{
SecurityPolicy securityPolicy = container.getPolicy();
return securityPolicy.getAssignments().stream()
.filter(r -> r.getRole().equals(role)
&& UserManager.getUser(r.getUserId()) != null) // Ignore user groups
.map(r -> UserManager.getUser(r.getUserId())).collect(Collectors.toList());
.map(r -> UserManager.getUser(r.getUserId())).collect(Collectors.toSet());

}

Expand Down Expand Up @@ -726,11 +749,6 @@ private void updateLastCopiedExperiment(ExperimentAnnotations previousCopy, Jour

log.info("Setting the permanent link on the previous copy to " + newShortUrl.getShortURL());
previousCopy.setShortUrl(newShortUrl);
if (previousCopy.getDoi() != null)
{
log.info("Removing DOI from the previous copy");
previousCopy.setDoi(null);
}

ExperimentAnnotationsManager.save(previousCopy, user);
}
Expand Down
5 changes: 1 addition & 4 deletions panoramapublic/src/org/labkey/panoramapublic/view/publish/confirmPublish.jsp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,8 @@
var items = [];
if (<%=!bean.isPublic()%>) {
let html = 'Data at ' + <%=qh(bean.getAccessUrl())%> + ' will be made public.';
if (<%= bean.getLicense() != null %>) {
html += ' It will be available under the ' + <%=qh(bean.getLicense().getDisplayName())%> + ' license.';
}
html += <%= qh(bean.getLicense() != null ? " It will be available under the " + bean.getLicense().getDisplayName() + " license." : "") %>;
items.push({xtype: 'component', style: 'margin: 5px 0 5px 0', html: html});
}
if (<%=form.hasPubmedId()%>) {
items.push({xtype: 'component', html: '<b>PubMed ID:</b> ' + <%=qh(form.getPubmedId())%>});
Expand Down
Loading

0 comments on commit 776f6a8

Please sign in to comment.