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..9468bdde7dc 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,7 @@ import java.util.List; import java.util.Map; import java.util.function.Predicate; +import java.util.stream.Collectors; public abstract class BaseViewAction extends PermissionCheckableAction implements Validator, HasPageConfig, ContainerUser { @@ -122,28 +124,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 +148,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 +165,6 @@ public PropertyValues getPropertyValues() return _pvs; } - public static PropertyValues getPropertyValuesForFormBinding(PropertyValues pvs, @NotNull Predicate allowBind) { if (null == pvs) @@ -184,7 +178,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 +211,6 @@ public ModelAndView handleRequest(@NotNull HttpServletRequest request, @NotNull return handleRequest(); } - private void handleSpecialProperties() { _robot = PageFlowUtil.isRobotUserAgent(getViewContext().getRequest().getHeader("User-Agent")); @@ -260,55 +252,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 +308,6 @@ protected Object newInstance(Class c) } } - protected @NotNull FORM getCommand(HttpServletRequest request) throws Exception { FORM command = (FORM) createCommand(); @@ -335,13 +318,11 @@ protected Object newInstance(Class c) return command; } - protected @NotNull FORM getCommand() throws Exception { return getCommand(getViewContext().getRequest()); } - // // PARAMETER BINDING // @@ -353,6 +334,32 @@ 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) : defaultBindParameters(getCommand(), params); + } + + // Simple binding for Java records: no support for binding errors, arrays, lists, etc. + public static BindException bindParametersToRecord(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"); + } public static @NotNull BindException defaultBindParameters(Object form, String commandName, PropertyValues params) { 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/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/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();