Skip to content

Commit

Permalink
Merge 24.11 to 25.2
Browse files Browse the repository at this point in the history
  • Loading branch information
labkey-teamcity committed Feb 18, 2025
2 parents b7c124b + 27b73a1 commit 520f04b
Show file tree
Hide file tree
Showing 22 changed files with 120 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public class DefaultAuditTypeTable extends FilteredTable<UserSchema>
@Override
protected ContainerFilter getDefaultContainerFilter()
{
return ContainerFilter.Type.CurrentWithUser.create(_userSchema);
return ContainerFilter.Type.Current.create(_userSchema);
}

public DefaultAuditTypeTable(AuditTypeProvider provider, TableInfo storage, UserSchema schema, ContainerFilter cf, List<FieldKey> defaultVisibleColumns)
Expand Down
27 changes: 11 additions & 16 deletions api/src/org/labkey/api/data/ContainerFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ protected SQLFragment _getSQLFragment(DbSchema schema, Container container, SQLF
List<Container> containers = ids.stream()
.map(ContainerManager::getForId)
.filter(Objects::nonNull)
.collect(Collectors.toUnmodifiableList());
.toList();
boolean hasNoSpecialChildren = includedChildTypes.isEmpty() ||
containers.stream().noneMatch(c -> c.hasChildrenOfAnyType(finalIncludedChildTypes));

Expand Down Expand Up @@ -355,14 +355,6 @@ public interface Factory
public enum Type implements Factory
{
Current("Current folder")
{
@Override
public ContainerFilter create(Container c, User user)
{
return new CurrentContainerFilter(c);
}
},
CurrentWithUser("Current folder with permissions applied to user")
{
@Override
public ContainerFilter create(Container c, User user)
Expand Down Expand Up @@ -494,7 +486,7 @@ public ContainerFilter create(Container container, User user)

private final String _description;

private Type(String description)
Type(String description)
{
_description = description;
}
Expand All @@ -514,17 +506,16 @@ public ContainerFilter create(ContainerUser cu)
}
}

// short for ContainerFilter.Type.Current.create(container, null)
// Does not validate permissions!
public static ContainerFilter current(Container c)
{
return new CurrentContainerFilter(c);
}

public static class CurrentContainerFilter extends ContainerFilter
private static class CurrentContainerFilter extends ContainerFilter
{
CurrentContainerFilter(Container c)
{
// CurrentContainerFilter does not validate permission
super(c,null);
Objects.requireNonNull(c);
}
Expand Down Expand Up @@ -563,6 +554,10 @@ public static class ContainerFilterWithPermission extends ContainerFilter
public ContainerFilterWithPermission(Container c, User user)
{
super(c, user);
// TODO: InternalNoContainerFilter should extend ContainerFilter instead of ContainerFilterWithPermission,
// which would allow a more strict check below (c != null && user != null). Also, once verified on
// TeamCity, throw an exception here instead of asserting.
assert c == null || user != null : "User is required for permissions check if container is provided!";
}

@Override
Expand All @@ -588,7 +583,7 @@ public SQLFragment getSQLFragment(DbSchema schema, SQLFragment containerColumnSQ
return getSQLFragment(schema, _container, containerColumnSQL, ids, allowNulls, getIncludedChildTypes());
}

/** return null means return all rows (1=1), empty collection means return no rows (1=0) */
/** return null means return all rows (1=1), empty collection means return no rows (1=0) */
@Nullable
public Collection<GUID> generateIds(Container currentContainer, Class<? extends Permission> permission, Set<Role> roles)
{
Expand Down Expand Up @@ -619,7 +614,7 @@ public final Collection<GUID> getIds()
@Override
public Type getType()
{
return Type.CurrentWithUser;
return Type.Current;
}
}

Expand Down Expand Up @@ -1029,7 +1024,7 @@ public Type getType()

public static class StudyAndSourceStudy extends ContainerFilterWithPermission
{
private boolean _skipPermissionChecks;
private final boolean _skipPermissionChecks;

public StudyAndSourceStudy(Container c, User user, boolean skipPermissionChecks)
{
Expand Down
4 changes: 2 additions & 2 deletions api/src/org/labkey/api/data/DataRegion.java
Original file line number Diff line number Diff line change
Expand Up @@ -2341,9 +2341,9 @@ private void renderForm(RenderContext ctx, Writer out) throws IOException
{
out.write("<input type='hidden' name='");
if (viewForm != null)
out.write(viewForm.getFormFieldName(pkCol));
out.write(PageFlowUtil.filter(viewForm.getFormFieldName(pkCol)));
else
out.write(pkColName);
out.write(PageFlowUtil.filter(pkColName));
out.write("' value=\"");
out.write(PageFlowUtil.filter(pkVal.toString()));
out.write("\">");
Expand Down
7 changes: 7 additions & 0 deletions api/src/org/labkey/api/search/SearchService.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;

public interface SearchService extends SearchMXBean
Expand Down Expand Up @@ -113,6 +114,12 @@ static void setInstance(SearchService impl)
*/
void reindexContainerFiles(Container c);

/**
* Puts work in the indexer queue at the specified priority and waits up to the timeout for it to complete
* @return true if the task in the queue completed before the timeout
*/
boolean drainQueue(PRIORITY priority, long timeout, TimeUnit unit) throws InterruptedException;

enum PRIORITY
{
commit,
Expand Down
4 changes: 3 additions & 1 deletion api/webapp/extPatches/ext3-patches.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,4 +377,6 @@ Ext.override(Ext.tree.TreeNodeUI, {
this.anchor = cs[index];
this.textNode = cs[index].firstChild;
}
});
});

Ext.labkeyPatches = true; // Allow short-circuiting in 'LABKEY.requiresExt3'
1 change: 1 addition & 0 deletions api/webapp/extPatches/ext4-patches.js
Original file line number Diff line number Diff line change
Expand Up @@ -1149,3 +1149,4 @@ Ext4.override(Ext4.form.FieldSet, {
}
});

Ext4.labkeyPatches = true; // Allow short-circuiting in 'LABKEY.requiresExt4Sandbox'
2 changes: 1 addition & 1 deletion core/src/org/labkey/core/admin/createFolder.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@
const getTemplateFolders = function(data) {
// add the container itself to the templateFolder object if it is not the root and the user has admin perm to it
// and if it is not a workbook or container tab folder
if (data.path !== "/" && LABKEY.Security.hasEffectivePermission(data.effectivePermissions, LABKEY.Security.effectivePermissions.admin)
if (data.path !== "/" && data.effectivePermissions && LABKEY.Security.hasEffectivePermission(data.effectivePermissions, LABKEY.Security.effectivePermissions.admin)
&& !data.isWorkbook && !data.isContainerTab)
{
templateFolders.push([data.id, data.path]);
Expand Down
6 changes: 6 additions & 0 deletions core/src/org/labkey/core/search/NoopSearchService.java
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,12 @@ public void reindexContainerFiles(Container c)
{
}

@Override
public boolean drainQueue(PRIORITY priority, long timeout, TimeUnit unit)
{
return true;
}

@Override
public void addPathToCrawl(Path path, Date d)
{
Expand Down
18 changes: 16 additions & 2 deletions core/webapp/labkey.js
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,14 @@ if (typeof LABKEY == "undefined")

checkCallback('requiresExt3', callback);

requiresLib('Ext3', callback, scope);
if (window.Ext && window.Ext.labkeyPatches)
{
handle(callback, scope);
}
else
{
requiresLib('Ext3', callback, scope);
}
};

var requiresExt3ClientAPI = function(callback, scope)
Expand Down Expand Up @@ -522,7 +529,14 @@ if (typeof LABKEY == "undefined")

checkCallback('requiresExt4Sandbox', callback);

requiresLib('Ext4', callback, scope);
if (window.Ext4 && window.Ext4.labkeyPatches)
{
handle(callback, scope);
}
else
{
requiresLib('Ext4', callback, scope);
}
};

var requiresLib = function(lib, callback, scope)
Expand Down
2 changes: 1 addition & 1 deletion core/webapp/vis/src/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -2142,7 +2142,7 @@ boxPlot.render();
// during the log conversion at getLogScale L810, the below code ensures that the minimum scale of the y-axis
// is not less than 0 due to calculations in convertYAxisDomain
if (config.properties.yAxisScale === 'log') {
if (config?.properties?.yAxisDomain?.length >= 0 && config.properties.yAxisDomain[0] <= 0) {
if (config?.properties?.yAxisDomain?.length >= 0 && config.properties.yAxisDomain[0] < 0) {
config.properties.yAxisDomain[0] = minimumValue - cushion;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@
import org.labkey.api.exp.query.ExpSchema;
import org.labkey.api.query.FieldKey;
import org.labkey.api.query.FilteredTable;
import org.labkey.api.security.UserPrincipal;
import org.labkey.api.security.permissions.AdminPermission;
import org.labkey.api.security.permissions.Permission;

public abstract class BaseFieldNamesTable extends FilteredTable<ExpSchema>
{
Expand All @@ -40,11 +38,10 @@ protected MutableColumnInfo addColumn(String name, JdbcType type)
return addColumn(new BaseColumnInfo(FieldKey.fromParts(name), this, type));
}

// This is a hack, required because ContainerFilter.Type.Current doesn't fill in the user
@Override
public boolean hasPermission(@NotNull UserPrincipal user, @NotNull Class<? extends Permission> perm)
protected ContainerFilter getDefaultContainerFilter()
{
return getContainer().hasPermission(user, AdminPermission.class);
return ContainerFilter.Type.Current.create(getUserSchema());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@
<%@ page import="org.labkey.api.data.Sort" %>
<%@ page import="org.labkey.api.exp.api.DataClassDomainKindProperties" %>
<%@ page import="org.labkey.api.action.ApiUsageException" %>
<%@ page import="org.labkey.api.search.SearchService" %>
<%@ page import="java.util.concurrent.TimeUnit" %>

<%@ page extends="org.labkey.api.jsp.JspTest.BVT" %>

Expand All @@ -118,8 +120,10 @@ public void setUp()
}
@After
public void tearDown()
public void tearDown() throws InterruptedException
{
// Wait for the indexer to finish working on the data we just added to help avoid deadlocks
SearchService.get().drainQueue(SearchService.PRIORITY.crawl, 15, TimeUnit.SECONDS);
ContainerManager.deleteAll(c, TestContext.get().getUser());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@
<%@ page import="org.labkey.api.reader.MapLoader" %>
<%@ page import="org.labkey.api.action.ApiUsageException" %>
<%@ page import="org.labkey.api.exp.api.ExpLineageService" %>
<%@ page import="org.labkey.api.search.SearchService" %>
<%@ page import="java.util.concurrent.TimeUnit" %>

<%@ page extends="org.labkey.api.jsp.JspTest.BVT" %>

Expand All @@ -114,8 +116,10 @@ public void setUp()
}
@After
public void tearDown()
public void tearDown() throws InterruptedException
{
// Wait for the indexer to finish working on the data we just added to help avoid deadlocks
SearchService.get().drainQueue(SearchService.PRIORITY.crawl, 15, TimeUnit.SECONDS);
ContainerManager.deleteAll(c, TestContext.get().getUser());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2233,7 +2233,7 @@ public Set<String> getDataInputRoles(Container container, ContainerFilter filter
@Override
public Set<String> getMaterialInputRoles(Container container, ExpProtocol.ApplicationType... types)
{
return getInputRoles(container, ContainerFilter.Type.Current.create(container, null), getTinfoMaterialInput(), types);
return getInputRoles(container, ContainerFilter.current(container), getTinfoMaterialInput(), types);
}

private Set<String> getInputRoles(Container container, ContainerFilter filter, TableInfo table, ExpProtocol.ApplicationType... types)
Expand Down
29 changes: 23 additions & 6 deletions issues/src/org/labkey/issue/model/IssueManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,18 @@ private static IssueObject _getRawIssue(@Nullable Container c, int issueId)

@Nullable
public static IssueObject getIssue(@Nullable Container c, User user, int issueId)
{
return getIssue(c, user, issueId, true);
}

@Nullable
public static IssueObject getIssue(
@Nullable Container c,
User user,
int issueId,
boolean throwOnRestrictedFailure // controls whether we throw on a RestrictedIssueProvider failure
// or just return null
)
{
IssueObject issue = _getIssue(c, user, issueId);

Expand All @@ -206,12 +218,17 @@ public static IssueObject getIssue(@Nullable Container c, User user, int issueId

if (!provider.hasPermission(user, issue, relatedIssues, errors))
{
StringBuilder msg = new StringBuilder(errors.isEmpty() ? "Access denied" : "");
for (ValidationError ve : errors)
if (throwOnRestrictedFailure)
{
msg.append(ve.getMessage()).append("\n");
StringBuilder msg = new StringBuilder(errors.isEmpty() ? "Access denied" : "");
for (ValidationError ve : errors)
{
msg.append(ve.getMessage()).append("\n");
}
throw new UnauthorizedException(msg.toString());
}
throw new UnauthorizedException(msg.toString());
else
return null;
}
}
return issue;
Expand Down Expand Up @@ -280,7 +297,7 @@ public static List<IssueObject.CommentObject> getCommentsForRelatedIssues(IssueO
for (Integer relatedIssueInt : relatedIssues)
{
// only add related issues that the user has permission to see
IssueObject relatedIssue = IssueManager.getIssue(null, user, relatedIssueInt);
IssueObject relatedIssue = IssueManager.getIssue(null, user, relatedIssueInt, false);
if (relatedIssue != null)
{
boolean hasReadPermission = ContainerManager.getForId(relatedIssue.getContainerId()).hasPermission(user, ReadPermission.class);
Expand Down Expand Up @@ -315,7 +332,7 @@ public static boolean hasRelatedIssues(IssueObject issue, User user)
{
for (Integer relatedIssueInt : issue.getRelatedIssues())
{
IssueObject relatedIssue = IssueManager.getIssue(null, user, relatedIssueInt);
IssueObject relatedIssue = IssueManager.getIssue(null, user, relatedIssueInt, false);
if (relatedIssue != null && relatedIssue.getCommentObjects().size() > 0)
{
boolean hasReadPermission = ContainerManager.getForId(relatedIssue.getContainerId()).hasPermission(user, ReadPermission.class);
Expand Down
2 changes: 1 addition & 1 deletion issues/src/org/labkey/issue/model/IssuePage.java
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ public HtmlString renderAttachments(ViewContext context, CommentObject comment)

public String renderIssueIdLink(Integer id)
{
IssueObject issue = IssueManager.getIssue(null, _user, id);
IssueObject issue = IssueManager.getIssue(null, _user, id, false);
Container c = issue != null ? issue.lookupContainer() : null;
if (c != null && c.hasPermission(_user, ReadPermission.class))
{
Expand Down
6 changes: 6 additions & 0 deletions issues/src/org/labkey/issue/view/RelatedIssuesView.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.labkey.api.view.ViewContext;
import org.labkey.issue.model.IssueListDef;
import org.labkey.issue.model.IssueManager;
import org.labkey.issue.model.IssueObject;
import org.labkey.issue.query.IssuesQuerySchema;
import org.springframework.validation.BindException;

Expand Down Expand Up @@ -67,13 +68,18 @@ public RelatedIssuesView(@NotNull ViewContext context, @NotNull Set<Integer> rel
Integer issueId = (Integer)m.get("issueId");
String containerId = (String)m.get("container");
Container c = ContainerManager.getForId(containerId);

if (c == null || !c.hasPermission(getViewContext().getUser(), ReadPermission.class))
return;

IssueListDef d = IssueManager.getIssueListDef(c, issueDefId);
if (d == null)
return;

IssueObject issue = IssueManager.getIssue(null, context.getUser(), issueId, false);
if (issue == null)
return;

// If the user doesn't have ReadPermission to the domain container, we won't be able to create a query
// table in that container. In this case, just use the issue's container. As a consequence, any other
// from the same domain definition issueListDef that live in different containers will appear in separate grids.
Expand Down
Loading

0 comments on commit 520f04b

Please sign in to comment.