diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index 1a0ebb21c07..54600d0b67f 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -46,6 +46,7 @@ import org.labkey.api.data.AbstractForeignKey; import org.labkey.api.data.Aggregate; import org.labkey.api.data.AtomicDatabaseInteger; +import org.labkey.api.data.BindingTestCase; import org.labkey.api.data.BooleanFormat; import org.labkey.api.data.BuilderObjectFactory; import org.labkey.api.data.CompareType; @@ -508,6 +509,7 @@ public void registerServlets(ServletContext servletCtx) ApiKeyManager.TestCase.class, AppPropsTestCase.class, AtomicDatabaseInteger.TestCase.class, + BindingTestCase.class, BlockingCache.BlockingCacheTest.class, CompareType.TestCase.class, ContainerDisplayColumn.TestCase.class, diff --git a/api/src/org/labkey/api/action/BaseApiAction.java b/api/src/org/labkey/api/action/BaseApiAction.java index 2eac48f1672..0ad51280c21 100644 --- a/api/src/org/labkey/api/action/BaseApiAction.java +++ b/api/src/org/labkey/api/action/BaseApiAction.java @@ -44,6 +44,7 @@ import org.labkey.api.view.UnauthorizedException; import org.labkey.api.view.ViewContext; import org.springframework.beans.MutablePropertyValues; +import org.springframework.beans.PropertyValues; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.validation.BindException; import org.springframework.validation.Errors; @@ -339,7 +340,7 @@ private FormAndErrors
defaultPopulateForm() throws Exception { saveRequestedApiVersion(getViewContext().getRequest(), null); - BindException errors = defaultBindParameters(getCommand(), getPropertyValues()); + BindException errors = defaultBindParameters(getPropertyValues()); FORM form = (FORM)errors.getTarget(); return new FormAndErrors<>(form, errors); @@ -460,6 +461,16 @@ private FormAndErrors populateJSONObjectForm() throws Exception } saveRequestedApiVersion(getViewContext().getRequest(), jsonObj); + // Records are immutable, so we can't instantiate the form up front and populate it; instead collect the JSON + // properties and construct the record via defaultBindParameters(). Note: record forms can't implement + // ApiJsonForm since that relies on mutating an existing instance. + if (getCommandClass().isRecord()) + { + PropertyValues values = null == jsonObj ? new MutablePropertyValues() : new JsonPropertyValues(jsonObj); + BindException errors = defaultBindParameters(values); + return new FormAndErrors<>((FORM)errors.getTarget(), errors); + } + FORM form = getCommand(); BindException errors = populateForm(jsonObj, form); return new FormAndErrors<>(form, errors); diff --git a/api/src/org/labkey/api/action/BaseViewAction.java b/api/src/org/labkey/api/action/BaseViewAction.java index 9af05802d35..7d4f3020b85 100644 --- a/api/src/org/labkey/api/action/BaseViewAction.java +++ b/api/src/org/labkey/api/action/BaseViewAction.java @@ -31,6 +31,7 @@ import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.data.Container; import org.labkey.api.data.ConvertHelper; +import org.labkey.api.data.ObjectFactory; import org.labkey.api.security.User; import org.labkey.api.util.HelpTopic; import org.labkey.api.util.HttpUtil; @@ -78,6 +79,9 @@ import java.util.List; import java.util.Map; import java.util.function.Predicate; +import java.util.stream.Collectors; + +import static org.labkey.api.action.SpringActionController.ERROR_MSG; public abstract class BaseViewAction extends PermissionCheckableAction implements Validator, HasPageConfig, ContainerUser { @@ -122,28 +126,23 @@ protected BaseViewAction() setCommandClass(typeBest); } - protected abstract String getCommandClassMethodName(); - protected BaseViewAction(@NotNull Class commandClass) { setCommandClass(commandClass); } - public void setProperties(PropertyValues pvs) { _pvs = pvs; } - public void setProperties(Map m) { _pvs = new MutablePropertyValues(m); } - /* Doesn't guarantee non-null, non-empty */ public Object getProperty(String key, String d) { @@ -151,14 +150,12 @@ public Object getProperty(String key, String d) return pv == null ? d : pv.getValue(); } - public Object getProperty(Enum key) { PropertyValue pv = _pvs.getPropertyValue(key.name()); return pv == null ? null : pv.getValue(); } - public Object getProperty(String key) { PropertyValue pv = _pvs.getPropertyValue(key); @@ -170,7 +167,6 @@ public PropertyValues getPropertyValues() return _pvs; } - public static PropertyValues getPropertyValuesForFormBinding(PropertyValues pvs, @NotNull Predicate allowBind) { if (null == pvs) @@ -184,7 +180,6 @@ public static PropertyValues getPropertyValuesForFormBinding(PropertyValues pvs, return ret; } - /// Some characters can be mishandled by the browser in multipart/formdata requests (e.g. doublequote and backslask). /// We support an encoding from fields to avoid these characters, see {@link PageFlowUtil#encodeFormName} and {@link PageFlowUtil#decodeFormName}. static public class ViewActionParameterPropertyValues extends ServletRequestParameterPropertyValues @@ -218,7 +213,6 @@ public ModelAndView handleRequest(@NotNull HttpServletRequest request, @NotNull return handleRequest(); } - private void handleSpecialProperties() { _robot = PageFlowUtil.isRobotUserAgent(getViewContext().getRequest().getHeader("User-Agent")); @@ -260,55 +254,47 @@ private boolean hasStringValue(String propertyName) public abstract ModelAndView handleRequest() throws Exception; - @Override public void setPageConfig(PageConfig page) { _pageConfig = page; } - @Override public Container getContainer() { return getViewContext().getContainer(); } - @Override public User getUser() { return getViewContext().getUser(); } - @Override public PageConfig getPageConfig() { return _pageConfig; } - public void setTitle(String title) { assert null != getPageConfig() : "action not initialized property"; getPageConfig().setTitle(title); } - public void setHelpTopic(String topicName) { setHelpTopic(new HelpTopic(topicName)); } - public void setHelpTopic(HelpTopic topic) { assert null != getPageConfig() : "action not initialized properly"; getPageConfig().setHelpTopic(topic); } - protected Object newInstance(Class c) { try @@ -324,7 +310,6 @@ protected Object newInstance(Class c) } } - protected @NotNull FORM getCommand(HttpServletRequest request) throws Exception { FORM command = (FORM) createCommand(); @@ -335,13 +320,11 @@ protected Object newInstance(Class c) return command; } - protected @NotNull FORM getCommand() throws Exception { return getCommand(getViewContext().getRequest()); } - // // PARAMETER BINDING // @@ -353,6 +336,45 @@ protected Object newInstance(Class c) return defaultBindParameters(form, getCommandName(), params); } + /** + * Bind request parameters to the action's command class, dispatching to record binding when that class is a Java + * record. Records are immutable (no setters), so instead of instantiating the form and populating it via Spring data + * binding we collect the property values into a map and construct the record via its {@link ObjectFactory}. Callers + * that previously invoked {@code defaultBindParameters(getCommand(), params)} directly should prefer this method so + * they transparently gain record support. + */ + public @NotNull BindException defaultBindParameters(PropertyValues params) throws Exception + { + Class commandClass = getCommandClass(); + return commandClass.isRecord() ? bindParametersToRecord(commandClass, params, getCommandName()) : defaultBindParameters(getCommand(), params); + } + + // Simple binding for Java records: provides binding errors for missing primitive parameters and type conversions + // failures. Currently, no support for array or list parameter types. + public static BindException bindParametersToRecord(Class recordClass, PropertyValues pvs, String commandName) + { + // Note: We don't support record-based forms implementing HasAllowBindParameter since we must populate all + // properties at record construction time and therefore can't invoke allowBindParameter() prior to that. + PropertyValues m = getPropertyValuesForFormBinding(pvs, HasAllowBindParameter.getDefaultPredicate()); + ObjectFactory factory = ObjectFactory.Registry.getFactory(recordClass); + Map map = m.stream() + .filter(pv -> pv.getValue() != null) + .collect(Collectors.toMap(PropertyValue::getName, PropertyValue::getValue)); + BindException errors; + try + { + R record = factory.fromMap(map); + errors = new NullSafeBindException(record, commandName); + } + catch (IllegalArgumentException | ConversionException e) + { + // Missing primitive parameter or type conversion error. We have no instance to bind to, so report a global + // error with details. + errors = new NullSafeBindException(new Object(), commandName); + errors.reject(ERROR_MSG, e.getMessage()); + } + return errors; + } public static @NotNull BindException defaultBindParameters(Object form, String commandName, PropertyValues params) { @@ -398,28 +420,28 @@ protected Object newInstance(Class c) // Maybe we should propagate exception and return SC_BAD_REQUEST (in ExceptionUtil.handleException()) // most POST handlers check errors.hasErrors(), but not all GET handlers do BindException errors = new BindException(command, commandName); - errors.reject(SpringActionController.ERROR_MSG, "Error binding property: " + x.getPropertyName()); + errors.reject(ERROR_MSG, "Error binding property: " + x.getPropertyName()); return errors; } catch (NumberFormatException x) { // Malformed array parameter throws this exception, unfortunately. Just reject the request. #21931 BindException errors = new BindException(command, commandName); - errors.reject(SpringActionController.ERROR_MSG, "Error binding array property; invalid array index (" + x.getMessage() + ")"); + errors.reject(ERROR_MSG, "Error binding array property; invalid array index (" + x.getMessage() + ")"); return errors; } catch (NegativeArraySizeException x) { // Another malformed array parameter throws this exception. #23929 BindException errors = new BindException(command, commandName); - errors.reject(SpringActionController.ERROR_MSG, "Error binding array property; negative array size (" + x.getMessage() + ")"); + errors.reject(ERROR_MSG, "Error binding array property; negative array size (" + x.getMessage() + ")"); return errors; } catch (IllegalArgumentException x) { // General bean binding problem. #23929 BindException errors = new BindException(command, commandName); - errors.reject(SpringActionController.ERROR_MSG, "Error binding property; (" + x.getMessage() + ")"); + errors.reject(ERROR_MSG, "Error binding property; (" + x.getMessage() + ")"); return errors; } } diff --git a/api/src/org/labkey/api/action/ConfirmAction.java b/api/src/org/labkey/api/action/ConfirmAction.java index 32dd2efa002..791218d6d35 100644 --- a/api/src/org/labkey/api/action/ConfirmAction.java +++ b/api/src/org/labkey/api/action/ConfirmAction.java @@ -112,7 +112,7 @@ protected String getCommandClassMethodName() public BindException bindParameters(PropertyValues m) throws Exception { - return defaultBindParameters(getCommand(), m); + return defaultBindParameters(m); } /** diff --git a/api/src/org/labkey/api/action/FormApiAction.java b/api/src/org/labkey/api/action/FormApiAction.java index fefe8af4942..0360e182388 100644 --- a/api/src/org/labkey/api/action/FormApiAction.java +++ b/api/src/org/labkey/api/action/FormApiAction.java @@ -79,7 +79,7 @@ protected ModelAndView getPrintView(FORM form, BindException errors) throws Exce protected BindException bindParameters(PropertyValues pvs) throws Exception { - return SimpleViewAction.defaultBindParameters(getCommand(), getCommandName(), pvs); + return defaultBindParameters(pvs); } /** diff --git a/api/src/org/labkey/api/action/FormViewAction.java b/api/src/org/labkey/api/action/FormViewAction.java index 8c32b8ddf41..f3ddb11c5c5 100644 --- a/api/src/org/labkey/api/action/FormViewAction.java +++ b/api/src/org/labkey/api/action/FormViewAction.java @@ -17,13 +17,11 @@ package org.labkey.api.action; import org.jetbrains.annotations.NotNull; -import org.labkey.api.data.ObjectFactory; import org.labkey.api.miniprofiler.MiniProfiler; import org.labkey.api.miniprofiler.Timing; import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.URLHelper; import org.labkey.api.view.HttpView; -import org.springframework.beans.PropertyValue; import org.springframework.beans.PropertyValues; import org.springframework.validation.BindException; import org.springframework.validation.Errors; @@ -31,8 +29,6 @@ import org.springframework.web.servlet.ModelAndView; import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; /** * Is this better than BaseCommandController? Probably not, but it understands TableViewForm. @@ -149,22 +145,7 @@ protected String getCommandClassMethodName() public BindException bindParameters(PropertyValues m) throws Exception { - Class commandClass = getCommandClass(); - return commandClass.isRecord() ? defaultBindParametersToRecord(commandClass, m) : defaultBindParameters(getCommand(), m); - } - - // Simple binding for Java records: no support for binding errors, arrays, lists, etc. - private BindException defaultBindParametersToRecord(Class recordClass, PropertyValues pvs) - { - // Note: We don't support record-based forms implementing HasAllowBindParameter since we must populate all - // properties at record construction time and therefore can't invoke allowBindParameter() prior to that. - PropertyValues m = getPropertyValuesForFormBinding(pvs, HasAllowBindParameter.getDefaultPredicate()); - ObjectFactory factory = ObjectFactory.Registry.getFactory(recordClass); - Map map = m.stream() - .filter(pv -> pv.getValue() != null) - .collect(Collectors.toMap(PropertyValue::getName, PropertyValue::getValue)); - R record = factory.fromMap(map); - return new NullSafeBindException(record, "Form"); + return defaultBindParameters(m); } @Override diff --git a/api/src/org/labkey/api/action/PermissionCheckableAction.java b/api/src/org/labkey/api/action/PermissionCheckableAction.java index d5b0fe91a02..83b291b49c8 100644 --- a/api/src/org/labkey/api/action/PermissionCheckableAction.java +++ b/api/src/org/labkey/api/action/PermissionCheckableAction.java @@ -258,7 +258,6 @@ private void _checkActionPermissions(Set contextualRoles) throws Unauthori throw new DeprecatedActionException(actionClass); } - private void checkPermissionsAndTermsOfUse(Set contextualRoles, boolean isSendBasic) throws UnauthorizedException { checkActionPermissions(contextualRoles); @@ -267,7 +266,6 @@ private void checkPermissionsAndTermsOfUse(Set contextualRoles, boolean is verifyTermsOfUse(isSendBasic); } - /** * Check if terms of use are ever required for this request. If so, enumerate all the terms-of-use providers and ask * each to verify terms are set via its custom mechanism. If a provider's terms are active and the user hasn't yet @@ -289,7 +287,6 @@ protected void verifyTermsOfUse(boolean isSendBasic) throws RedirectException provider.verifyTermsOfUse(context, isBasicAuth); } - /** * Actions may provide a set of {@link Role}s used during permission checking * or null if no contextual roles apply. diff --git a/api/src/org/labkey/api/action/SimpleViewAction.java b/api/src/org/labkey/api/action/SimpleViewAction.java index 631b0f74f5b..7afcf6b362b 100644 --- a/api/src/org/labkey/api/action/SimpleViewAction.java +++ b/api/src/org/labkey/api/action/SimpleViewAction.java @@ -114,7 +114,7 @@ protected String getCommandClassMethodName() public BindException bindParameters(PropertyValues pvs) throws Exception { - return defaultBindParameters(getCommand(), pvs); + return defaultBindParameters(pvs); } public void validate(FORM form, BindException errors) diff --git a/api/src/org/labkey/api/data/BindingTestCase.java b/api/src/org/labkey/api/data/BindingTestCase.java new file mode 100644 index 00000000000..f0ca2ab6e7e --- /dev/null +++ b/api/src/org/labkey/api/data/BindingTestCase.java @@ -0,0 +1,154 @@ +package org.labkey.api.data; + +import org.junit.Assert; +import org.junit.Test; +import org.labkey.api.action.BaseViewAction; +import org.labkey.api.data.RecordFactory.MiniUser; +import org.labkey.api.util.DateUtil; +import org.springframework.beans.MutablePropertyValues; +import org.springframework.beans.PropertyValues; +import org.springframework.validation.BindException; +import org.springframework.validation.ObjectError; + +import java.util.Date; +import java.util.Map; + +public class BindingTestCase extends Assert +{ + @Test + public void testBinding() + { + Date lastLogin = new Date(); + + // Provide all parameters + Map params = Map.of( + "firstName", "Fred", + "lastName", "Flintstone", + "lastLogin", DateUtil.formatIsoDateLongTime(lastLogin), + "userId", 1009 + ); + String toString = "MiniUser[FIRSTname=Fred, LASTNAME=Flintstone, lastLogin=" + lastLogin + ", UserId=1009]"; + testRecordBinding(params, toString); + testFormBinding(params, toString); + + // Provide just the primitive parameter; others are nullable + params = Map.of( + "userId", 1009 + ); + toString = "MiniUser[FIRSTname=null, LASTNAME=null, lastLogin=null, UserId=1009]"; + testRecordBinding(params, toString); + testFormBinding(params, toString); + + // No parameters should fail for record due to "userid" primitive. Ensure a reasonable error message. + testRecordBinding(Map.of(), "Primitive parameter \"UserId\" is required"); + // No parameters should succeed for form class. UserId simply defaults to 0; + testFormBinding(Map.of(), "MiniUser[FIRSTname=null, LASTNAME=null, lastLogin=null, UserId=0]"); + + // Verify message for conversion error + params = Map.of("UserId", "abc"); + String errorMessage = "Failed to convert property value of type 'java.lang.String' to required type 'int' for property 'UserId'; Could not convert 'abc' to an integer"; + testRecordBinding(params, errorMessage); + testFormBinding(params, errorMessage); + } + + private void testRecordBinding(Map map, String expectedToStringOrError) + { + PropertyValues pvs = new MutablePropertyValues(map); + BindException be = BaseViewAction.bindParametersToRecord(MiniUser.class, pvs, "form"); + + if (be.hasErrors()) + { + validateError(be, expectedToStringOrError); + } + else + { + validateTarget(be.getTarget(), expectedToStringOrError); + } + } + + private void testFormBinding(Map map, String expectedToStringOrError) + { + PropertyValues pvs = new MutablePropertyValues(map); + BindException be = BaseViewAction.defaultBindParameters(new MiniUserForm(), "form", pvs); + + if (be.hasErrors()) + { + validateError(be, expectedToStringOrError); + } + else + { + validateTarget(be.getTarget(), expectedToStringOrError); + } + } + + private void validateError(BindException be, String expectedErrorMessage) + { + ObjectError error = be.getAllErrors().getFirst(); + assertNotNull(error); + assertEquals(expectedErrorMessage, error.getDefaultMessage()); + } + + private void validateTarget(Object user, String expectedToString) + { + assertNotNull(user); + assertEquals(expectedToString, user.toString()); + } + + // Simple test form + @SuppressWarnings("unused") + private static class MiniUserForm + { + String _firstName; + String _lastName; + Date _lastLogin; + int _userId; + + public String getFirstName() + { + return _firstName; + } + + public void setFirstName(String firstName) + { + _firstName = firstName; + } + + public String getLastName() + { + return _lastName; + } + + public void setLastName(String lastName) + { + _lastName = lastName; + } + + public Date getLastLogin() + { + return _lastLogin; + } + + public void setLastLogin(Date lastLogin) + { + _lastLogin = lastLogin; + } + + public int getUserId() + { + return _userId; + } + + public void setUserId(int userId) + { + _userId = userId; + } + + @Override + public String toString() + { + // No useful default toString(), so emulate the standard record toString(). That's good enough to verify + // that the parameters were bound correctly. + return "MiniUser[FIRSTname=" + _firstName + ", LASTNAME=" + _lastName + ", lastLogin=" + _lastLogin + ", UserId=" + _userId + "]"; + } + } +} diff --git a/api/src/org/labkey/api/data/RecordFactory.java b/api/src/org/labkey/api/data/RecordFactory.java index ce94aa1070f..fa5b7be796a 100644 --- a/api/src/org/labkey/api/data/RecordFactory.java +++ b/api/src/org/labkey/api/data/RecordFactory.java @@ -15,6 +15,7 @@ */ package org.labkey.api.data; +import org.apache.commons.beanutils.ConversionException; import org.apache.commons.beanutils.ConvertUtils; import org.jetbrains.annotations.Nullable; import org.junit.Assert; @@ -63,17 +64,39 @@ public RecordFactory(Class clazz) .toList(); } + // Throws IllegalArgumentExceptions for missing primitive parameters and conversion errors private & CaseInsensitiveCollection> K fromCaseInsensitiveMap(MAP m) { Object[] params = Arrays.stream(_parameters).map(p -> { Object value = m.get(p.getName()); - return value != null ? ConvertUtils.convert(value, p.getType()) : null; + try + { + return value != null ? ConvertUtils.convert(value, p.getType()) : null; + } + catch (ConversionException e) + { + throw new IllegalArgumentException("Failed to convert property value of type '" + value.getClass().getName() + "' to required type '" + p.getType() + "' for property '" + p.getName() + "'; " + e.getMessage()); + } }).toArray(); try { return _constructor.newInstance(params); } + catch (IllegalArgumentException e) + { + // Try to determine if this failed due to missing primitive parameters to improve the exception message + List missingPrimitiveParameters = Arrays.stream(_parameters) + .filter(p -> p.getType().isPrimitive()) + .map(Parameter::getName) + .filter(name -> m.get(name) == null) + .toList(); + if (missingPrimitiveParameters.isEmpty()) + throw e; // Unclear what the problem is, so just re-throw + if (missingPrimitiveParameters.size() == 1) + throw new IllegalArgumentException("Primitive parameter \"" + missingPrimitiveParameters.getFirst() + "\" is required"); + throw new IllegalArgumentException("One or more primitive parameters are missing. Primitive parameters include: " + missingPrimitiveParameters); + } catch (InstantiationException | IllegalAccessException | InvocationTargetException e) { throw new RuntimeException(e); @@ -136,7 +159,7 @@ public ArrayList handleArrayList(ResultSet rs) public static class TestCase extends Assert { @Test - public void test() throws SQLException + public void testDatabase() throws SQLException { Map adHocMap = new CaseInsensitiveHashMap<>(); adHocMap.put("FirstName", "Keyser"); @@ -161,17 +184,17 @@ public void test() throws SQLException rs.next(); Assert.assertEquals(users.getFirst(), factory.handle(rs)); } - MiniUser randomUser = users.get((int)(Math.random() * users.size())); - MiniUser selectedUser = new TableSelector(CoreSchema.getInstance().getTableInfoUsers(), new SimpleFilter(FieldKey.fromString("UserId"), randomUser.userid), null).getObject(MiniUser.class); + MiniUser randomUser = users.get((int) (Math.random() * users.size())); + MiniUser selectedUser = new TableSelector(CoreSchema.getInstance().getTableInfoUsers(), new SimpleFilter(FieldKey.fromString("UserId"), randomUser.UserId), null).getObject(MiniUser.class); Assert.assertEquals(randomUser, selectedUser); // Test fromMap() variant (should ignore selectedUser) assertEquals(adHocUser, factory.fromMap(selectedUser, adHocMap)); } + } - // Simple test record. Weird casing is intentional to test case-insensitivity. - private record MiniUser(String FIRSTname, String LASTNAME, Date LastLogin, int userid) - { - } + // Simple test record. Weird casing is intentional to test case-insensitivity. + record MiniUser(String FIRSTname, String LASTNAME, Date lastLogin, int UserId) + { } } diff --git a/core/src/org/labkey/core/CoreController.java b/core/src/org/labkey/core/CoreController.java index 6988a789486..dfdb4b5abef 100644 --- a/core/src/org/labkey/core/CoreController.java +++ b/core/src/org/labkey/core/CoreController.java @@ -128,7 +128,6 @@ import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.security.roles.FolderAdminRole; -import org.labkey.api.security.roles.ProjectAdminRole; import org.labkey.api.security.roles.ReaderRole; import org.labkey.api.security.roles.RoleManager; import org.labkey.api.services.ServiceRegistry; @@ -323,7 +322,7 @@ public ActionURL getManageQCStatesURL(@NotNull Container container) } } - abstract static class BaseStylesheetAction extends ExportAction + abstract static class BaseStylesheetAction extends ExportAction { @Override public void checkPermissions() throws UnauthorizedException @@ -2872,8 +2871,7 @@ public Object execute(CustomLabelForm form, BindException errors) { try { - HashMap labels = - JsonUtil.DEFAULT_MAPPER.readValue(form.getLabelsJson(), HashMap.class); + HashMap labels = JsonUtil.DEFAULT_MAPPER.readValue(form.getLabelsJson(), HashMap.class); _customLabelProvider.saveLabels(labels, getContainer(), getUser()); } catch (Exception e) diff --git a/core/src/org/labkey/core/security/SecurityApiActions.java b/core/src/org/labkey/core/security/SecurityApiActions.java index cfd5aaa03b0..1e9990d888e 100644 --- a/core/src/org/labkey/core/security/SecurityApiActions.java +++ b/core/src/org/labkey/core/security/SecurityApiActions.java @@ -1089,7 +1089,7 @@ public ApiResponse execute(GroupForm form, BindException errors) { Container container = getContainer(); if (!container.isRoot() && !container.isProject()) - throw new IllegalArgumentException("You may not create groups at the folder level. Call this API at the project or root level."); + throw new IllegalArgumentException("You may not manage groups at the folder level. Call this API at the project or root level."); if (_group == null && getContainer().isRoot() && !getUser().hasRootPermission(UpdateUserPermission.class) ) { @@ -1558,6 +1558,9 @@ public ApiResponse execute(IdForm form, BindException errors) } } + /** + * Note: DeleteUserPermission is granted to Application Admins and above, so Project Admins can never delete a user + */ @RequiresPermission(DeleteUserPermission.class) public static class DeleteUserAction extends MutatingApiAction { @@ -1911,7 +1914,8 @@ else if (!c.hasPermission(getUser(), AddUserPermission.class)) } /** - * Invalidate existing password and send new password link + * Invalidate existing password and send new password link. Note: UpdateUserPermission is granted to Application + * Admins and above, so Project Admins can never rotate a user's password. */ @RequiresPermission(UpdateUserPermission.class) public static class AdminRotatePasswordAction extends MutatingApiAction diff --git a/wiki/src/org/labkey/wiki/WikiController.java b/wiki/src/org/labkey/wiki/WikiController.java index a48f322dd34..53e929dc104 100644 --- a/wiki/src/org/labkey/wiki/WikiController.java +++ b/wiki/src/org/labkey/wiki/WikiController.java @@ -1849,27 +1849,11 @@ public void setIsDeletingSubtree(boolean isDeletingSubtree) } } - public static class ContainerForm - { - private String _id; - - public String getId() - { - return _id; - } - - @SuppressWarnings({"UnusedDeclaration"}) - public void setId(String id) - { - _id = id; - } - } - @RequiresPermission(ReadPermission.class) - public static class GetPagesAction extends ReadOnlyApiAction + public static class GetPagesAction extends ReadOnlyApiAction { @Override - public ApiResponse execute(ContainerForm form, BindException errors) + public ApiResponse execute(Object o, BindException errors) { Map wikiMap = WikiSelectManager.getNameTitleMap(getContainer()); if (null == wikiMap) @@ -2532,21 +2516,7 @@ public ApiResponse execute(AttachFilesForm form, BindException errors) } } - public static class GetWikiTocForm - { - private String _currentPage = null; - - public String getCurrentPage() - { - return _currentPage; - } - - @SuppressWarnings({"UnusedDeclaration"}) - public void setCurrentPage(String currentPage) - { - _currentPage = currentPage; - } - } + public record GetWikiTocForm(String currentPage){} @RequiresPermission(ReadPermission.class) public static class GetWikiTocAction extends ReadOnlyApiAction @@ -2564,7 +2534,7 @@ public ApiResponse execute(GetWikiTocForm form, BindException errors) response.put("pages", pageProps); Set expandedPaths = NavTreeManager.getExpandedPathsCopy(getViewContext(), WikiTOC.getNavTreeId(getContainer())); - applyExpandedState(pageProps, expandedPaths, form.getCurrentPage()); + applyExpandedState(pageProps, expandedPaths, form.currentPage()); //include info about the current container Map containerProps = new HashMap<>(); @@ -2572,9 +2542,9 @@ public ApiResponse execute(GetWikiTocForm form, BindException errors) containerProps.put("id", container.getId()); containerProps.put("path", container.getPath()); - if (form.getCurrentPage() != null) + if (form.currentPage() != null) { - Wiki wiki = WikiSelectManager.getWiki(getContainer(), form.getCurrentPage()); + Wiki wiki = WikiSelectManager.getWiki(getContainer(), form.currentPage()); if (wiki != null) { WikiVersion version = wiki.getLatestVersion();