From 1209d6bf3293f1cf0e27c21fbaa89491f33f1905 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Fri, 24 Oct 2025 13:19:51 -0700 Subject: [PATCH 01/19] TableViewForm no longer implements DynaBean uses Map of String or String[] internally insteadof Map checkpoint... --- .../AnnouncementsController.java | 24 +- .../src/org/labkey/announcements/insert.jsp | 8 +- .../src/org/labkey/announcements/respond.jsp | 8 +- .../org/labkey/api/action/BaseViewAction.java | 1510 ++++++++--------- api/src/org/labkey/api/data/BeanViewForm.java | 8 +- api/src/org/labkey/api/data/DataColumn.java | 5 +- api/src/org/labkey/api/data/DataRegion.java | 2 +- .../org/labkey/api/data/DisplayColumn.java | 2 +- .../org/labkey/api/data/MVDisplayColumn.java | 2 +- .../labkey/api/data/StringBeanDynaClass.java | 14 +- .../org/labkey/api/data/TableViewForm.java | 177 +- .../api/data/TableWrapperDynaClass.java | 2 +- .../ParticipantVisitResolverChooser.java | 2 +- .../study/assay/FileLinkDisplayColumn.java | 2 +- .../api/study/assay/thawListSelector.jsp | 10 +- api/src/org/labkey/api/util/HtmlString.java | 1 + .../core/view/TableViewFormTestCase.java | 30 +- .../org/labkey/issue/IssuesController.java | 35 +- .../study/controllers/CohortController.java | 2 +- .../study/controllers/StudyController.java | 6 +- .../StudyPropertiesController.java | 2 +- 21 files changed, 935 insertions(+), 917 deletions(-) diff --git a/announcements/src/org/labkey/announcements/AnnouncementsController.java b/announcements/src/org/labkey/announcements/AnnouncementsController.java index 42637b9a1f2..8f32edc966d 100644 --- a/announcements/src/org/labkey/announcements/AnnouncementsController.java +++ b/announcements/src/org/labkey/announcements/AnnouncementsController.java @@ -1071,7 +1071,7 @@ public BaseInsertView(String page, InsertBean bean, AnnouncementForm form, URLHe if (reshow) { - currentRendererType = EnumUtils.getEnum(WikiRendererType.class, form.get("rendererType"), DEFAULT_MESSAGE_RENDERER_TYPE); + currentRendererType = EnumUtils.getEnum(WikiRendererType.class, form.getAsString("rendererType"), DEFAULT_MESSAGE_RENDERER_TYPE); AnnouncementModel ann = form.getBean(); assignedTo = ann.getAssignedTo(); @@ -1084,18 +1084,18 @@ else if (null == latestPost) cal.add(Calendar.MONTH, 1); String expires = DateUtil.formatDate(c, cal.getTime()); - form.set("expires", expires); + form.setStringToBind("expires", expires); currentRendererType = DEFAULT_MESSAGE_RENDERER_TYPE; assignedTo = settings.getDefaultAssignedTo(); } else { // Response... set values to match most recent properties on this thread - assert null == form.get("title"); - assert null == form.get("expires"); + assert null == form.getAsString("title"); + assert null == form.getAsString("expires"); - form.set("title", latestPost.getTitle()); - form.set("status", "Active"); // By default, every new response resets status to active, #35047 + form.setStringToBind("title", latestPost.getTitle()); + form.setStringToBind("status", "Active"); // By default, every new response resets status to active, #35047 form.setTypedValue("expires", DateUtil.formatDate(c, latestPost.getExpires())); assignedTo = latestPost.getAssignedTo(); @@ -1103,12 +1103,12 @@ else if (null == latestPost) } bean.assignedToSelect = getAssignedToSelect(c, assignedTo, "assignedTo", getViewContext().getUser()); - bean.statusSelect = getStatusSelect(form.get("status")); + bean.statusSelect = getStatusSelect(form.getAsString("status")); bean.renderAsSelect = getRenderAsSelect(currentRendererType); bean.settings = settings; User u = form.getUser() == null ? getViewContext().getUser() : form.getUser(); - bean.memberList = getMemberList(u, c, latestPost, reshow ? form.get("memberList") : null); + bean.memberList = getMemberList(u, c, latestPost, reshow ? form.getAsString("memberList") : null); bean.form = form; bean.cancelURL = cancelURL; @@ -1642,7 +1642,7 @@ public AnnouncementForm() // XXX: change return value to typed GuidString public String getParentId() { - return _stringValues.get("parentid"); + return getAsString("parentid"); } AnnouncementModel selectAnnouncement() @@ -1671,7 +1671,7 @@ public void validate(Errors errors) // Validate "expires" conversion from String to Date try { - String expires = StringUtils.trimToNull(get("expires")); + String expires = StringUtils.trimToNull(getAsString("expires")); if (null != expires) DateUtil.parseDateTime(expires); } @@ -2270,7 +2270,7 @@ public ThreadView(Container c, ActionURL url, AnnouncementModel ann, Permissions public ThreadView(AnnouncementForm form, Container c, ActionURL url, Permissions perm, boolean print) { this(); - AnnouncementModel ann = findThread(c, form.get("rowId"), form.get("entityId")); + AnnouncementModel ann = findThread(c, form.getAsString("rowId"), form.getAsString("entityId")); init(c, ann, url, perm, false, print); } @@ -2514,7 +2514,7 @@ public class UpdateBean private UpdateBean(AnnouncementForm form, AnnouncementModel ann) { Container c = form.getContainer(); - String reshowMemberList = form.get("memberList"); + String reshowMemberList = form.getAsString("memberList"); annModel = ann; settings = getSettings(c); diff --git a/announcements/src/org/labkey/announcements/insert.jsp b/announcements/src/org/labkey/announcements/insert.jsp index 0c206146d6f..17b6c270e6b 100644 --- a/announcements/src/org/labkey/announcements/insert.jsp +++ b/announcements/src/org/labkey/announcements/insert.jsp @@ -70,7 +70,7 @@ %>Note: This <%=h(settings.getConversationName().toLowerCase())%> will not be posted immediately; it will appear after the content has been reviewed.

<% } %> - Title * <%= helpPopup("Title", "This field is required.") %> + Title * <%= helpPopup("Title", "This field is required.") %> <% if (settings.hasStatus()) { @@ -101,7 +101,7 @@ } if (settings.hasExpires()) { - %>ExpiresBy default the Expires field is set to one month from today.
Expired messages are not deleted, they are just no longer shown on the Portal page.<% + %>ExpiresBy default the Expires field is set to one month from today.
Expired messages are not deleted, they are just no longer shown on the Portal page.<% } %> @@ -119,7 +119,7 @@
<% addHandler("body", "change", "LABKEY.setDirty(true);"); %> - +
@@ -165,7 +165,7 @@ else %><%= generateBackButton("Cancel") %><% } %> - +

<% diff --git a/announcements/src/org/labkey/announcements/respond.jsp b/announcements/src/org/labkey/announcements/respond.jsp index 43df10ae78e..4eb1b0345ef 100644 --- a/announcements/src/org/labkey/announcements/respond.jsp +++ b/announcements/src/org/labkey/announcements/respond.jsp @@ -75,11 +75,11 @@ if (!mr.isApproved(c, user, false /* Not a new thread */)) if (settings.isTitleEditable()) { - %>Title * <%=helpPopup("Title", "This field is required.") %><% + %>Title * <%=helpPopup("Title", "This field is required.") %><% } else { - %><% + %><% } if (settings.hasStatus()) @@ -111,7 +111,7 @@ if (settings.hasMemberList()) if (settings.hasExpires()) { - %>ExpiresExpired messages are not deleted, they are just no longer shown on the Portal page.<% + %>ExpiresExpired messages are not deleted, they are just no longer shown on the Portal page.<% } %> @@ -129,7 +129,7 @@ if (settings.hasExpires())

<% addHandler("body", "change", "LABKEY.setDirty(true);"); %> - +
diff --git a/api/src/org/labkey/api/action/BaseViewAction.java b/api/src/org/labkey/api/action/BaseViewAction.java index d31dda91433..130f9a8c483 100644 --- a/api/src/org/labkey/api/action/BaseViewAction.java +++ b/api/src/org/labkey/api/action/BaseViewAction.java @@ -1,755 +1,755 @@ -/* - * Copyright (c) 2008-2019 LabKey Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.labkey.api.action; - -import jakarta.servlet.ServletRequest; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import org.apache.commons.beanutils.ConversionException; -import org.apache.commons.beanutils.ConvertUtils; -import org.apache.commons.beanutils.DynaBean; -import org.apache.commons.beanutils.PropertyUtils; -import org.apache.commons.lang3.StringUtils; -import org.apache.logging.log4j.Logger; -import org.jetbrains.annotations.NotNull; -import org.labkey.api.attachments.AttachmentFile; -import org.labkey.api.attachments.SpringAttachmentFile; -import org.labkey.api.data.Container; -import org.labkey.api.data.ConvertHelper; -import org.labkey.api.security.User; -import org.labkey.api.util.HelpTopic; -import org.labkey.api.util.PageFlowUtil; -import org.labkey.api.util.logging.LogHelper; -import org.labkey.api.view.HttpView; -import org.labkey.api.view.template.PageConfig; -import org.labkey.api.writer.ContainerUser; -import org.springframework.beans.AbstractPropertyAccessor; -import org.springframework.beans.BeanUtils; -import org.springframework.beans.BeanWrapper; -import org.springframework.beans.BeansException; -import org.springframework.beans.InvalidPropertyException; -import org.springframework.beans.MutablePropertyValues; -import org.springframework.beans.NotReadablePropertyException; -import org.springframework.beans.NotWritablePropertyException; -import org.springframework.beans.PropertyAccessException; -import org.springframework.beans.PropertyValue; -import org.springframework.beans.PropertyValues; -import org.springframework.beans.TypeMismatchException; -import org.springframework.core.MethodParameter; -import org.springframework.core.convert.TypeDescriptor; -import org.springframework.lang.Nullable; -import org.springframework.validation.BeanPropertyBindingResult; -import org.springframework.validation.BindException; -import org.springframework.validation.BindingErrorProcessor; -import org.springframework.validation.BindingResult; -import org.springframework.validation.FieldError; -import org.springframework.validation.ObjectError; -import org.springframework.validation.Validator; -import org.springframework.web.bind.ServletRequestDataBinder; -import org.springframework.web.bind.ServletRequestParameterPropertyValues; -import org.springframework.web.multipart.MultipartFile; -import org.springframework.web.multipart.MultipartHttpServletRequest; -import org.springframework.web.servlet.ModelAndView; - -import java.beans.PropertyDescriptor; -import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.function.Predicate; - -public abstract class BaseViewAction
extends PermissionCheckableAction implements Validator, HasPageConfig, ContainerUser -{ - protected static final Logger logger = LogHelper.getLogger(BaseViewAction.class, "BaseViewAction"); - - private PageConfig _pageConfig = null; - private PropertyValues _pvs; - private boolean _robot = false; // Is this request from GoogleBot or some other crawler? - private boolean _debug = false; - - protected boolean _print = false; - protected Class _commandClass; - protected String _commandName = "form"; - - protected BaseViewAction() - { - String methodName = getCommandClassMethodName(); - - if (null == methodName) - return; - - // inspect the action's *public* methods to determine form class - Class typeBest = null; - for (Method m : this.getClass().getMethods()) - { - if (methodName.equals(m.getName())) - { - Class[] types = m.getParameterTypes(); - if (types.length < 1) - continue; - Class typeCurrent = types[0]; - assert null == _commandClass || typeCurrent.equals(_commandClass); - - // Using templated classes to extend a base action can lead to multiple - // versions of a method with acceptable types, so take the most extended - // type we can find. - if (typeBest == null || typeBest.isAssignableFrom(typeCurrent)) - typeBest = typeCurrent; - } - } - if (typeBest != null) - 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) - { - PropertyValue pv = _pvs.getPropertyValue(key); - 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); - return pv == null ? null : pv.getValue(); - } - - public PropertyValues getPropertyValues() - { - return _pvs; - } - - - public static PropertyValues getPropertyValuesForFormBinding(PropertyValues pvs, @NotNull Predicate allowBind) - { - if (null == pvs) - return null; - MutablePropertyValues ret = new MutablePropertyValues(); - for (PropertyValue pv : pvs.getPropertyValues()) - { - if (allowBind.test(pv.getName())) - ret.addPropertyValue(pv); - } - return ret; - } - - static final String FORM_DATE_ENCODED_PARAM = "formDataEncoded"; - - /** - * When a double quote is encountered in a multipart/form-data context, it is encoded as %22 using URL-encoding by browsers. - * This process replaces the double quote with its hexadecimal equivalent in a URL-safe format, preventing it from being misinterpreted as the end of a value or a boundary. - * The consequence of such encoding is we can't distinguish '"' from the actual '%22' in parameter name. - * As a workaround, a client-side util `encodeFormDataQuote` is used to convert %22 to %2522 and " to %22 explicitly, while passing in an additional param formDataEncoded=true. - * This class converts those encoded param names back to its decoded form during PropertyValues binding. - * See Issue 52827, 52925 and 52119 for more information. - */ - static public class ViewActionParameterPropertyValues extends ServletRequestParameterPropertyValues - { - - public ViewActionParameterPropertyValues(ServletRequest request) { - this(request, null, null); - } - - public ViewActionParameterPropertyValues(ServletRequest request, @Nullable String prefix, @Nullable String prefixSeparator) - { - super(request, prefix, prefixSeparator); - if (isFormDataEncoded()) - { - for (int i = 0; i < getPropertyValues().length; i++) - { - PropertyValue formDataPropValue = getPropertyValues()[i]; - String propValueName = formDataPropValue.getName(); - String decoded = PageFlowUtil.decodeQuoteEncodedFormDataKey(propValueName); - if (!propValueName.equals(decoded)) - setPropertyValueAt(new PropertyValue(decoded, formDataPropValue.getValue()), i); - } - } - } - - private boolean isFormDataEncoded() - { - PropertyValue formDataPropValue = getPropertyValue(FORM_DATE_ENCODED_PARAM); - if (formDataPropValue != null) - { - Object v = formDataPropValue.getValue(); - String formDataPropValueStr = v == null ? null : String.valueOf(v); - if (StringUtils.isNotBlank(formDataPropValueStr)) - return (Boolean) ConvertUtils.convert(formDataPropValueStr, Boolean.class); - } - - return false; - } - } - - @Override - public ModelAndView handleRequest(@NotNull HttpServletRequest request, @NotNull HttpServletResponse response) throws Exception - { - if (null == getPropertyValues()) - setProperties(new ViewActionParameterPropertyValues(request)); - getViewContext().setBindPropertyValues(getPropertyValues()); - handleSpecialProperties(); - - return handleRequest(); - } - - - private void handleSpecialProperties() - { - _robot = PageFlowUtil.isRobotUserAgent(getViewContext().getRequest().getHeader("User-Agent")); - - // Special flag puts actions in "debug" mode, during which they should log extra information that would be - // helpful for testing or debugging problems - if (!_robot && hasStringValue("_debug")) - { - _debug = true; - } - - // SpringActionController.defaultPageConfig() has logic for isPrint, don't need to duplicate here - _print = PageConfig.Template.Print == HttpView.currentPageConfig().getTemplate(); - } - - private boolean hasStringValue(String propertyName) - { - Object o = getProperty(propertyName); - if (o == null) - { - return false; - } - if (o instanceof String s) - { - return null != StringUtils.trimToNull(s); - } - if (o instanceof String[] strings) - { - for (String s : strings) - { - if (null != StringUtils.trimToNull(s)) - { - return true; - } - } - } - return false; - } - - 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 - { - return c == null ? null : c.getConstructor().newInstance(); - } - catch (Exception x) - { - if (x instanceof RuntimeException) - throw ((RuntimeException)x); - else - throw new RuntimeException(x); - } - } - - - protected @NotNull FORM getCommand(HttpServletRequest request) throws Exception - { - FORM command = (FORM) createCommand(); - - if (command instanceof HasViewContext) - ((HasViewContext)command).setViewContext(getViewContext()); - - return command; - } - - - protected @NotNull FORM getCommand() throws Exception - { - return getCommand(getViewContext().getRequest()); - } - - - // - // PARAMETER BINDING - // - // don't assume parameters always come from a request, use PropertyValues interface - // - - public @NotNull BindException defaultBindParameters(FORM form, PropertyValues params) - { - return defaultBindParameters(form, getCommandName(), params); - } - - - public static @NotNull BindException defaultBindParameters(Object form, String commandName, PropertyValues params) - { - /* check for do-it-myself forms */ - if (form instanceof HasBindParameters) - { - return ((HasBindParameters)form).bindParameters(params); - } - - if (form instanceof DynaBean) - { - return simpleBindParameters(form, commandName, params); - } - else - { - return springBindParameters(form, commandName, params); - } - } - - public static @NotNull BindException springBindParameters(Object command, String commandName, PropertyValues params) - { - Predicate allow = command instanceof HasAllowBindParameter allowBP ? allowBP.allowBindParameter() : HasAllowBindParameter.getDefaultPredicate(); - ServletRequestDataBinder binder = new ServletRequestDataBinder(command, commandName); - - String[] fields = binder.getDisallowedFields(); - List fieldList = new ArrayList<>(fields != null ? Arrays.asList(fields) : Collections.emptyList()); - fieldList.addAll(Arrays.asList("class.*", "Class.*", "*.class.*", "*.Class.*")); - binder.setDisallowedFields(fieldList.toArray(new String[] {})); - - ConvertHelper.getPropertyEditorRegistrar().registerCustomEditors(binder); - BindingErrorProcessor defaultBEP = binder.getBindingErrorProcessor(); - binder.setBindingErrorProcessor(getBindingErrorProcessor(defaultBEP)); - binder.setFieldMarkerPrefix(SpringActionController.FIELD_MARKER); - try - { - // most paths probably called getPropertyValuesForFormBinding() already, but this is a public static method, so call it again - binder.bind(getPropertyValuesForFormBinding(params, allow)); - BindException errors = new NullSafeBindException(binder.getBindingResult()); - return errors; - } - catch (InvalidPropertyException x) - { - // 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()); - 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() + ")"); - 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() + ")"); - 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() + ")"); - return errors; - } - } - - - static BindingErrorProcessor getBindingErrorProcessor(final BindingErrorProcessor defaultBEP) - { - return new BindingErrorProcessor() - { - @Override - public void processMissingFieldError(String missingField, BindingResult bindingResult) - { - defaultBEP.processMissingFieldError(missingField, bindingResult); - } - - @Override - public void processPropertyAccessException(PropertyAccessException ex, BindingResult bindingResult) - { - Object newValue = ex.getPropertyChangeEvent().getNewValue(); - if (newValue instanceof String) - newValue = StringUtils.trimToNull((String)newValue); - - // convert NULL conversion errors to required errors - if (null == newValue) - defaultBEP.processMissingFieldError(ex.getPropertyChangeEvent().getPropertyName(), bindingResult); - else - defaultBEP.processPropertyAccessException(ex, bindingResult); - } - }; - } - - - /* - * This binder doesn't have much to offer over the standard spring data binding except that it will - * handle DynaBeans. - */ - public static @NotNull BindException simpleBindParameters(Object command, String commandName, PropertyValues params) - { - Predicate allow = command instanceof HasAllowBindParameter allowBP ? allowBP.allowBindParameter() : HasAllowBindParameter.getDefaultPredicate(); - - BindException errors = new NullSafeBindException(command, "Form"); - - // unfortunately ObjectFactory and BeanObjectFactory are not good about reporting errors - // do this by hand - for (PropertyValue pv : params.getPropertyValues()) - { - String propertyName = pv.getName(); - Object value = pv.getValue(); - if (!allow.test(propertyName)) - continue; - - try - { - Object converted = value; - Class propClass = PropertyUtils.getPropertyType(command, propertyName); - if (null == propClass) - continue; - if (value == null) - { - /* */ - } - else if (propClass.isPrimitive()) - { - converted = ConvertUtils.convert(String.valueOf(value), propClass); - } - else if (propClass.isArray()) - { - if (value instanceof Collection) - value = ((Collection) value).toArray(new String[0]); - else if (!value.getClass().isArray()) - value = new String[] {String.valueOf(value)}; - converted = ConvertUtils.convert((String[])value, propClass); - } - PropertyUtils.setProperty(command, propertyName, converted); - } - catch (ConversionException x) - { - errors.addError(new FieldError(commandName, propertyName, value, true, new String[] {"ConversionError", "typeMismatch"}, null, "Could not convert to value: " + value)); - } - catch (Exception x) - { - errors.addError(new ObjectError(commandName, new String[]{"Error"}, new Object[] {value}, x.getMessage())); - logger.error("unexpected error", x); - } - } - return errors; - } - - @Override - public boolean supports(Class clazz) - { - return getCommandClass().isAssignableFrom(clazz); - } - - - /* for TableViewForm, uses BeanUtils to work with DynaBeans */ - static public class BeanUtilsPropertyBindingResult extends BeanPropertyBindingResult - { - public BeanUtilsPropertyBindingResult(Object target, String objectName) - { - super(target, objectName); - } - - @Override - protected BeanWrapper createBeanWrapper() - { - return new BeanUtilsWrapperImpl((DynaBean)getTarget()); - } - } - - static public class BeanUtilsWrapperImpl extends AbstractPropertyAccessor implements BeanWrapper - { - private Object object; - private boolean autoGrowNestedPaths = false; - private int autoGrowCollectionLimit = 0; - - public BeanUtilsWrapperImpl() - { - // registerDefaultEditors(); - } - - public BeanUtilsWrapperImpl(DynaBean target) - { - this(); - object = target; - } - - @Override - public Object getPropertyValue(String propertyName) throws BeansException - { - try - { - return PropertyUtils.getProperty(object, propertyName); - } - catch (Exception e) - { - throw new NotReadablePropertyException(object.getClass(), propertyName); - } - } - - @Override - public void setPropertyValue(String propertyName, Object value) throws BeansException - { - try - { - PropertyUtils.setProperty(object, propertyName, value); - } - catch (Exception e) - { - throw new NotWritablePropertyException(object.getClass(), propertyName); - } - } - - @Override - public boolean isReadableProperty(String propertyName) - { - return true; - } - - @Override - public boolean isWritableProperty(String propertyName) - { - return true; - } - - @Override - public TypeDescriptor getPropertyTypeDescriptor(String s) throws BeansException - { - return null; - } - - public void setWrappedInstance(Object obj) - { - object = obj; - } - - @Override - public Object getWrappedInstance() - { - return object; - } - - @Override - public Class getWrappedClass() - { - return object.getClass(); - } - - @Override - public PropertyDescriptor[] getPropertyDescriptors() - { - throw new UnsupportedOperationException(); - } - - @Override - public PropertyDescriptor getPropertyDescriptor(String propertyName) throws BeansException - { - throw new UnsupportedOperationException(); - } - - @Override - public void setAutoGrowNestedPaths(boolean b) - { - this.autoGrowNestedPaths = b; - } - - @Override - public boolean isAutoGrowNestedPaths() - { - return this.autoGrowNestedPaths; - } - - @Override - public void setAutoGrowCollectionLimit(int i) - { - this.autoGrowCollectionLimit = i; - } - - @Override - public int getAutoGrowCollectionLimit() - { - return this.autoGrowCollectionLimit; - } - - @Override - public T convertIfNecessary(Object value, Class requiredType) throws TypeMismatchException - { - if (value == null) - return null; - return (T)ConvertUtils.convert(String.valueOf(value), requiredType); - } - - @Override - public T convertIfNecessary(Object value, Class requiredType, MethodParameter methodParam) throws TypeMismatchException - { - return convertIfNecessary(value, requiredType); - } - } - - /** - * @return a map from form element name to uploaded files - */ - protected Map getFileMap() - { - if (getViewContext().getRequest() instanceof MultipartHttpServletRequest) - return ((MultipartHttpServletRequest)getViewContext().getRequest()).getFileMap(); - return Collections.emptyMap(); - } - - protected List getAttachmentFileList() - { - return SpringAttachmentFile.createList(getFileMap()); - } - - public boolean isRobot() - { - return _robot; - } - - public boolean isPrint() - { - return _print; - } - - public boolean isDebug() - { - return _debug; - } - - public @NotNull Class getCommandClass() - { - if (null == _commandClass) - throw new IllegalStateException("NULL _commandClass in " + getClass().getName()); - return _commandClass; - } - - public void setCommandClass(@NotNull Class commandClass) - { - _commandClass = commandClass; - } - - protected final @NotNull Object createCommand() - { - return BeanUtils.instantiateClass(getCommandClass()); - } - - public void setCommandName(String commandName) - { - _commandName = commandName; - } - - public String getCommandName() - { - return _commandName; - } - - /** - * Cacheable resources can calculate a last modified timestamp to send to the browser. - */ - protected long getLastModified(FORM form) - { - return Long.MIN_VALUE; - } - - /** - * Cacheable resources can calculate an ETag header to send to the browser. - */ - protected String getETag(FORM form) - { - return null; - } -} +/* + * Copyright (c) 2008-2019 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.labkey.api.action; + +import jakarta.servlet.ServletRequest; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.apache.commons.beanutils.ConversionException; +import org.apache.commons.beanutils.ConvertUtils; +import org.apache.commons.beanutils.DynaBean; +import org.apache.commons.beanutils.PropertyUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.NotNull; +import org.labkey.api.attachments.AttachmentFile; +import org.labkey.api.attachments.SpringAttachmentFile; +import org.labkey.api.data.Container; +import org.labkey.api.data.ConvertHelper; +import org.labkey.api.security.User; +import org.labkey.api.util.HelpTopic; +import org.labkey.api.util.PageFlowUtil; +import org.labkey.api.util.logging.LogHelper; +import org.labkey.api.view.HttpView; +import org.labkey.api.view.template.PageConfig; +import org.labkey.api.writer.ContainerUser; +import org.springframework.beans.AbstractPropertyAccessor; +import org.springframework.beans.BeanUtils; +import org.springframework.beans.BeanWrapper; +import org.springframework.beans.BeansException; +import org.springframework.beans.InvalidPropertyException; +import org.springframework.beans.MutablePropertyValues; +import org.springframework.beans.NotReadablePropertyException; +import org.springframework.beans.NotWritablePropertyException; +import org.springframework.beans.PropertyAccessException; +import org.springframework.beans.PropertyValue; +import org.springframework.beans.PropertyValues; +import org.springframework.beans.TypeMismatchException; +import org.springframework.core.MethodParameter; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.lang.Nullable; +import org.springframework.validation.BeanPropertyBindingResult; +import org.springframework.validation.BindException; +import org.springframework.validation.BindingErrorProcessor; +import org.springframework.validation.BindingResult; +import org.springframework.validation.FieldError; +import org.springframework.validation.ObjectError; +import org.springframework.validation.Validator; +import org.springframework.web.bind.ServletRequestDataBinder; +import org.springframework.web.bind.ServletRequestParameterPropertyValues; +import org.springframework.web.multipart.MultipartFile; +import org.springframework.web.multipart.MultipartHttpServletRequest; +import org.springframework.web.servlet.ModelAndView; + +import java.beans.PropertyDescriptor; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.function.Predicate; + +public abstract class BaseViewAction extends PermissionCheckableAction implements Validator, HasPageConfig, ContainerUser +{ + protected static final Logger logger = LogHelper.getLogger(BaseViewAction.class, "BaseViewAction"); + + private PageConfig _pageConfig = null; + private PropertyValues _pvs; + private boolean _robot = false; // Is this request from GoogleBot or some other crawler? + private boolean _debug = false; + + protected boolean _print = false; + protected Class _commandClass; + protected String _commandName = "form"; + + protected BaseViewAction() + { + String methodName = getCommandClassMethodName(); + + if (null == methodName) + return; + + // inspect the action's *public* methods to determine form class + Class typeBest = null; + for (Method m : this.getClass().getMethods()) + { + if (methodName.equals(m.getName())) + { + Class[] types = m.getParameterTypes(); + if (types.length < 1) + continue; + Class typeCurrent = types[0]; + assert null == _commandClass || typeCurrent.equals(_commandClass); + + // Using templated classes to extend a base action can lead to multiple + // versions of a method with acceptable types, so take the most extended + // type we can find. + if (typeBest == null || typeBest.isAssignableFrom(typeCurrent)) + typeBest = typeCurrent; + } + } + if (typeBest != null) + 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) + { + PropertyValue pv = _pvs.getPropertyValue(key); + 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); + return pv == null ? null : pv.getValue(); + } + + public PropertyValues getPropertyValues() + { + return _pvs; + } + + + public static PropertyValues getPropertyValuesForFormBinding(PropertyValues pvs, @NotNull Predicate allowBind) + { + if (null == pvs) + return null; + MutablePropertyValues ret = new MutablePropertyValues(); + for (PropertyValue pv : pvs.getPropertyValues()) + { + if (allowBind.test(pv.getName())) + ret.addPropertyValue(pv); + } + return ret; + } + + static final String FORM_DATE_ENCODED_PARAM = "formDataEncoded"; + + /** + * When a double quote is encountered in a multipart/form-data context, it is encoded as %22 using URL-encoding by browsers. + * This process replaces the double quote with its hexadecimal equivalent in a URL-safe format, preventing it from being misinterpreted as the end of a value or a boundary. + * The consequence of such encoding is we can't distinguish '"' from the actual '%22' in parameter name. + * As a workaround, a client-side util `encodeFormDataQuote` is used to convert %22 to %2522 and " to %22 explicitly, while passing in an additional param formDataEncoded=true. + * This class converts those encoded param names back to its decoded form during PropertyValues binding. + * See Issue 52827, 52925 and 52119 for more information. + */ + static public class ViewActionParameterPropertyValues extends ServletRequestParameterPropertyValues + { + + public ViewActionParameterPropertyValues(ServletRequest request) { + this(request, null, null); + } + + public ViewActionParameterPropertyValues(ServletRequest request, @Nullable String prefix, @Nullable String prefixSeparator) + { + super(request, prefix, prefixSeparator); + if (isFormDataEncoded()) + { + for (int i = 0; i < getPropertyValues().length; i++) + { + PropertyValue formDataPropValue = getPropertyValues()[i]; + String propValueName = formDataPropValue.getName(); + String decoded = PageFlowUtil.decodeQuoteEncodedFormDataKey(propValueName); + if (!propValueName.equals(decoded)) + setPropertyValueAt(new PropertyValue(decoded, formDataPropValue.getValue()), i); + } + } + } + + private boolean isFormDataEncoded() + { + PropertyValue formDataPropValue = getPropertyValue(FORM_DATE_ENCODED_PARAM); + if (formDataPropValue != null) + { + Object v = formDataPropValue.getValue(); + String formDataPropValueStr = v == null ? null : String.valueOf(v); + if (StringUtils.isNotBlank(formDataPropValueStr)) + return (Boolean) ConvertUtils.convert(formDataPropValueStr, Boolean.class); + } + + return false; + } + } + + @Override + public ModelAndView handleRequest(@NotNull HttpServletRequest request, @NotNull HttpServletResponse response) throws Exception + { + if (null == getPropertyValues()) + setProperties(new ViewActionParameterPropertyValues(request)); + getViewContext().setBindPropertyValues(getPropertyValues()); + handleSpecialProperties(); + + return handleRequest(); + } + + + private void handleSpecialProperties() + { + _robot = PageFlowUtil.isRobotUserAgent(getViewContext().getRequest().getHeader("User-Agent")); + + // Special flag puts actions in "debug" mode, during which they should log extra information that would be + // helpful for testing or debugging problems + if (!_robot && hasStringValue("_debug")) + { + _debug = true; + } + + // SpringActionController.defaultPageConfig() has logic for isPrint, don't need to duplicate here + _print = PageConfig.Template.Print == HttpView.currentPageConfig().getTemplate(); + } + + private boolean hasStringValue(String propertyName) + { + Object o = getProperty(propertyName); + if (o == null) + { + return false; + } + if (o instanceof String s) + { + return !StringUtils.isBlank(s); + } + if (o instanceof String[] strings) + { + for (String s : strings) + { + if (!StringUtils.isBlank(s)) + { + return true; + } + } + } + return false; + } + + 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 + { + return c == null ? null : c.getConstructor().newInstance(); + } + catch (Exception x) + { + if (x instanceof RuntimeException) + throw ((RuntimeException)x); + else + throw new RuntimeException(x); + } + } + + + protected @NotNull FORM getCommand(HttpServletRequest request) throws Exception + { + FORM command = (FORM) createCommand(); + + if (command instanceof HasViewContext) + ((HasViewContext)command).setViewContext(getViewContext()); + + return command; + } + + + protected @NotNull FORM getCommand() throws Exception + { + return getCommand(getViewContext().getRequest()); + } + + + // + // PARAMETER BINDING + // + // don't assume parameters always come from a request, use PropertyValues interface + // + + public @NotNull BindException defaultBindParameters(FORM form, PropertyValues params) + { + return defaultBindParameters(form, getCommandName(), params); + } + + + public static @NotNull BindException defaultBindParameters(Object form, String commandName, PropertyValues params) + { + /* check for do-it-myself forms */ + if (form instanceof HasBindParameters) + { + return ((HasBindParameters)form).bindParameters(params); + } + + if (form instanceof DynaBean) + { + return simpleBindParameters(form, commandName, params); + } + else + { + return springBindParameters(form, commandName, params); + } + } + + public static @NotNull BindException springBindParameters(Object command, String commandName, PropertyValues params) + { + Predicate allow = command instanceof HasAllowBindParameter allowBP ? allowBP.allowBindParameter() : HasAllowBindParameter.getDefaultPredicate(); + ServletRequestDataBinder binder = new ServletRequestDataBinder(command, commandName); + + String[] fields = binder.getDisallowedFields(); + List fieldList = new ArrayList<>(fields != null ? Arrays.asList(fields) : Collections.emptyList()); + fieldList.addAll(Arrays.asList("class.*", "Class.*", "*.class.*", "*.Class.*")); + binder.setDisallowedFields(fieldList.toArray(new String[] {})); + + ConvertHelper.getPropertyEditorRegistrar().registerCustomEditors(binder); + BindingErrorProcessor defaultBEP = binder.getBindingErrorProcessor(); + binder.setBindingErrorProcessor(getBindingErrorProcessor(defaultBEP)); + binder.setFieldMarkerPrefix(SpringActionController.FIELD_MARKER); + try + { + // most paths probably called getPropertyValuesForFormBinding() already, but this is a public static method, so call it again + binder.bind(getPropertyValuesForFormBinding(params, allow)); + BindException errors = new NullSafeBindException(binder.getBindingResult()); + return errors; + } + catch (InvalidPropertyException x) + { + // 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()); + 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() + ")"); + 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() + ")"); + 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() + ")"); + return errors; + } + } + + + static BindingErrorProcessor getBindingErrorProcessor(final BindingErrorProcessor defaultBEP) + { + return new BindingErrorProcessor() + { + @Override + public void processMissingFieldError(String missingField, BindingResult bindingResult) + { + defaultBEP.processMissingFieldError(missingField, bindingResult); + } + + @Override + public void processPropertyAccessException(PropertyAccessException ex, BindingResult bindingResult) + { + Object newValue = ex.getPropertyChangeEvent().getNewValue(); + if (newValue instanceof String) + newValue = StringUtils.trimToNull((String)newValue); + + // convert NULL conversion errors to required errors + if (null == newValue) + defaultBEP.processMissingFieldError(ex.getPropertyChangeEvent().getPropertyName(), bindingResult); + else + defaultBEP.processPropertyAccessException(ex, bindingResult); + } + }; + } + + + /* + * This binder doesn't have much to offer over the standard spring data binding except that it will + * handle DynaBeans. + */ + public static @NotNull BindException simpleBindParameters(Object command, String commandName, PropertyValues params) + { + Predicate allow = command instanceof HasAllowBindParameter allowBP ? allowBP.allowBindParameter() : HasAllowBindParameter.getDefaultPredicate(); + + BindException errors = new NullSafeBindException(command, "Form"); + + // unfortunately ObjectFactory and BeanObjectFactory are not good about reporting errors + // do this by hand + for (PropertyValue pv : params.getPropertyValues()) + { + String propertyName = pv.getName(); + Object value = pv.getValue(); + if (!allow.test(propertyName)) + continue; + + try + { + Object converted = value; + Class propClass = PropertyUtils.getPropertyType(command, propertyName); + if (null == propClass) + continue; + if (value == null) + { + /* */ + } + else if (propClass.isPrimitive()) + { + converted = ConvertUtils.convert(String.valueOf(value), propClass); + } + else if (propClass.isArray()) + { + if (value instanceof Collection) + value = ((Collection) value).toArray(new String[0]); + else if (!value.getClass().isArray()) + value = new String[] {String.valueOf(value)}; + converted = ConvertUtils.convert((String[])value, propClass); + } + PropertyUtils.setProperty(command, propertyName, converted); + } + catch (ConversionException x) + { + errors.addError(new FieldError(commandName, propertyName, value, true, new String[] {"ConversionError", "typeMismatch"}, null, "Could not convert to value: " + value)); + } + catch (Exception x) + { + errors.addError(new ObjectError(commandName, new String[]{"Error"}, new Object[] {value}, x.getMessage())); + logger.error("unexpected error", x); + } + } + return errors; + } + + @Override + public boolean supports(Class clazz) + { + return getCommandClass().isAssignableFrom(clazz); + } + + + /* for TableViewForm, uses BeanUtils to work with DynaBeans */ + static public class BeanUtilsPropertyBindingResult extends BeanPropertyBindingResult + { + public BeanUtilsPropertyBindingResult(Object target, String objectName) + { + super(target, objectName); + } + + @Override + protected BeanWrapper createBeanWrapper() + { + return new BeanUtilsWrapperImpl((DynaBean)getTarget()); + } + } + + static public class BeanUtilsWrapperImpl extends AbstractPropertyAccessor implements BeanWrapper + { + private Object object; + private boolean autoGrowNestedPaths = false; + private int autoGrowCollectionLimit = 0; + + public BeanUtilsWrapperImpl() + { + // registerDefaultEditors(); + } + + public BeanUtilsWrapperImpl(DynaBean target) + { + this(); + object = target; + } + + @Override + public Object getPropertyValue(String propertyName) throws BeansException + { + try + { + return PropertyUtils.getProperty(object, propertyName); + } + catch (Exception e) + { + throw new NotReadablePropertyException(object.getClass(), propertyName); + } + } + + @Override + public void setPropertyValue(String propertyName, Object value) throws BeansException + { + try + { + PropertyUtils.setProperty(object, propertyName, value); + } + catch (Exception e) + { + throw new NotWritablePropertyException(object.getClass(), propertyName); + } + } + + @Override + public boolean isReadableProperty(String propertyName) + { + return true; + } + + @Override + public boolean isWritableProperty(String propertyName) + { + return true; + } + + @Override + public TypeDescriptor getPropertyTypeDescriptor(String s) throws BeansException + { + return null; + } + + public void setWrappedInstance(Object obj) + { + object = obj; + } + + @Override + public Object getWrappedInstance() + { + return object; + } + + @Override + public Class getWrappedClass() + { + return object.getClass(); + } + + @Override + public PropertyDescriptor[] getPropertyDescriptors() + { + throw new UnsupportedOperationException(); + } + + @Override + public PropertyDescriptor getPropertyDescriptor(String propertyName) throws BeansException + { + throw new UnsupportedOperationException(); + } + + @Override + public void setAutoGrowNestedPaths(boolean b) + { + this.autoGrowNestedPaths = b; + } + + @Override + public boolean isAutoGrowNestedPaths() + { + return this.autoGrowNestedPaths; + } + + @Override + public void setAutoGrowCollectionLimit(int i) + { + this.autoGrowCollectionLimit = i; + } + + @Override + public int getAutoGrowCollectionLimit() + { + return this.autoGrowCollectionLimit; + } + + @Override + public T convertIfNecessary(Object value, Class requiredType) throws TypeMismatchException + { + if (value == null) + return null; + return (T)ConvertUtils.convert(String.valueOf(value), requiredType); + } + + @Override + public T convertIfNecessary(Object value, Class requiredType, MethodParameter methodParam) throws TypeMismatchException + { + return convertIfNecessary(value, requiredType); + } + } + + /** + * @return a map from form element name to uploaded files + */ + protected Map getFileMap() + { + if (getViewContext().getRequest() instanceof MultipartHttpServletRequest) + return ((MultipartHttpServletRequest)getViewContext().getRequest()).getFileMap(); + return Collections.emptyMap(); + } + + protected List getAttachmentFileList() + { + return SpringAttachmentFile.createList(getFileMap()); + } + + public boolean isRobot() + { + return _robot; + } + + public boolean isPrint() + { + return _print; + } + + public boolean isDebug() + { + return _debug; + } + + public @NotNull Class getCommandClass() + { + if (null == _commandClass) + throw new IllegalStateException("NULL _commandClass in " + getClass().getName()); + return _commandClass; + } + + public void setCommandClass(@NotNull Class commandClass) + { + _commandClass = commandClass; + } + + protected final @NotNull Object createCommand() + { + return BeanUtils.instantiateClass(getCommandClass()); + } + + public void setCommandName(String commandName) + { + _commandName = commandName; + } + + public String getCommandName() + { + return _commandName; + } + + /** + * Cacheable resources can calculate a last modified timestamp to send to the browser. + */ + protected long getLastModified(FORM form) + { + return Long.MIN_VALUE; + } + + /** + * Cacheable resources can calculate an ETag header to send to the browser. + */ + protected String getETag(FORM form) + { + return null; + } +} diff --git a/api/src/org/labkey/api/data/BeanViewForm.java b/api/src/org/labkey/api/data/BeanViewForm.java index c3b7c1b6b0a..40f817f1b0a 100644 --- a/api/src/org/labkey/api/data/BeanViewForm.java +++ b/api/src/org/labkey/api/data/BeanViewForm.java @@ -24,7 +24,7 @@ import java.util.Map; -public class BeanViewForm extends TableViewForm implements DynaBean +public class BeanViewForm extends TableViewForm { private final Class _wrappedClass; @@ -83,11 +83,11 @@ public void setBean(K bean) } @Override - public Map getStrings() + public Map getStrings() { //If we don't have strings and do have typed values then //make the strings match the typed values - Map strings = super.getStrings(); + Map strings = super.getStrings(); if (null == strings || strings.isEmpty() && (null != _values && !_values.isEmpty())) { strings = new HashMap<>(); @@ -95,7 +95,7 @@ public Map getStrings() { strings.put(entry.getKey(), ConvertUtils.convert(entry.getValue())); } - _stringValues = strings; + setStrings(strings); } return strings; diff --git a/api/src/org/labkey/api/data/DataColumn.java b/api/src/org/labkey/api/data/DataColumn.java index e19c61ec6ef..7245fbbdb25 100644 --- a/api/src/org/labkey/api/data/DataColumn.java +++ b/api/src/org/labkey/api/data/DataColumn.java @@ -445,9 +445,8 @@ protected String renderURLorValueURL(RenderContext ctx) { // See if the value is itself a URL Object value = getDisplayValue(ctx); - if (value != null) + if (value instanceof String toString) { - String toString = value.toString(); if (StringUtilsLabKey.startsWithURL(toString) && !toString.contains(" ") && !toString.contains("\n") && @@ -798,7 +797,7 @@ protected void renderSelectFormInputFromFk(RenderContext ctx, HtmlWriter out, St if (viewForm != null && viewForm.contains(this, ctx)) { // On error reshow, use the user supplied form value - displayValue = viewForm.get(formFieldName); + displayValue = viewForm.getAsString(formFieldName); } if (displayValue == null) displayValue = getDisplayValue(ctx); diff --git a/api/src/org/labkey/api/data/DataRegion.java b/api/src/org/labkey/api/data/DataRegion.java index e5bac1b8929..76d4f84bddd 100644 --- a/api/src/org/labkey/api/data/DataRegion.java +++ b/api/src/org/labkey/api/data/DataRegion.java @@ -2309,7 +2309,7 @@ private void renderForm(RenderContext ctx, HtmlWriter out) //UNDONE: Should we require a viewForm whenever someone //posts? I tend to think so. if (null != viewForm) - pkVal = viewForm.get(pkColName); + pkVal = viewForm.getAsString(pkColName); if (pkVal == null) pkVal = valueMap.get(pkColName); diff --git a/api/src/org/labkey/api/data/DisplayColumn.java b/api/src/org/labkey/api/data/DisplayColumn.java index 9a582246844..2d673af6256 100644 --- a/api/src/org/labkey/api/data/DisplayColumn.java +++ b/api/src/org/labkey/api/data/DisplayColumn.java @@ -1163,7 +1163,7 @@ protected Object getInputValue(RenderContext ctx) if (viewForm.hasTypedValue(formFieldName)) val = viewForm.getTypedValue(formFieldName); else - val = viewForm.get(formFieldName); + val = viewForm.getAsString(formFieldName); } else if (ctx.getRow() != null) val = col.getValue(ctx); diff --git a/api/src/org/labkey/api/data/MVDisplayColumn.java b/api/src/org/labkey/api/data/MVDisplayColumn.java index af465ddca98..c0fb4894a0c 100644 --- a/api/src/org/labkey/api/data/MVDisplayColumn.java +++ b/api/src/org/labkey/api/data/MVDisplayColumn.java @@ -165,7 +165,7 @@ protected Object getInputValue(RenderContext ctx) { String formFieldName = ctx.getForm().getFormFieldName(col); if (null != viewForm && viewForm.getStrings().containsKey(formFieldName)) - val = viewForm.get(formFieldName); + val = viewForm.getAsString(formFieldName); else if (ctx.getRow() != null) { val = getRawValue(ctx); diff --git a/api/src/org/labkey/api/data/StringBeanDynaClass.java b/api/src/org/labkey/api/data/StringBeanDynaClass.java index 0782941e9e1..5f554e3fb3c 100644 --- a/api/src/org/labkey/api/data/StringBeanDynaClass.java +++ b/api/src/org/labkey/api/data/StringBeanDynaClass.java @@ -90,18 +90,6 @@ public static StringBeanDynaClass createDynaClass(Class beanClass) public static StringBeanDynaClass createDynaClass(Class beanClass, Map extraProps) { - - /* - - WrapStringDynaClass dynaClass = - (WrapStringDynaClass) _dynaClasses.get(beanClass); - if (dynaClass == null) - { - dynaClass = new WrapStringDynaClass(beanClass); - _dynaClasses.put(beanClass, dynaClass); - } - return (dynaClass); - */ return new StringBeanDynaClass(beanClass, extraProps); } @@ -117,6 +105,6 @@ public Class getBeanClass() @Override public DynaBean newInstance() { - return new BeanViewForm(_beanClass); + throw new UnsupportedOperationException("StringBeanDynaClass does not support newInstance()"); } } diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index fd7aae09fbb..7c7c6ce8a04 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -19,7 +19,6 @@ import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.beanutils.ConversionException; import org.apache.commons.beanutils.ConvertUtils; -import org.apache.commons.beanutils.DynaBean; import org.apache.commons.beanutils.DynaClass; import org.apache.commons.beanutils.PropertyUtils; import org.apache.commons.collections4.IteratorUtils; @@ -34,11 +33,11 @@ import org.labkey.api.action.NullSafeBindException; import org.labkey.api.action.SpringActionController; import org.labkey.api.collections.CaseInsensitiveHashMap; -import org.labkey.api.ontology.Quantity; import org.labkey.api.security.permissions.DeletePermission; import org.labkey.api.security.permissions.InsertPermission; import org.labkey.api.security.permissions.Permission; import org.labkey.api.security.permissions.UpdatePermission; +import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ActionURL; import org.labkey.api.view.NotFoundException; @@ -55,6 +54,8 @@ import java.beans.Introspector; import java.io.File; import java.sql.SQLException; +import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -67,11 +68,13 @@ * Supports insert, update, delete functionality with a minimum of fuss *

*/ -public class TableViewForm extends ViewForm implements DynaBean, HasBindParameters +public class TableViewForm extends ViewForm implements HasBindParameters { private static final Logger _log = LogHelper.getLogger(TableViewForm.class, "Table operation warnings"); - protected Map _stringValues = new CaseInsensitiveHashMap<>(); + // This is called "stringValues" as this is expected to come from a form POST, + // However, it can also be String[], other types must be converted + protected Map _stringValues = new CaseInsensitiveHashMap<>(); protected Map _values = null; protected StringWrapperDynaClass _dynaClass; protected Object _oldValues; @@ -160,7 +163,7 @@ public void doInsert() throws SQLException throw new UnauthorizedException(); } if (null != _tinfo.getColumn("container")) - set("container", _c.getId()); + setStringToBind("container", _c.getId()); Map newMap = Table.insert(_user, _tinfo, getTypedValues()); setTypedValues(newMap, false); @@ -184,7 +187,7 @@ public void doUpdate() throws SQLException } if (null != _tinfo.getColumn("container")) - set("container", _c.getId()); + setStringToBind("container", _c.getId()); Object[] pkVal = getPkVals(); Map newMap = Table.update(_user, _tinfo, getTypedValues(), pkVal); @@ -296,14 +299,12 @@ public List getPkNamesList() public void setPkVal(String str) { assertSinglePK(); - - set(getPkName(), str); + setStringToBind(getPkName(), str); } public void setPkVal(Object o) { assertSinglePK(); - setTypedValue(getPkName(), o); } @@ -318,16 +319,22 @@ public void setPkVals(String s) { //Issue 42042: Lists with text primary key don't handle commas in key value when viewing row details if (getPkNamesList().size() == 1) - set(getPkNamesList().get(0), s); + { + setStringToBind(getPkNamesList().get(0), s); + } else + { + // TODO replace with new splitStringToValues + // setPkVals(PageFlowUtil.splitStringToValuesForImport(s)); setPkVals(s.split(",")); + } } public void setPkVals(String[] s) { List pkNames = getPkNamesList(); for (int i = 0; i < pkNames.size() && i < s.length; i++) - set(pkNames.get(i), s[i]); + setStringToBind(pkNames.get(i), s[i]); } /** @@ -359,13 +366,17 @@ public Object[] getPkVals() { Object oldValues = getOldValues(); if (oldValues instanceof Map m) + { pkVal = m.get(pkName); + } else + { try { pkVal = PropertyUtils.getProperty(oldValues, pkName); } catch (Exception ignored) {} + } } pkVals[i] = pkVal; } @@ -392,6 +403,30 @@ public void setValidateRequired(boolean validateRequired) _validateRequired = validateRequired; } + + private Object getStringValuesToBind(String propName) + { + Object value = _stringValues.get(propName); + if (null == value) + { + return null; + } + else if (value instanceof String str) + { + return StringUtils.isEmpty(str) ? null : str; + } + else if (value instanceof String[]) + { + return value; + } + else + { + // TODO MultiChoice use a custom checked Map + throw new ConversionException("Unexpected value of type: " + value.getClass()); + } + } + + protected void _populateValues(BindException errors) { // Don't do anything special if dynaclass is null @@ -409,31 +444,29 @@ protected void _populateValues(BindException errors) for (String propName : keys) { + // NOTE later code relies on false==contains(propName) when there is a conversion error + Object bindValue = getStringValuesToBind(propName); + ColumnInfo col = getColumnByFormFieldName(propName); - String str = _stringValues.get(propName); String caption = _dynaClass.getPropertyCaption(propName); Class propType = null; - if (StringUtils.isEmpty(str)) - str = null; - try { - - if (null != str) + if (null != bindValue) { Object val; - if (null != col && null != col.getKindOfQuantity()) + if (null != col) { - val = Quantity.convert(str, col.getDisplayUnit()); + val = col.getConvertFn().apply(bindValue); } else { propType = _dynaClass.getTruePropType(propName); if (propType != null) - val = ConvertUtils.convert(str, propType); + val = ConvertUtils.convert(bindValue, propType); else - val = str; + val = bindValue; } values.put(propName, val); } @@ -455,7 +488,7 @@ else if (_validateRequired && null != _tinfo) if (mvCol != null) { String ff_mvName = getFormFieldName(mvCol); - isError = StringUtils.trimToNull(_stringValues.get(ff_mvName)) == null; + isError = null == getStringValuesToBind(ff_mvName); } } if (isError) @@ -463,7 +496,6 @@ else if (_validateRequired && null != _tinfo) else values.put(propName, null); } - } else { @@ -482,6 +514,7 @@ else if (_validateRequired && null != _tinfo) Container container = fk.getLookupContainer() != null ? fk.getLookupContainer() : getContainer(); try { + String str = null==bindValue ? null : bindValue instanceof String[] ? ((String[])bindValue)[0] : (String)bindValue; Object remappedValue = cache.remap(fk.getLookupSchemaKey(), fk.getLookupTableName(), getUser(), container, ContainerFilter.Type.CurrentPlusProjectAndShared, str); if (remappedValue != null) { @@ -500,6 +533,7 @@ else if (_validateRequired && null != _tinfo) String error = SpringActionController.ERROR_CONVERSION; if (null != propType) error += "." + propType.getSimpleName(); + String str = bindValue instanceof String[] strs ? PageFlowUtil.joinValuesToString(Arrays.asList(strs),',') : String.valueOf(bindValue); errors.addError(new FieldError(errors.getObjectName(), propName, this, true, new String[] {error}, new String[] {str, caption}, Objects.toString(defaultMessage, "Could not convert value: " + str))); } } @@ -537,7 +571,7 @@ public boolean hasTypedValue(ColumnInfo column) public void setTypedValue(String propName, Object val) { getTypedValues().put(propName, val); - _stringValues.put(propName, ConvertUtils.convert(val)); + setStringToBind(propName, val); } /** @@ -572,9 +606,13 @@ public CaseInsensitiveHashMap getTypedColumns(boolean includeUntyped) for (ColumnInfo column : getTable().getColumns()) { if (hasTypedValue(column)) + { values.put(column.getName(), getTypedValue(column)); - else if (includeUntyped && contains(column)) - values.put(column.getName(), get(column)); + } + else if (includeUntyped && _stringValues.containsKey(getFormFieldName(column))) + { + values.put(column.getName(), _stringValues.get(getFormFieldName(column))); + } else if (getRequest() instanceof MultipartHttpServletRequest request) { String fieldName = getMultiPartFormFieldName(column); @@ -603,8 +641,8 @@ else if (File.class.equals(column.getJavaClass())) { if (hasTypedValue(mvColumn)) values.put(mvColumn.getName(), getTypedValue(mvColumn)); - else if (includeUntyped && contains(mvColumn)) - values.put(mvColumn.getName(), get(mvColumn)); + else if (includeUntyped && _stringValues.containsKey(getFormFieldName(mvColumn))) + values.put(mvColumn.getName(), _stringValues.get(getFormFieldName(mvColumn))); } } } @@ -642,107 +680,101 @@ public void setTypedValues(Map values, boolean merge) String propName = e.getKey(); if (Character.isUpperCase(propName.charAt(0))) propName = Introspector.decapitalize(propName); - _values.put(propName, e.getValue()); - _stringValues.put(propName, ConvertUtils.convert(e.getValue())); + setTypedValue(propName, e.getValue()); } } - public void setStrings(Map strings) + public void setStrings(Map strings) { assert null != _dynaClass; - _stringValues = strings; + _stringValues.clear(); + _stringValues.putAll(strings); _values = null; } - public Map getStrings() + public Map getStrings() { return _stringValues; } - public boolean contains(ColumnInfo col) - { - return _stringValues.containsKey(getFormFieldName(col)); - } +// public boolean contains(ColumnInfo col) +// { +// return _stringValues.containsKey(getFormFieldName(col)); +// } public boolean contains(DisplayColumn col, RenderContext ctx) { return _stringValues.containsKey(col.getFormFieldName(ctx)); } - @Override - public String get(String arg0) + public String getAsString(String propName) { - return _stringValues.get(arg0); + Object value = _stringValues.get(propName); + if (value == null || value instanceof String) + return (String)value; + String[] arr = (String[])value; + if (arr.length == 0) + return null; + return arr.length > 0 ? arr[0] : null; } - public String get(ColumnInfo col) + public String getAsString(ColumnInfo col) { - return _stringValues.get(getFormFieldName(col)); + return getAsString(getFormFieldName(col)); } - @Override - public void set(String arg0, Object arg1) + public void setStringToBind(String propName, Object value) { - String v; - if (arg1 == null) - v = null; - else if (arg1 instanceof Object[]) - { - // HACK: This is annoying, but TableViewForm insists on converting values to Strings before letting populateValues() bind. - // Doubly annoying is we need to work around StringArrayConverter's poor parsing of single string values as seen in Issue 5340. - // Convert into stringified array that org.apache.commons.beanutils.converters.StringArrayConverter can parse. - v = "{" + StringUtils.join((Object[])arg1, ",") + "}"; - } + if (null == value || value instanceof String || value instanceof String[]) + _stringValues.put(propName, value); + else if (value instanceof Collection col && col.stream().allMatch(e -> null==e || e instanceof String)) + _stringValues.put(propName, col.toArray(new String[0])); else - { - // Trim to prevent users from inadvertently letting in leading/trailing spaces, which cause confusion on filtering, sorting, joins, and many other places - v = arg1.toString().trim(); - } - _stringValues.put(arg0, v); + _stringValues.put(propName, ConvertUtils.convert(value)); _values = null; } - @Override + // TODO TableViewForm public boolean contains(String arg0, String arg1) { throw new UnsupportedOperationException("No mapped properties in a table"); } - @Override + // TODO TableViewForm public Object get(String arg0, String arg1) { throw new UnsupportedOperationException("No mapped properties in a table"); } - @Override + // TODO TableViewForm public Object get(String arg0, int arg1) { throw new UnsupportedOperationException("No indexed properties in a table"); } - @Override + // TODO TableViewForm public DynaClass getDynaClass() { - return _dynaClass; + throw new UnsupportedOperationException("No mapped properties in a table"); } - @Override + // TODO TableViewForm public void remove(String arg0, String arg1) { - throw new UnsupportedOperationException("No indexed properties in a table"); + throw new UnsupportedOperationException("No mapped properties in a table"); } - @Override + // TODO TableViewForm public void set(String arg0, String arg1, Object arg2) { throw new UnsupportedOperationException("No mapped properties in a table"); } - @Override + // TODO TableViewForm public void set(String arg0, int arg1, Object arg2) { - throw new UnsupportedOperationException("No indexed properties in a table"); + throw new UnsupportedOperationException("No mapped properties in a table"); } public void validateBind(BindException errors) @@ -843,11 +875,9 @@ public void setViewContext(@NotNull ViewContext context) // handle Spring style markers IteratorUtils.asIterator(request.getParameterNames()).forEachRemaining(name -> { if (name.startsWith(SpringActionController.FIELD_MARKER)) - set(name.substring(SpringActionController.FIELD_MARKER.length()), "0"); + setStringToBind(name.substring(SpringActionController.FIELD_MARKER.length()), "0"); }); - BindException errors = new NullSafeBindException(new BaseViewAction.BeanUtilsPropertyBindingResult(this, "form")); - // handle binding of base class ReturnURLForm PropertyValue pvReturn = params.getPropertyValue(ActionURL.Param.returnUrl.toString()); if (null != pvReturn) @@ -863,9 +893,10 @@ public void setViewContext(@NotNull ViewContext context) { Object value = pv.getValue(); if (value instanceof String || value instanceof String[]) - set(pv.getName(), value); + _stringValues.put(pv.getName(), value); } + BindException errors = new NullSafeBindException(new BaseViewAction.BeanUtilsPropertyBindingResult(this, "form")); validateBind(errors); return errors; } diff --git a/api/src/org/labkey/api/data/TableWrapperDynaClass.java b/api/src/org/labkey/api/data/TableWrapperDynaClass.java index 07e07232e05..125b981e400 100644 --- a/api/src/org/labkey/api/data/TableWrapperDynaClass.java +++ b/api/src/org/labkey/api/data/TableWrapperDynaClass.java @@ -69,6 +69,6 @@ public String getName() @Override public DynaBean newInstance() { - return new TableViewForm(_tinfo); + throw new UnsupportedOperationException("TableWrapperDynaClass does not support newInstance()"); } } diff --git a/api/src/org/labkey/api/study/actions/ParticipantVisitResolverChooser.java b/api/src/org/labkey/api/study/actions/ParticipantVisitResolverChooser.java index 782523c6fdf..c65aa3b827a 100644 --- a/api/src/org/labkey/api/study/actions/ParticipantVisitResolverChooser.java +++ b/api/src/org/labkey/api/study/actions/ParticipantVisitResolverChooser.java @@ -202,7 +202,7 @@ protected Object getInputValue(RenderContext ctx) TableViewForm viewForm = ctx.getForm(); // check to see if our insert view has explicit initial values: if (null != viewForm && viewForm.getStrings().containsKey(_typeInputName)) - return viewForm.get(_typeInputName); + return viewForm.getAsString(_typeInputName); return ctx.get(_typeInputName); } } diff --git a/api/src/org/labkey/api/study/assay/FileLinkDisplayColumn.java b/api/src/org/labkey/api/study/assay/FileLinkDisplayColumn.java index 06ae841347c..970fa700288 100644 --- a/api/src/org/labkey/api/study/assay/FileLinkDisplayColumn.java +++ b/api/src/org/labkey/api/study/assay/FileLinkDisplayColumn.java @@ -227,7 +227,7 @@ protected Object getInputValue(RenderContext ctx) { if (null != viewForm && viewForm.contains(this, ctx)) { - val = viewForm.get(getFormFieldName(ctx)); + val = viewForm.getAsString(getFormFieldName(ctx)); } else if (ctx.getRow() != null) val = col.getValue(ctx); diff --git a/api/src/org/labkey/api/study/assay/thawListSelector.jsp b/api/src/org/labkey/api/study/assay/thawListSelector.jsp index bcadc951c5a..840790ab13b 100644 --- a/api/src/org/labkey/api/study/assay/thawListSelector.jsp +++ b/api/src/org/labkey/api/study/assay/thawListSelector.jsp @@ -39,10 +39,10 @@ JspView thisView = HttpView.currentView(); RenderContext ctx = thisView.getModelBean(); boolean renderAll = ctx.get(RenderSubSelectors.class.getSimpleName()) == null ? true : ctx.get(RenderSubSelectors.class.getSimpleName()).equals(RenderSubSelectors.ALL); - boolean listType = ThawListResolverType.LIST_NAMESPACE_SUFFIX.equalsIgnoreCase(ctx.getForm().get(ThawListResolverType.THAW_LIST_TYPE_INPUT_NAME)); + boolean listType = ThawListResolverType.LIST_NAMESPACE_SUFFIX.equalsIgnoreCase(ctx.getForm().getAsString(ThawListResolverType.THAW_LIST_TYPE_INPUT_NAME)); boolean textType = !listType; - String containerPath = ctx.getForm().get(ThawListResolverType.THAW_LIST_LIST_CONTAINER_INPUT_NAME); + String containerPath = ctx.getForm().getAsString(ThawListResolverType.THAW_LIST_LIST_CONTAINER_INPUT_NAME); Container container = containerPath == null ? null : ContainerManager.getForPath(containerPath); String textTypeId = "RadioBtn-" + ThawListResolverType.THAW_LIST_TYPE_INPUT_NAME + "-" + ThawListResolverType.TEXT_NAMESPACE_SUFFIX; @@ -96,7 +96,7 @@ typeAhead : true, typeAheadDelay : 250, forceSelection : true, - initialValue : <%=q(ctx.getForm().get(ThawListResolverType.THAW_LIST_LIST_SCHEMA_NAME_INPUT_NAME))%>, + initialValue : <%=q(ctx.getForm().getAsString(ThawListResolverType.THAW_LIST_LIST_SCHEMA_NAME_INPUT_NAME))%>, fieldLabel : 'Schema', name: <%= q(ThawListResolverType.THAW_LIST_LIST_SCHEMA_NAME_INPUT_NAME) %>, validateOnBlur: false, @@ -104,14 +104,14 @@ })); var queryCombo = Ext4.create('Ext.form.field.ComboBox', sqvModel.makeQueryComboConfig({ - defaultSchema : <%=q(ctx.getForm().get(ThawListResolverType.THAW_LIST_LIST_SCHEMA_NAME_INPUT_NAME))%>, + defaultSchema : <%=q(ctx.getForm().getAsString(ThawListResolverType.THAW_LIST_LIST_SCHEMA_NAME_INPUT_NAME))%>, id : 'thawListQueryName', includeUserQueries: true, typeAhead : true, typeAheadDelay : 250, fieldLabel : 'Query', name: <%= q(ThawListResolverType.THAW_LIST_LIST_QUERY_NAME_INPUT_NAME) %>, - initialValue : <%=q(ctx.getForm().get(ThawListResolverType.THAW_LIST_LIST_QUERY_NAME_INPUT_NAME))%>, + initialValue : <%=q(ctx.getForm().getAsString(ThawListResolverType.THAW_LIST_LIST_QUERY_NAME_INPUT_NAME))%>, width: 500 })); diff --git a/api/src/org/labkey/api/util/HtmlString.java b/api/src/org/labkey/api/util/HtmlString.java index 54943a4d771..9a669260f4f 100644 --- a/api/src/org/labkey/api/util/HtmlString.java +++ b/api/src/org/labkey/api/util/HtmlString.java @@ -30,6 +30,7 @@ public final class HtmlString implements SafeToRender, DOM.Renderable, Comparabl { // Helpful constants for convenience (and efficiency) public static final HtmlString EMPTY_STRING = HtmlString.of(""); + public static final HtmlString SP = HtmlString.of(" "); public static final HtmlString NBSP = HtmlString.unsafe(" "); public static final HtmlString NDASH = HtmlString.unsafe("–"); public static final HtmlString BR = HtmlString.unsafe("
"); diff --git a/core/src/org/labkey/core/view/TableViewFormTestCase.java b/core/src/org/labkey/core/view/TableViewFormTestCase.java index 3d8a96084c2..cc0e6dccdc2 100644 --- a/core/src/org/labkey/core/view/TableViewFormTestCase.java +++ b/core/src/org/labkey/core/view/TableViewFormTestCase.java @@ -45,19 +45,19 @@ public void testBasic() Assert.assertEquals(ctx.getRequest().getUserPrincipal(), tf.getUser()); //Test date handling - tf.set("datetimeNotNull", "2004-06-20"); + tf.setStringToBind("datetimeNotNull", "2004-06-20"); Date dt = (Date) tf.getTypedValue("datetimeNotNull"); Assert.assertTrue("Date get", dt.equals(new Timestamp(DateUtil.parseISODateTime("2004-06-20")))); //Should turn empty strings into nulls - tf.set("text", ""); + tf.setStringToBind("text", ""); Assert.assertNull("Turn string to null", tf.getTypedValue("text")); - tf.set("bitNull", "1"); + tf.setStringToBind("bitNull", "1"); Assert.assertTrue((Boolean) tf.getTypedValue("bitNull")); tf.setPkVal(20); - Assert.assertEquals("20", tf.get("rowId")); + Assert.assertEquals("20", tf.getAsString("rowId")); Assert.assertEquals(20, tf.getTypedValue("rowId")); } @@ -76,10 +76,10 @@ public void testErrorHandling() //Assert.assertEquals("3 Non-null fields", errors.size(), 3); //Non-nullable fields are named NotNull - tf.set("datetimeNotNull", "2004-06-20"); - tf.set("bitNotNull", "1"); - tf.set("intNotNull", "20"); - tf.set("datetimeNull", "garbage"); + tf.setStringToBind("datetimeNotNull", "2004-06-20"); + tf.setStringToBind("bitNotNull", "1"); + tf.setStringToBind("intNotNull", "20"); + tf.setStringToBind("datetimeNull", "garbage"); BindException errors = new NullSafeBindException(tf, "form"); tf.validateBind(errors); @@ -103,11 +103,11 @@ public void testDbOperations() throws SQLException TestForm tf = new TestForm(); tf.setViewContext(ctx); - tf.set("datetimeNotNull", "2004-06-20"); - tf.set("bitNotNull", "1"); - tf.set("intNotNull", "20"); - tf.set("datetimeNull", "2004-06-20"); - tf.set("text", "First test record"); + tf.setStringToBind("datetimeNotNull", "2004-06-20"); + tf.setStringToBind("bitNotNull", "1"); + tf.setStringToBind("intNotNull", "20"); + tf.setStringToBind("datetimeNull", "2004-06-20"); + tf.setStringToBind("text", "First test record"); tf.doInsert(); Assert.assertNotNull(tf.getPkVal()); @@ -116,8 +116,8 @@ public void testDbOperations() throws SQLException Date createdDate = (Date) tf.getTypedValue("created"); //Make sure date->string->date comes out right... - tf.set("datetimeNotNull", tf.get("created")); - tf.set("text", "Second test record"); + tf.setStringToBind("datetimeNotNull", tf.getAsString("created")); + tf.setStringToBind("text", "Second test record"); tf.getStrings().remove("rowId"); tf.doInsert(); Assert.assertEquals("Date time roundtrip: ", createdDate.getTime(), ((Date) tf.getTypedValue("datetimeNotNull")).getTime()); diff --git a/issues/src/org/labkey/issue/IssuesController.java b/issues/src/org/labkey/issue/IssuesController.java index 5e652fe5c0b..720d9b49cb3 100644 --- a/issues/src/org/labkey/issue/IssuesController.java +++ b/issues/src/org/labkey/issue/IssuesController.java @@ -63,7 +63,6 @@ import org.labkey.api.data.NormalContainerType; import org.labkey.api.data.ObjectFactory; import org.labkey.api.data.PHI; -import org.labkey.api.data.PropertyStorageSpec; import org.labkey.api.data.RenderContext; import org.labkey.api.data.Results; import org.labkey.api.data.SimpleFilter; @@ -748,7 +747,7 @@ public List getIssueForms() IssuesForm form = new IssuesForm(); form.setUser(getViewContext().getUser()); form.setContainer(getViewContext().getContainer()); - Map stringMap = new CaseInsensitiveHashMap<>(); + Map stringMap = new CaseInsensitiveHashMap<>(); for (String prop : rec.keySet()) { Object value = rec.get(prop); @@ -784,7 +783,7 @@ public class IssuesAction extends AbstractIssueApiAction Issue.action getAction(IssuesForm form) { if (form.getStrings().containsKey("action")) - return Issue.action.valueOf(form.getStrings().get("action")); + return Issue.action.valueOf(form.getAsString("action")); return null; } } @@ -816,7 +815,7 @@ public void validateForm(IssuesApiForm form, Errors errors) IssueObject prevIssue = action != Issue.action.insert ? IssueManager.getIssue(getContainer(), getUser(), issuesForm.getIssueId()) : null; Map prevIssueProps = prevIssue == null ? Collections.emptyMap() : prevIssue.getProperties(); - Map stringMap = new CaseInsensitiveHashMap<>(issuesForm.getStrings()); + Map stringMap = new CaseInsensitiveHashMap<>(issuesForm.getStrings()); for (DomainProperty prop : issueListDef.getDomain(getUser()).getProperties()) { if (!IssueDefDomainKind.RESOLUTION_LOOKUP.equalsIgnoreCase(prop.getName())) @@ -864,7 +863,7 @@ public ApiResponse execute(IssuesApiForm form, BindException errors) throws Exce setTypedProperties(issue, issuesForm, issueListDef.getName()); // handle attachments, the attachment value is a | delimited array of file names - String attachments = issuesForm.get("attachment"); + String attachments = issuesForm.getAsString("attachment"); List attachmentFiles = new ArrayList<>(); if (!StringUtils.isBlank(attachments)) { @@ -1370,9 +1369,9 @@ public ModelAndView getView(IssuesController.IssuesForm form, boolean reshow, Bi if (_issue.getResolution() == null || _issue.getResolution().isEmpty()) { - if (form.get("resolution") != null) + if (form.getAsString("resolution") != null) { - _issue.setResolution(form.get("resolution")); + _issue.setResolution(form.getAsString("resolution")); } } beforeReshow(reshow, form, _issue, getIssueListDef()); @@ -2229,45 +2228,45 @@ private static Map extraProps() public Issue.action getAction() { if (getStrings().containsKey("action")) - return Issue.action.valueOf(getStrings().get("action")); + return Issue.action.valueOf(getAsString("action")); throw new NotFoundException("No action specified"); } public String getComment() { - return _stringValues.get("comment"); + return getAsString("comment"); } public String getNotifyList() { - return _stringValues.get("notifyList"); + return getAsString("notifyList"); } // XXX: change return value to typed ReturnURLString public String getCallbackURL() { - return _stringValues.get("callbackURL"); + return getAsString("callbackURL"); } public String getBody() { - return _stringValues.get("body"); + return getAsString("body"); } public String getPriority() { - return _stringValues.get("priority"); + return getAsString("priority"); } private String getIssueDefName() { - return _stringValues.get(IssuesListView.ISSUE_LIST_DEF_NAME); + return getAsString(IssuesListView.ISSUE_LIST_DEF_NAME); } private String getIssueDefId() { - return _stringValues.get(IssuesListView.ISSUE_LIST_DEF_ID); + return getAsString(IssuesListView.ISSUE_LIST_DEF_ID); } // Make this method public @@ -2284,7 +2283,7 @@ public void setTable(@NotNull TableInfo table) */ public boolean getSkipPost() { - return BooleanUtils.toBoolean(_stringValues.get("skipPost")); + return BooleanUtils.toBoolean(getAsString("skipPost")); } public ActionURL getForwardURL() @@ -2304,12 +2303,12 @@ public ActionURL getForwardURL() public int getIssueId() { - return NumberUtils.toInt(_stringValues.get("issueId")); + return NumberUtils.toInt(getAsString("issueId")); } public boolean isDirty() { - return BooleanUtils.toBoolean(_stringValues.get("dirty")); + return BooleanUtils.toBoolean(getAsString("dirty")); } } diff --git a/study/src/org/labkey/study/controllers/CohortController.java b/study/src/org/labkey/study/controllers/CohortController.java index 22da7a171f1..6fd1c62f68d 100644 --- a/study/src/org/labkey/study/controllers/CohortController.java +++ b/study/src/org/labkey/study/controllers/CohortController.java @@ -341,7 +341,7 @@ public ModelAndView getView(EditCohortForm form, boolean reshow, BindException e { view = new InsertView(updateForm, errors); // by default, cohorts are enrolled - updateForm.set("enrolled", true); + updateForm.setStringToBind("enrolled", true); } else { diff --git a/study/src/org/labkey/study/controllers/StudyController.java b/study/src/org/labkey/study/controllers/StudyController.java index 4cd599b94ee..55aa0fee64b 100644 --- a/study/src/org/labkey/study/controllers/StudyController.java +++ b/study/src/org/labkey/study/controllers/StudyController.java @@ -1716,7 +1716,7 @@ public void validateForm(TableViewForm form, Errors errors) { // Issue 47444 and Issue 47881: Validate that subject noun singular doesn't match the name of an existing // study table or dataset - String subjectNounSingular = form.get("SubjectNounSingular"); + String subjectNounSingular = form.getAsString("SubjectNounSingular"); if (null != subjectNounSingular) { String message = StudyService.get().getSubjectNounSingularValidationErrorMessage(getContainer(), subjectNounSingular); @@ -1728,7 +1728,7 @@ public void validateForm(TableViewForm form, Errors errors) // Skip validation if Spring binding already has an error for subject noun plural if (errors.getFieldError("SubjectNounPlural") == null) { - String subjectNounPlural = form.get("SubjectNounPlural"); + String subjectNounPlural = form.getAsString("SubjectNounPlural"); if (null != subjectNounPlural) { String message = StudyService.get().getSubjectNounPluralValidationErrorMessage(getContainer(), subjectNounPlural); @@ -1741,7 +1741,7 @@ public void validateForm(TableViewForm form, Errors errors) if (errors.getFieldError("SubjectColumnName") == null) { // Issue 43898: Validate that the subject column name is not a user-defined field in one of the datasets - String subjectColName = form.get("SubjectColumnName"); + String subjectColName = form.getAsString("SubjectColumnName"); if (null != subjectColName) { String message = StudyService.get().getSubjectColumnNameValidationErrorMessage(getContainer(), subjectColName); diff --git a/study/src/org/labkey/study/controllers/StudyPropertiesController.java b/study/src/org/labkey/study/controllers/StudyPropertiesController.java index baf075e77c8..16f6917444a 100644 --- a/study/src/org/labkey/study/controllers/StudyPropertiesController.java +++ b/study/src/org/labkey/study/controllers/StudyPropertiesController.java @@ -72,7 +72,7 @@ public ModelAndView getView(StudyProperties studyPropertiesForm, boolean reshow, // In order to pull the data out for an edit, we need to explicitly add the container id to the parameters // that the query update form will use - updateForm.set("container", getContainer().getId()); + updateForm.setStringToBind("container", getContainer().getId()); UpdateView view = new UpdateView(updateForm, errors); DataRegion dataRegion = view.getDataRegion(); From 34f41a50798b979ddf49210621449b4a2e9a66c6 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Fri, 24 Oct 2025 16:58:32 -0700 Subject: [PATCH 02/19] revert convert path for now --- api/src/org/labkey/api/data/TableViewForm.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index 7c7c6ce8a04..632fa17b79c 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -33,6 +33,7 @@ import org.labkey.api.action.NullSafeBindException; import org.labkey.api.action.SpringActionController; import org.labkey.api.collections.CaseInsensitiveHashMap; +import org.labkey.api.ontology.Quantity; import org.labkey.api.security.permissions.DeletePermission; import org.labkey.api.security.permissions.InsertPermission; import org.labkey.api.security.permissions.Permission; @@ -455,14 +456,15 @@ protected void _populateValues(BindException errors) { if (null != bindValue) { + propType = _dynaClass.getTruePropType(propName); Object val; - if (null != col) + if (null != col && null != col.getKindOfQuantity()) { - val = col.getConvertFn().apply(bindValue); + // TODO MultiChoice switch to col.getConvertFn().apply(bindValue) + val = Quantity.convert(bindValue, col.getDisplayUnit()); } else { - propType = _dynaClass.getTruePropType(propName); if (propType != null) val = ConvertUtils.convert(bindValue, propType); else From b3291a5ae29f7fabf6cae08101885f42c2346e5a Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Mon, 27 Oct 2025 14:59:03 -0700 Subject: [PATCH 03/19] rm TableWrapperDynaClass NOTE: BeanViewForm also probably doesn't need to extend DynaBean and use StringBeanDynaClass --- .../AnnouncementsController.java | 6 +- .../api/assay/actions/UploadWizardAction.java | 2 +- api/src/org/labkey/api/data/BeanViewForm.java | 84 ++++++++- .../api/data/ColumnRenderPropertiesImpl.java | 15 +- api/src/org/labkey/api/data/DataRegion.java | 2 +- .../org/labkey/api/data/MVDisplayColumn.java | 2 +- .../labkey/api/data/StringBeanDynaClass.java | 22 +-- .../org/labkey/api/data/TableViewForm.java | 168 ++++++------------ .../api/data/TableWrapperDynaClass.java | 74 -------- .../labkey/api/exp/SamplePropertyHelper.java | 2 +- .../org/labkey/api/query/QueryUpdateForm.java | 3 +- .../api/query/QueryWrapperDynaClass.java | 77 -------- .../ParticipantVisitResolverChooser.java | 2 +- .../core/view/TableViewFormTestCase.java | 30 ++-- .../org/labkey/issue/IssuesController.java | 16 +- .../study/controllers/CohortController.java | 2 +- .../StudyPropertiesController.java | 2 +- 17 files changed, 179 insertions(+), 330 deletions(-) delete mode 100644 api/src/org/labkey/api/data/TableWrapperDynaClass.java delete mode 100644 api/src/org/labkey/api/query/QueryWrapperDynaClass.java diff --git a/announcements/src/org/labkey/announcements/AnnouncementsController.java b/announcements/src/org/labkey/announcements/AnnouncementsController.java index 8f32edc966d..40dada4a7b5 100644 --- a/announcements/src/org/labkey/announcements/AnnouncementsController.java +++ b/announcements/src/org/labkey/announcements/AnnouncementsController.java @@ -1084,7 +1084,7 @@ else if (null == latestPost) cal.add(Calendar.MONTH, 1); String expires = DateUtil.formatDate(c, cal.getTime()); - form.setStringToBind("expires", expires); + form.setValueToBind("expires", expires); currentRendererType = DEFAULT_MESSAGE_RENDERER_TYPE; assignedTo = settings.getDefaultAssignedTo(); } @@ -1094,8 +1094,8 @@ else if (null == latestPost) assert null == form.getAsString("title"); assert null == form.getAsString("expires"); - form.setStringToBind("title", latestPost.getTitle()); - form.setStringToBind("status", "Active"); // By default, every new response resets status to active, #35047 + form.setValueToBind("title", latestPost.getTitle()); + form.setValueToBind("status", "Active"); // By default, every new response resets status to active, #35047 form.setTypedValue("expires", DateUtil.formatDate(c, latestPost.getExpires())); assignedTo = latestPost.getAssignedTo(); diff --git a/api/src/org/labkey/api/assay/actions/UploadWizardAction.java b/api/src/org/labkey/api/assay/actions/UploadWizardAction.java index e5520b9731c..6ab8552359e 100644 --- a/api/src/org/labkey/api/assay/actions/UploadWizardAction.java +++ b/api/src/org/labkey/api/assay/actions/UploadWizardAction.java @@ -909,7 +909,7 @@ public void renderInputHtml(RenderContext ctx, HtmlWriter out, Object value) protected Object getInputValue(RenderContext ctx) { TableViewForm viewForm = ctx.getForm(); - return viewForm.getStrings().get(_inputName); + return viewForm.getValuesToBind().get(_inputName); } } diff --git a/api/src/org/labkey/api/data/BeanViewForm.java b/api/src/org/labkey/api/data/BeanViewForm.java index 40f817f1b0a..8d7677d991b 100644 --- a/api/src/org/labkey/api/data/BeanViewForm.java +++ b/api/src/org/labkey/api/data/BeanViewForm.java @@ -19,13 +19,16 @@ import org.apache.commons.beanutils.BeanUtils; import org.apache.commons.beanutils.ConvertUtils; import org.apache.commons.beanutils.DynaBean; +import org.apache.commons.beanutils.DynaClass; +import org.labkey.api.action.HasBindParameters; import java.util.HashMap; import java.util.Map; -public class BeanViewForm extends TableViewForm +public class BeanViewForm extends TableViewForm implements DynaBean, HasBindParameters { + private final StringBeanDynaClass _dynaClass; private final Class _wrappedClass; protected BeanViewForm(Class clss) @@ -39,9 +42,10 @@ public BeanViewForm(Class clss, TableInfo tinfo) } - public BeanViewForm(Class clss, TableInfo tinfo, Map extraProps) + public BeanViewForm(Class clss, TableInfo tinfo, Map> extraProps) { - super(StringBeanDynaClass.createDynaClass(clss, extraProps), tinfo); + super(tinfo); + _dynaClass = StringBeanDynaClass.createDynaClass(clss, extraProps); _wrappedClass = clss; } @@ -60,7 +64,7 @@ public K getBean() else bean = (K) BeanUtils.cloneBean(_oldValues); - factory.fromMap(bean, getStrings()); + factory.fromMap(bean, getValuesToBind()); return bean; } catch (ReflectiveOperationException x) @@ -71,7 +75,7 @@ public K getBean() else { ObjectFactory factory = ObjectFactory.Registry.getFactory(_wrappedClass); - return factory.fromMap(getStrings()); + return factory.fromMap(getValuesToBind()); } } @@ -83,11 +87,11 @@ public void setBean(K bean) } @Override - public Map getStrings() + public Map getValuesToBind() { //If we don't have strings and do have typed values then //make the strings match the typed values - Map strings = super.getStrings(); + Map strings = super.getValuesToBind(); if (null == strings || strings.isEmpty() && (null != _values && !_values.isEmpty())) { strings = new HashMap<>(); @@ -95,7 +99,7 @@ public Map getStrings() { strings.put(entry.getKey(), ConvertUtils.convert(entry.getValue())); } - setStrings(strings); + setValuesToBind(strings); } return strings; @@ -118,4 +122,68 @@ else if (o instanceof Map) throw new IllegalArgumentException("Type of old values is incompatible with wrapped class"); } } + + @Override + protected Class getTruePropType(String propName) + { + var ret = _dynaClass.getTruePropType(propName); + if (null == ret) + ret = super.getTruePropType(propName); + return ret; + } + + // DynaBean + @Override + public DynaClass getDynaClass() + { + return _dynaClass; + } + + @Override + public Object get(String name) + { + return super.getValueToBind(name); + } + + @Override + public void set(String name, Object value) + { + super.setValueToBind(name,value); + } + + @Override + public boolean contains(String arg0, String arg1) + { + throw new UnsupportedOperationException("No mapped properties in a table"); + } + + @Override + public Object get(String arg0, String arg1) + { + throw new UnsupportedOperationException("No mapped properties in a table"); + } + + @Override + public Object get(String arg0, int arg1) + { + throw new UnsupportedOperationException("No indexed properties in a table"); + } + + @Override + public void remove(String arg0, String arg1) + { + throw new UnsupportedOperationException("No indexed properties in a table"); + } + + @Override + public void set(String arg0, String arg1, Object arg2) + { + throw new UnsupportedOperationException("No mapped properties in a table"); + } + + @Override + public void set(String arg0, int arg1, Object arg2) + { + throw new UnsupportedOperationException("No indexed properties in a table"); + } } diff --git a/api/src/org/labkey/api/data/ColumnRenderPropertiesImpl.java b/api/src/org/labkey/api/data/ColumnRenderPropertiesImpl.java index 85d452d90a6..43b7b35651e 100644 --- a/api/src/org/labkey/api/data/ColumnRenderPropertiesImpl.java +++ b/api/src/org/labkey/api/data/ColumnRenderPropertiesImpl.java @@ -785,10 +785,15 @@ public final Class getJavaClass() @Override public Class getJavaClass(boolean isNullable) + { + return defaultJavaClass(this, isNullable); + } + + public static Class defaultJavaClass(ColumnRenderProperties col, boolean isNullable) { Class ret; boolean isNumeric; - PropertyType pt = getPropertyType(); + PropertyType pt = col.getPropertyType(); if (pt != null) { ret = pt.getJavaType(); @@ -796,13 +801,13 @@ public Class getJavaClass(boolean isNullable) } else { - ret = getJdbcType().getJavaClass(isNullable); - isNumeric = getJdbcType().isNumeric(); + JdbcType jdbcType = col.getJdbcType(); + ret =jdbcType.getJavaClass(isNullable); + isNumeric = jdbcType.isNumeric(); } - if (isNumeric) { - Unit unit = getDisplayUnit(); + Unit unit = col.getDisplayUnit(); if (null != unit) return unit.getQuantityClass(); } diff --git a/api/src/org/labkey/api/data/DataRegion.java b/api/src/org/labkey/api/data/DataRegion.java index 76d4f84bddd..a42b78f3dab 100644 --- a/api/src/org/labkey/api/data/DataRegion.java +++ b/api/src/org/labkey/api/data/DataRegion.java @@ -2058,7 +2058,7 @@ private void renderInputForm(RenderContext ctx, HtmlWriter out) { TableViewForm form = ctx.getForm(); if (null != form) - ctx.setRow((Map) form.getStrings()); + ctx.setRow(form.getValuesToBind()); } renderForm(ctx, out); } diff --git a/api/src/org/labkey/api/data/MVDisplayColumn.java b/api/src/org/labkey/api/data/MVDisplayColumn.java index c0fb4894a0c..c60b6907c66 100644 --- a/api/src/org/labkey/api/data/MVDisplayColumn.java +++ b/api/src/org/labkey/api/data/MVDisplayColumn.java @@ -164,7 +164,7 @@ protected Object getInputValue(RenderContext ctx) if (col != null) { String formFieldName = ctx.getForm().getFormFieldName(col); - if (null != viewForm && viewForm.getStrings().containsKey(formFieldName)) + if (null != viewForm && viewForm.getValuesToBind().containsKey(formFieldName)) val = viewForm.getAsString(formFieldName); else if (ctx.getRow() != null) { diff --git a/api/src/org/labkey/api/data/StringBeanDynaClass.java b/api/src/org/labkey/api/data/StringBeanDynaClass.java index 5f554e3fb3c..67259bcd485 100644 --- a/api/src/org/labkey/api/data/StringBeanDynaClass.java +++ b/api/src/org/labkey/api/data/StringBeanDynaClass.java @@ -38,10 +38,10 @@ protected StringBeanDynaClass(Class beanClass) this(beanClass, null); } - protected StringBeanDynaClass(Class beanClass, Map extras) + protected StringBeanDynaClass(Class beanClass, Map> extras) { _beanClass = beanClass; - PropertyDescriptor propDescriptors[] = PropertyUtils.getPropertyDescriptors(beanClass); + PropertyDescriptor[] propDescriptors = PropertyUtils.getPropertyDescriptors(beanClass); if (propDescriptors == null) propDescriptors = new PropertyDescriptor[0]; Map> propTypes = new CaseInsensitiveHashMap<>(); @@ -49,7 +49,7 @@ protected StringBeanDynaClass(Class beanClass, Map extras) propTypes.put(propDescriptor.getName(), propDescriptor.getPropertyType()); if (null != extras) { - for (Map.Entry entry : extras.entrySet()) + for (Map.Entry> entry : extras.entrySet()) { String prop = entry.getKey(); if (propTypes.containsKey(prop)) @@ -70,25 +70,13 @@ protected StringBeanDynaClass(Class beanClass, Map extras) * * @param beanClass Bean class for which a WrapDynaClass is requested */ - public static StringBeanDynaClass createDynaClass(Class beanClass) + public static StringBeanDynaClass createDynaClass(Class beanClass) { - - /* - - WrapStringDynaClass dynaClass = - (WrapStringDynaClass) _dynaClasses.get(beanClass); - if (dynaClass == null) - { - dynaClass = new WrapStringDynaClass(beanClass); - _dynaClasses.put(beanClass, dynaClass); - } - return (dynaClass); - */ return new StringBeanDynaClass(beanClass); } - public static StringBeanDynaClass createDynaClass(Class beanClass, Map extraProps) + public static StringBeanDynaClass createDynaClass(Class beanClass, Map> extraProps) { return new StringBeanDynaClass(beanClass, extraProps); } diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index 632fa17b79c..d18ccaaa915 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -19,7 +19,6 @@ import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.beanutils.ConversionException; import org.apache.commons.beanutils.ConvertUtils; -import org.apache.commons.beanutils.DynaClass; import org.apache.commons.beanutils.PropertyUtils; import org.apache.commons.collections4.IteratorUtils; import org.apache.commons.lang3.StringUtils; @@ -54,6 +53,7 @@ import java.beans.Introspector; import java.io.File; +import java.lang.reflect.Array; import java.sql.SQLException; import java.util.Arrays; import java.util.Collection; @@ -73,11 +73,10 @@ public class TableViewForm extends ViewForm implements HasBindParameters { private static final Logger _log = LogHelper.getLogger(TableViewForm.class, "Table operation warnings"); - // This is called "stringValues" as this is expected to come from a form POST, - // However, it can also be String[], other types must be converted + // This is called "stringValues" as this is expected to come from a form POST (but it was never just string valure) + // However, it can also be String[] abd other types protected Map _stringValues = new CaseInsensitiveHashMap<>(); protected Map _values = null; - protected StringWrapperDynaClass _dynaClass; protected Object _oldValues; protected TableInfo _tinfo = null; protected String[] _selectedRows = null; @@ -89,19 +88,12 @@ public class TableViewForm extends ViewForm implements HasBindParameters public static final String DATA_SUBMIT_NAME = ".dataSubmit"; public static final String BULK_UPDATE_NAME = ".bulkUpdate"; - /** - * Creates a TableViewForm with no underlying dynaclass. - */ + protected TableViewForm() { super(); } - public TableViewForm(StringWrapperDynaClass dynaClass) - { - super(); - _dynaClass = dynaClass; - } /** * Creates a view form that wraps a table. @@ -111,25 +103,9 @@ public TableViewForm(@NotNull TableInfo tinfo) setTable(tinfo); } - /** - * Creates a view form that uses the supplied dynaClass for the property - * list, but stashes the tableInfo for insert/update purposes and - * to perform additional validation. - */ - public TableViewForm(StringWrapperDynaClass dynaClass, TableInfo tinfo) - { - _dynaClass = dynaClass; - _tinfo = tinfo; - } - - /** - * Sets the table. NOTE This will also overwrite any previously - * set dynaClass with one derived from the table. - */ protected void setTable(@NotNull TableInfo tinfo) { _tinfo = tinfo; - _dynaClass = TableWrapperDynaClass.getDynaClassInstance(tinfo); } public TableInfo getTable() @@ -164,7 +140,7 @@ public void doInsert() throws SQLException throw new UnauthorizedException(); } if (null != _tinfo.getColumn("container")) - setStringToBind("container", _c.getId()); + setValueToBind("container", _c.getId()); Map newMap = Table.insert(_user, _tinfo, getTypedValues()); setTypedValues(newMap, false); @@ -188,7 +164,7 @@ public void doUpdate() throws SQLException } if (null != _tinfo.getColumn("container")) - setStringToBind("container", _c.getId()); + setValueToBind("container", _c.getId()); Object[] pkVal = getPkVals(); Map newMap = Table.update(_user, _tinfo, getTypedValues(), pkVal); @@ -300,7 +276,7 @@ public List getPkNamesList() public void setPkVal(String str) { assertSinglePK(); - setStringToBind(getPkName(), str); + setValueToBind(getPkName(), str); } public void setPkVal(Object o) @@ -321,7 +297,7 @@ public void setPkVals(String s) //Issue 42042: Lists with text primary key don't handle commas in key value when viewing row details if (getPkNamesList().size() == 1) { - setStringToBind(getPkNamesList().get(0), s); + setValueToBind(getPkNamesList().get(0), s); } else { @@ -335,7 +311,7 @@ public void setPkVals(String[] s) { List pkNames = getPkNamesList(); for (int i = 0; i < pkNames.size() && i < s.length; i++) - setStringToBind(pkNames.get(i), s[i]); + setValueToBind(pkNames.get(i), s[i]); } /** @@ -399,40 +375,26 @@ public BindException populateValues(BindException errors) return errors; } + public void setValidateRequired(boolean validateRequired) { _validateRequired = validateRequired; } - private Object getStringValuesToBind(String propName) + public Object getValueToBind(String propName) { Object value = _stringValues.get(propName); if (null == value) - { return null; - } - else if (value instanceof String str) - { + if (value instanceof String str) return StringUtils.isEmpty(str) ? null : str; - } - else if (value instanceof String[]) - { - return value; - } - else - { - // TODO MultiChoice use a custom checked Map - throw new ConversionException("Unexpected value of type: " + value.getClass()); - } + return value; } protected void _populateValues(BindException errors) { - // Don't do anything special if dynaclass is null - assert _dynaClass != null; - /* Note that nulls in the hashmap are NOT the same as missing values A null in the hashmap indicates an empty string was posted. @@ -446,17 +408,16 @@ protected void _populateValues(BindException errors) for (String propName : keys) { // NOTE later code relies on false==contains(propName) when there is a conversion error - Object bindValue = getStringValuesToBind(propName); - + Object bindValue = getValueToBind(propName); ColumnInfo col = getColumnByFormFieldName(propName); - String caption = _dynaClass.getPropertyCaption(propName); + String caption = getPropertyCaption(propName); Class propType = null; try { if (null != bindValue) { - propType = _dynaClass.getTruePropType(propName); + propType = getTruePropType(propName); Object val; if (null != col && null != col.getKindOfQuantity()) { @@ -490,7 +451,7 @@ else if (_validateRequired && null != _tinfo) if (mvCol != null) { String ff_mvName = getFormFieldName(mvCol); - isError = null == getStringValuesToBind(ff_mvName); + isError = null == getValueToBind(ff_mvName); } } if (isError) @@ -573,7 +534,9 @@ public boolean hasTypedValue(ColumnInfo column) public void setTypedValue(String propName, Object val) { getTypedValues().put(propName, val); - setStringToBind(propName, val); + // setValueToBind() but w/o invalidate. + // To convert or not to convert??? + _stringValues.put(propName, val); } /** @@ -586,10 +549,6 @@ public void setTypedValue(String propName, Object val) */ public Map getTypedValues() { - // Don't have values if dynaclass is null - if (null == _dynaClass) - return null; - if (null == _values) populateValues(null); @@ -666,7 +625,6 @@ public CaseInsensitiveHashMap getTypedColumns() */ public void setTypedValues(Map values, boolean merge) { - assert null != _dynaClass; values = Collections.unmodifiableMap(values); //We assume this means data is loaded. @@ -682,20 +640,19 @@ public void setTypedValues(Map values, boolean merge) String propName = e.getKey(); if (Character.isUpperCase(propName.charAt(0))) propName = Introspector.decapitalize(propName); - setTypedValue(propName, e.getValue()); + // TODO MultiChoice To convert or not to convert??? + _stringValues.put(propName, e.getValue()); } } - public void setStrings(Map strings) + public void setValuesToBind(Map strings) { - assert null != _dynaClass; - _stringValues.clear(); _stringValues.putAll(strings); _values = null; } - public Map getStrings() + public Map getValuesToBind() { return _stringValues; } @@ -726,7 +683,7 @@ public String getAsString(ColumnInfo col) return getAsString(getFormFieldName(col)); } - public void setStringToBind(String propName, Object value) + public void setValueToBind(String propName, Object value) { if (null == value || value instanceof String || value instanceof String[]) _stringValues.put(propName, value); @@ -737,47 +694,6 @@ else if (value instanceof Collection col && col.stream().allMatch(e -> null== _values = null; } - // TODO TableViewForm - public boolean contains(String arg0, String arg1) - { - throw new UnsupportedOperationException("No mapped properties in a table"); - } - - // TODO TableViewForm - public Object get(String arg0, String arg1) - { - throw new UnsupportedOperationException("No mapped properties in a table"); - } - - // TODO TableViewForm - public Object get(String arg0, int arg1) - { - throw new UnsupportedOperationException("No indexed properties in a table"); - } - - // TODO TableViewForm - public DynaClass getDynaClass() - { - throw new UnsupportedOperationException("No mapped properties in a table"); - } - - // TODO TableViewForm - public void remove(String arg0, String arg1) - { - throw new UnsupportedOperationException("No mapped properties in a table"); - } - - // TODO TableViewForm - public void set(String arg0, String arg1, Object arg2) - { - throw new UnsupportedOperationException("No mapped properties in a table"); - } - - // TODO TableViewForm - public void set(String arg0, int arg1, Object arg2) - { - throw new UnsupportedOperationException("No mapped properties in a table"); - } public void validateBind(BindException errors) { @@ -799,12 +715,38 @@ public void setOldValues(Object oldValues) public void forceReselect() { Object[] pk = getPkVals(); - setStrings(new HashMap<>()); + setValuesToBind(new HashMap<>()); setOldValues(null); setPkVals(pk); setDataLoaded(false); } + + protected Class getTruePropType(String propName) + { + ColumnInfo column = getColumnByFormFieldName(propName); + if (null == column) + return null; + // TODO MultiChoice : move this to ColumnInfo (it does not belong in this one place) + // TODO MultiChoice : Can we actually assume that the FK column (in this table) is the same type as the lookup column? + boolean multiValued = column.getFk() instanceof MultiValuedForeignKey && ((MultiValuedForeignKey)column.getFk()).isMultiSelectInput(); + if (multiValued) + return arrayClass(column.getJavaClass()); + return column.getJavaClass(); + } + + private static Class arrayClass(Class k) + { + Object o = Array.newInstance(k, 0); + return o.getClass(); + } + + private String getPropertyCaption(String propName) + { + ColumnInfo column = getColumnByFormFieldName(propName); + return null==column ? propName : column.getLabel(); + } + public String getFormFieldName(@NotNull ColumnInfo column) { return column.getName(); @@ -877,7 +819,7 @@ public void setViewContext(@NotNull ViewContext context) // handle Spring style markers IteratorUtils.asIterator(request.getParameterNames()).forEachRemaining(name -> { if (name.startsWith(SpringActionController.FIELD_MARKER)) - setStringToBind(name.substring(SpringActionController.FIELD_MARKER.length()), "0"); + setValueToBind(name.substring(SpringActionController.FIELD_MARKER.length()), "0"); }); // handle binding of base class ReturnURLForm @@ -893,9 +835,7 @@ public void setViewContext(@NotNull ViewContext context) for (PropertyValue pv : params.getPropertyValues()) { - Object value = pv.getValue(); - if (value instanceof String || value instanceof String[]) - _stringValues.put(pv.getName(), value); + _stringValues.put(pv.getName(), pv.getValue()); } BindException errors = new NullSafeBindException(new BaseViewAction.BeanUtilsPropertyBindingResult(this, "form")); diff --git a/api/src/org/labkey/api/data/TableWrapperDynaClass.java b/api/src/org/labkey/api/data/TableWrapperDynaClass.java deleted file mode 100644 index 125b981e400..00000000000 --- a/api/src/org/labkey/api/data/TableWrapperDynaClass.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright (c) 2004-2018 Fred Hutchinson Cancer Research Center - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.labkey.api.data; - -import org.apache.commons.beanutils.DynaBean; -import org.jetbrains.annotations.NotNull; -import org.labkey.api.collections.CaseInsensitiveHashMap; - -import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - -/** - * Creates a DynaClass for a table where all properties are strings. - */ -public class TableWrapperDynaClass extends StringWrapperDynaClass -{ - private final TableInfo _tinfo; - private static final Map _dynClasses = new ConcurrentHashMap<>(); - - private TableWrapperDynaClass(TableInfo tinfo) - { - _tinfo = tinfo; - List cols = tinfo.getColumns(); - Map> propMap = new CaseInsensitiveHashMap<>(); - for (ColumnInfo col : cols) - propMap.put(col.getName(), col.getJavaClass()); - - init(tinfo.getName(), propMap); - } - - public static TableWrapperDynaClass getDynaClassInstance(@NotNull TableInfo tinfo) - { - TableWrapperDynaClass tdc = _dynClasses.get(tinfo); - if (null == tdc) - { - tdc = new TableWrapperDynaClass(tinfo); - if (tinfo instanceof SchemaTableInfo) - _dynClasses.put(tinfo, tdc); - } - return tdc; - } - - public TableInfo getTable() - { - return _tinfo; - } - - @Override - public String getName() - { - return _tinfo.getName(); - } - - @Override - public DynaBean newInstance() - { - throw new UnsupportedOperationException("TableWrapperDynaClass does not support newInstance()"); - } -} diff --git a/api/src/org/labkey/api/exp/SamplePropertyHelper.java b/api/src/org/labkey/api/exp/SamplePropertyHelper.java index b259e01d0f7..1a41a77318b 100644 --- a/api/src/org/labkey/api/exp/SamplePropertyHelper.java +++ b/api/src/org/labkey/api/exp/SamplePropertyHelper.java @@ -246,7 +246,7 @@ public boolean isEditable() protected Object getInputValue(RenderContext ctx) { TableViewForm viewForm = ctx.getForm(); - return viewForm.getStrings().get(getName()); + return viewForm.getValuesToBind().get(getName()); } @Override diff --git a/api/src/org/labkey/api/query/QueryUpdateForm.java b/api/src/org/labkey/api/query/QueryUpdateForm.java index f3ad03b2360..c2585120947 100644 --- a/api/src/org/labkey/api/query/QueryUpdateForm.java +++ b/api/src/org/labkey/api/query/QueryUpdateForm.java @@ -51,8 +51,7 @@ public QueryUpdateForm(@NotNull TableInfo table, @NotNull ViewContext ctx, boole public QueryUpdateForm(@NotNull TableInfo table, @NotNull ViewContext ctx, @Nullable BindException errors) { - _tinfo = table; - _dynaClass = new QueryWrapperDynaClass(this); + super(table); setViewContext(ctx); // TODO: Fix this hack. diff --git a/api/src/org/labkey/api/query/QueryWrapperDynaClass.java b/api/src/org/labkey/api/query/QueryWrapperDynaClass.java deleted file mode 100644 index dfa346e8470..00000000000 --- a/api/src/org/labkey/api/query/QueryWrapperDynaClass.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright (c) 2008-2019 LabKey Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.labkey.api.query; - -import org.apache.commons.beanutils.DynaBean; -import org.jetbrains.annotations.NotNull; -import org.labkey.api.collections.CaseInsensitiveHashMap; -import org.labkey.api.data.ColumnInfo; -import org.labkey.api.data.MultiValuedForeignKey; -import org.labkey.api.data.StringWrapperDynaClass; -import org.labkey.api.data.TableInfo; - -import java.lang.reflect.Array; -import java.util.Map; - -public class QueryWrapperDynaClass extends StringWrapperDynaClass -{ - QueryUpdateForm _form; - - public QueryWrapperDynaClass(@NotNull QueryUpdateForm form) - { - _form = form; - TableInfo table = form.getTable(); - if (table == null) - throw new IllegalArgumentException(); - - // CONSIDER: Handle MultiValueFK in column.getJavaClass() directly - Map> propMap = new CaseInsensitiveHashMap<>(); - for (ColumnInfo column : table.getColumns()) - { - boolean multiValued = column.getFk() instanceof MultiValuedForeignKey && ((MultiValuedForeignKey)column.getFk()).isMultiSelectInput(); - if (multiValued) - propMap.put(_form.getFormFieldName(column), arrayClass(column.getJavaClass())); - else - propMap.put(_form.getFormFieldName(column), column.getJavaClass()); - } - - init("className", propMap); - - } - - private static Class arrayClass(Class k) - { - Object o = Array.newInstance(k, 0); - return o.getClass(); - } - - - @Override - public DynaBean newInstance() - { - throw new UnsupportedOperationException(); - } - - @Override - public String getPropertyCaption(String propName) - { - ColumnInfo column = _form.getColumnByFormFieldName(propName); - if (column == null) - return propName; - return column.getLabel(); - } -} diff --git a/api/src/org/labkey/api/study/actions/ParticipantVisitResolverChooser.java b/api/src/org/labkey/api/study/actions/ParticipantVisitResolverChooser.java index c65aa3b827a..c9a6d34f1ef 100644 --- a/api/src/org/labkey/api/study/actions/ParticipantVisitResolverChooser.java +++ b/api/src/org/labkey/api/study/actions/ParticipantVisitResolverChooser.java @@ -201,7 +201,7 @@ protected Object getInputValue(RenderContext ctx) { TableViewForm viewForm = ctx.getForm(); // check to see if our insert view has explicit initial values: - if (null != viewForm && viewForm.getStrings().containsKey(_typeInputName)) + if (null != viewForm && viewForm.getValuesToBind().containsKey(_typeInputName)) return viewForm.getAsString(_typeInputName); return ctx.get(_typeInputName); } diff --git a/core/src/org/labkey/core/view/TableViewFormTestCase.java b/core/src/org/labkey/core/view/TableViewFormTestCase.java index cc0e6dccdc2..0ee5366a76e 100644 --- a/core/src/org/labkey/core/view/TableViewFormTestCase.java +++ b/core/src/org/labkey/core/view/TableViewFormTestCase.java @@ -45,15 +45,15 @@ public void testBasic() Assert.assertEquals(ctx.getRequest().getUserPrincipal(), tf.getUser()); //Test date handling - tf.setStringToBind("datetimeNotNull", "2004-06-20"); + tf.setValueToBind("datetimeNotNull", "2004-06-20"); Date dt = (Date) tf.getTypedValue("datetimeNotNull"); Assert.assertTrue("Date get", dt.equals(new Timestamp(DateUtil.parseISODateTime("2004-06-20")))); //Should turn empty strings into nulls - tf.setStringToBind("text", ""); + tf.setValueToBind("text", ""); Assert.assertNull("Turn string to null", tf.getTypedValue("text")); - tf.setStringToBind("bitNull", "1"); + tf.setValueToBind("bitNull", "1"); Assert.assertTrue((Boolean) tf.getTypedValue("bitNull")); tf.setPkVal(20); @@ -76,10 +76,10 @@ public void testErrorHandling() //Assert.assertEquals("3 Non-null fields", errors.size(), 3); //Non-nullable fields are named NotNull - tf.setStringToBind("datetimeNotNull", "2004-06-20"); - tf.setStringToBind("bitNotNull", "1"); - tf.setStringToBind("intNotNull", "20"); - tf.setStringToBind("datetimeNull", "garbage"); + tf.setValueToBind("datetimeNotNull", "2004-06-20"); + tf.setValueToBind("bitNotNull", "1"); + tf.setValueToBind("intNotNull", "20"); + tf.setValueToBind("datetimeNull", "garbage"); BindException errors = new NullSafeBindException(tf, "form"); tf.validateBind(errors); @@ -103,11 +103,11 @@ public void testDbOperations() throws SQLException TestForm tf = new TestForm(); tf.setViewContext(ctx); - tf.setStringToBind("datetimeNotNull", "2004-06-20"); - tf.setStringToBind("bitNotNull", "1"); - tf.setStringToBind("intNotNull", "20"); - tf.setStringToBind("datetimeNull", "2004-06-20"); - tf.setStringToBind("text", "First test record"); + tf.setValueToBind("datetimeNotNull", "2004-06-20"); + tf.setValueToBind("bitNotNull", "1"); + tf.setValueToBind("intNotNull", "20"); + tf.setValueToBind("datetimeNull", "2004-06-20"); + tf.setValueToBind("text", "First test record"); tf.doInsert(); Assert.assertNotNull(tf.getPkVal()); @@ -116,9 +116,9 @@ public void testDbOperations() throws SQLException Date createdDate = (Date) tf.getTypedValue("created"); //Make sure date->string->date comes out right... - tf.setStringToBind("datetimeNotNull", tf.getAsString("created")); - tf.setStringToBind("text", "Second test record"); - tf.getStrings().remove("rowId"); + tf.setValueToBind("datetimeNotNull", tf.getAsString("created")); + tf.setValueToBind("text", "Second test record"); + tf.getValuesToBind().remove("rowId"); tf.doInsert(); Assert.assertEquals("Date time roundtrip: ", createdDate.getTime(), ((Date) tf.getTypedValue("datetimeNotNull")).getTime()); tf.doDelete(); diff --git a/issues/src/org/labkey/issue/IssuesController.java b/issues/src/org/labkey/issue/IssuesController.java index 720d9b49cb3..1eade2b3251 100644 --- a/issues/src/org/labkey/issue/IssuesController.java +++ b/issues/src/org/labkey/issue/IssuesController.java @@ -753,7 +753,7 @@ public List getIssueForms() Object value = rec.get(prop); stringMap.put(prop, JSONObject.NULL.equals(value) ? null : value.toString()); } - form.setStrings(stringMap); + form.setValuesToBind(stringMap); _issueForms.add(form); } } @@ -782,7 +782,7 @@ public class IssuesAction extends AbstractIssueApiAction @Override Issue.action getAction(IssuesForm form) { - if (form.getStrings().containsKey("action")) + if (form.getValuesToBind().containsKey("action")) return Issue.action.valueOf(form.getAsString("action")); return null; } @@ -815,15 +815,15 @@ public void validateForm(IssuesApiForm form, Errors errors) IssueObject prevIssue = action != Issue.action.insert ? IssueManager.getIssue(getContainer(), getUser(), issuesForm.getIssueId()) : null; Map prevIssueProps = prevIssue == null ? Collections.emptyMap() : prevIssue.getProperties(); - Map stringMap = new CaseInsensitiveHashMap<>(issuesForm.getStrings()); + Map stringMap = new CaseInsensitiveHashMap<>(issuesForm.getValuesToBind()); for (DomainProperty prop : issueListDef.getDomain(getUser()).getProperties()) { if (!IssueDefDomainKind.RESOLUTION_LOOKUP.equalsIgnoreCase(prop.getName())) stringMap.computeIfAbsent(prop.getName(), (propName) -> Objects.toString(prevIssueProps.get(propName), null)); } // Be sure that the posted values take precedence, even if they're null - stringMap.putAll(issuesForm.getStrings()); - issuesForm.setStrings(stringMap); + stringMap.putAll(issuesForm.getValuesToBind()); + issuesForm.setValuesToBind(stringMap); } IssueObject issue = issuesForm.getBean(); @@ -2217,9 +2217,9 @@ public IssuesForm() setValidateRequired(false); } - private static Map extraProps() + private static Map> extraProps() { - Map map = new LinkedHashMap<>(); + Map> map = new LinkedHashMap<>(); map.put("action", String.class); map.put("callbackURL", String.class); return map; @@ -2227,7 +2227,7 @@ private static Map extraProps() public Issue.action getAction() { - if (getStrings().containsKey("action")) + if (getValuesToBind().containsKey("action")) return Issue.action.valueOf(getAsString("action")); throw new NotFoundException("No action specified"); diff --git a/study/src/org/labkey/study/controllers/CohortController.java b/study/src/org/labkey/study/controllers/CohortController.java index 6fd1c62f68d..fc2463fa34d 100644 --- a/study/src/org/labkey/study/controllers/CohortController.java +++ b/study/src/org/labkey/study/controllers/CohortController.java @@ -341,7 +341,7 @@ public ModelAndView getView(EditCohortForm form, boolean reshow, BindException e { view = new InsertView(updateForm, errors); // by default, cohorts are enrolled - updateForm.setStringToBind("enrolled", true); + updateForm.setValueToBind("enrolled", true); } else { diff --git a/study/src/org/labkey/study/controllers/StudyPropertiesController.java b/study/src/org/labkey/study/controllers/StudyPropertiesController.java index 16f6917444a..c66f8f12c03 100644 --- a/study/src/org/labkey/study/controllers/StudyPropertiesController.java +++ b/study/src/org/labkey/study/controllers/StudyPropertiesController.java @@ -72,7 +72,7 @@ public ModelAndView getView(StudyProperties studyPropertiesForm, boolean reshow, // In order to pull the data out for an edit, we need to explicitly add the container id to the parameters // that the query update form will use - updateForm.setStringToBind("container", getContainer().getId()); + updateForm.setValueToBind("container", getContainer().getId()); UpdateView view = new UpdateView(updateForm, errors); DataRegion dataRegion = view.getDataRegion(); From bae6284e85eba26498d58e345182f4ebf36d7fe2 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Mon, 27 Oct 2025 15:44:28 -0700 Subject: [PATCH 04/19] oops --- api/src/org/labkey/api/data/TableViewForm.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index d18ccaaa915..7321b7185cb 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -640,6 +640,7 @@ public void setTypedValues(Map values, boolean merge) String propName = e.getKey(); if (Character.isUpperCase(propName.charAt(0))) propName = Introspector.decapitalize(propName); + setTypedValue(propName, e.getValue()); // TODO MultiChoice To convert or not to convert??? _stringValues.put(propName, e.getValue()); } @@ -672,10 +673,13 @@ public String getAsString(String propName) Object value = _stringValues.get(propName); if (value == null || value instanceof String) return (String)value; - String[] arr = (String[])value; - if (arr.length == 0) - return null; - return arr.length > 0 ? arr[0] : null; + if (value instanceof String[]) + { + String[] arr = (String[]) value; + if (arr.length == 0) + return null; + } + return ConvertUtils.convert(value); } public String getAsString(ColumnInfo col) From 81b37c5c1e2b5ef8d1cb1249b11a17d058a639ca Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Tue, 28 Oct 2025 09:55:33 -0700 Subject: [PATCH 05/19] LF->CRLF --- .../org/labkey/api/action/BaseViewAction.java | 1510 ++++++++--------- 1 file changed, 755 insertions(+), 755 deletions(-) diff --git a/api/src/org/labkey/api/action/BaseViewAction.java b/api/src/org/labkey/api/action/BaseViewAction.java index 130f9a8c483..18b0cb68dfe 100644 --- a/api/src/org/labkey/api/action/BaseViewAction.java +++ b/api/src/org/labkey/api/action/BaseViewAction.java @@ -1,755 +1,755 @@ -/* - * Copyright (c) 2008-2019 LabKey Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.labkey.api.action; - -import jakarta.servlet.ServletRequest; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import org.apache.commons.beanutils.ConversionException; -import org.apache.commons.beanutils.ConvertUtils; -import org.apache.commons.beanutils.DynaBean; -import org.apache.commons.beanutils.PropertyUtils; -import org.apache.commons.lang3.StringUtils; -import org.apache.logging.log4j.Logger; -import org.jetbrains.annotations.NotNull; -import org.labkey.api.attachments.AttachmentFile; -import org.labkey.api.attachments.SpringAttachmentFile; -import org.labkey.api.data.Container; -import org.labkey.api.data.ConvertHelper; -import org.labkey.api.security.User; -import org.labkey.api.util.HelpTopic; -import org.labkey.api.util.PageFlowUtil; -import org.labkey.api.util.logging.LogHelper; -import org.labkey.api.view.HttpView; -import org.labkey.api.view.template.PageConfig; -import org.labkey.api.writer.ContainerUser; -import org.springframework.beans.AbstractPropertyAccessor; -import org.springframework.beans.BeanUtils; -import org.springframework.beans.BeanWrapper; -import org.springframework.beans.BeansException; -import org.springframework.beans.InvalidPropertyException; -import org.springframework.beans.MutablePropertyValues; -import org.springframework.beans.NotReadablePropertyException; -import org.springframework.beans.NotWritablePropertyException; -import org.springframework.beans.PropertyAccessException; -import org.springframework.beans.PropertyValue; -import org.springframework.beans.PropertyValues; -import org.springframework.beans.TypeMismatchException; -import org.springframework.core.MethodParameter; -import org.springframework.core.convert.TypeDescriptor; -import org.springframework.lang.Nullable; -import org.springframework.validation.BeanPropertyBindingResult; -import org.springframework.validation.BindException; -import org.springframework.validation.BindingErrorProcessor; -import org.springframework.validation.BindingResult; -import org.springframework.validation.FieldError; -import org.springframework.validation.ObjectError; -import org.springframework.validation.Validator; -import org.springframework.web.bind.ServletRequestDataBinder; -import org.springframework.web.bind.ServletRequestParameterPropertyValues; -import org.springframework.web.multipart.MultipartFile; -import org.springframework.web.multipart.MultipartHttpServletRequest; -import org.springframework.web.servlet.ModelAndView; - -import java.beans.PropertyDescriptor; -import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.function.Predicate; - -public abstract class BaseViewAction extends PermissionCheckableAction implements Validator, HasPageConfig, ContainerUser -{ - protected static final Logger logger = LogHelper.getLogger(BaseViewAction.class, "BaseViewAction"); - - private PageConfig _pageConfig = null; - private PropertyValues _pvs; - private boolean _robot = false; // Is this request from GoogleBot or some other crawler? - private boolean _debug = false; - - protected boolean _print = false; - protected Class _commandClass; - protected String _commandName = "form"; - - protected BaseViewAction() - { - String methodName = getCommandClassMethodName(); - - if (null == methodName) - return; - - // inspect the action's *public* methods to determine form class - Class typeBest = null; - for (Method m : this.getClass().getMethods()) - { - if (methodName.equals(m.getName())) - { - Class[] types = m.getParameterTypes(); - if (types.length < 1) - continue; - Class typeCurrent = types[0]; - assert null == _commandClass || typeCurrent.equals(_commandClass); - - // Using templated classes to extend a base action can lead to multiple - // versions of a method with acceptable types, so take the most extended - // type we can find. - if (typeBest == null || typeBest.isAssignableFrom(typeCurrent)) - typeBest = typeCurrent; - } - } - if (typeBest != null) - 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) - { - PropertyValue pv = _pvs.getPropertyValue(key); - 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); - return pv == null ? null : pv.getValue(); - } - - public PropertyValues getPropertyValues() - { - return _pvs; - } - - - public static PropertyValues getPropertyValuesForFormBinding(PropertyValues pvs, @NotNull Predicate allowBind) - { - if (null == pvs) - return null; - MutablePropertyValues ret = new MutablePropertyValues(); - for (PropertyValue pv : pvs.getPropertyValues()) - { - if (allowBind.test(pv.getName())) - ret.addPropertyValue(pv); - } - return ret; - } - - static final String FORM_DATE_ENCODED_PARAM = "formDataEncoded"; - - /** - * When a double quote is encountered in a multipart/form-data context, it is encoded as %22 using URL-encoding by browsers. - * This process replaces the double quote with its hexadecimal equivalent in a URL-safe format, preventing it from being misinterpreted as the end of a value or a boundary. - * The consequence of such encoding is we can't distinguish '"' from the actual '%22' in parameter name. - * As a workaround, a client-side util `encodeFormDataQuote` is used to convert %22 to %2522 and " to %22 explicitly, while passing in an additional param formDataEncoded=true. - * This class converts those encoded param names back to its decoded form during PropertyValues binding. - * See Issue 52827, 52925 and 52119 for more information. - */ - static public class ViewActionParameterPropertyValues extends ServletRequestParameterPropertyValues - { - - public ViewActionParameterPropertyValues(ServletRequest request) { - this(request, null, null); - } - - public ViewActionParameterPropertyValues(ServletRequest request, @Nullable String prefix, @Nullable String prefixSeparator) - { - super(request, prefix, prefixSeparator); - if (isFormDataEncoded()) - { - for (int i = 0; i < getPropertyValues().length; i++) - { - PropertyValue formDataPropValue = getPropertyValues()[i]; - String propValueName = formDataPropValue.getName(); - String decoded = PageFlowUtil.decodeQuoteEncodedFormDataKey(propValueName); - if (!propValueName.equals(decoded)) - setPropertyValueAt(new PropertyValue(decoded, formDataPropValue.getValue()), i); - } - } - } - - private boolean isFormDataEncoded() - { - PropertyValue formDataPropValue = getPropertyValue(FORM_DATE_ENCODED_PARAM); - if (formDataPropValue != null) - { - Object v = formDataPropValue.getValue(); - String formDataPropValueStr = v == null ? null : String.valueOf(v); - if (StringUtils.isNotBlank(formDataPropValueStr)) - return (Boolean) ConvertUtils.convert(formDataPropValueStr, Boolean.class); - } - - return false; - } - } - - @Override - public ModelAndView handleRequest(@NotNull HttpServletRequest request, @NotNull HttpServletResponse response) throws Exception - { - if (null == getPropertyValues()) - setProperties(new ViewActionParameterPropertyValues(request)); - getViewContext().setBindPropertyValues(getPropertyValues()); - handleSpecialProperties(); - - return handleRequest(); - } - - - private void handleSpecialProperties() - { - _robot = PageFlowUtil.isRobotUserAgent(getViewContext().getRequest().getHeader("User-Agent")); - - // Special flag puts actions in "debug" mode, during which they should log extra information that would be - // helpful for testing or debugging problems - if (!_robot && hasStringValue("_debug")) - { - _debug = true; - } - - // SpringActionController.defaultPageConfig() has logic for isPrint, don't need to duplicate here - _print = PageConfig.Template.Print == HttpView.currentPageConfig().getTemplate(); - } - - private boolean hasStringValue(String propertyName) - { - Object o = getProperty(propertyName); - if (o == null) - { - return false; - } - if (o instanceof String s) - { - return !StringUtils.isBlank(s); - } - if (o instanceof String[] strings) - { - for (String s : strings) - { - if (!StringUtils.isBlank(s)) - { - return true; - } - } - } - return false; - } - - 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 - { - return c == null ? null : c.getConstructor().newInstance(); - } - catch (Exception x) - { - if (x instanceof RuntimeException) - throw ((RuntimeException)x); - else - throw new RuntimeException(x); - } - } - - - protected @NotNull FORM getCommand(HttpServletRequest request) throws Exception - { - FORM command = (FORM) createCommand(); - - if (command instanceof HasViewContext) - ((HasViewContext)command).setViewContext(getViewContext()); - - return command; - } - - - protected @NotNull FORM getCommand() throws Exception - { - return getCommand(getViewContext().getRequest()); - } - - - // - // PARAMETER BINDING - // - // don't assume parameters always come from a request, use PropertyValues interface - // - - public @NotNull BindException defaultBindParameters(FORM form, PropertyValues params) - { - return defaultBindParameters(form, getCommandName(), params); - } - - - public static @NotNull BindException defaultBindParameters(Object form, String commandName, PropertyValues params) - { - /* check for do-it-myself forms */ - if (form instanceof HasBindParameters) - { - return ((HasBindParameters)form).bindParameters(params); - } - - if (form instanceof DynaBean) - { - return simpleBindParameters(form, commandName, params); - } - else - { - return springBindParameters(form, commandName, params); - } - } - - public static @NotNull BindException springBindParameters(Object command, String commandName, PropertyValues params) - { - Predicate allow = command instanceof HasAllowBindParameter allowBP ? allowBP.allowBindParameter() : HasAllowBindParameter.getDefaultPredicate(); - ServletRequestDataBinder binder = new ServletRequestDataBinder(command, commandName); - - String[] fields = binder.getDisallowedFields(); - List fieldList = new ArrayList<>(fields != null ? Arrays.asList(fields) : Collections.emptyList()); - fieldList.addAll(Arrays.asList("class.*", "Class.*", "*.class.*", "*.Class.*")); - binder.setDisallowedFields(fieldList.toArray(new String[] {})); - - ConvertHelper.getPropertyEditorRegistrar().registerCustomEditors(binder); - BindingErrorProcessor defaultBEP = binder.getBindingErrorProcessor(); - binder.setBindingErrorProcessor(getBindingErrorProcessor(defaultBEP)); - binder.setFieldMarkerPrefix(SpringActionController.FIELD_MARKER); - try - { - // most paths probably called getPropertyValuesForFormBinding() already, but this is a public static method, so call it again - binder.bind(getPropertyValuesForFormBinding(params, allow)); - BindException errors = new NullSafeBindException(binder.getBindingResult()); - return errors; - } - catch (InvalidPropertyException x) - { - // 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()); - 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() + ")"); - 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() + ")"); - 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() + ")"); - return errors; - } - } - - - static BindingErrorProcessor getBindingErrorProcessor(final BindingErrorProcessor defaultBEP) - { - return new BindingErrorProcessor() - { - @Override - public void processMissingFieldError(String missingField, BindingResult bindingResult) - { - defaultBEP.processMissingFieldError(missingField, bindingResult); - } - - @Override - public void processPropertyAccessException(PropertyAccessException ex, BindingResult bindingResult) - { - Object newValue = ex.getPropertyChangeEvent().getNewValue(); - if (newValue instanceof String) - newValue = StringUtils.trimToNull((String)newValue); - - // convert NULL conversion errors to required errors - if (null == newValue) - defaultBEP.processMissingFieldError(ex.getPropertyChangeEvent().getPropertyName(), bindingResult); - else - defaultBEP.processPropertyAccessException(ex, bindingResult); - } - }; - } - - - /* - * This binder doesn't have much to offer over the standard spring data binding except that it will - * handle DynaBeans. - */ - public static @NotNull BindException simpleBindParameters(Object command, String commandName, PropertyValues params) - { - Predicate allow = command instanceof HasAllowBindParameter allowBP ? allowBP.allowBindParameter() : HasAllowBindParameter.getDefaultPredicate(); - - BindException errors = new NullSafeBindException(command, "Form"); - - // unfortunately ObjectFactory and BeanObjectFactory are not good about reporting errors - // do this by hand - for (PropertyValue pv : params.getPropertyValues()) - { - String propertyName = pv.getName(); - Object value = pv.getValue(); - if (!allow.test(propertyName)) - continue; - - try - { - Object converted = value; - Class propClass = PropertyUtils.getPropertyType(command, propertyName); - if (null == propClass) - continue; - if (value == null) - { - /* */ - } - else if (propClass.isPrimitive()) - { - converted = ConvertUtils.convert(String.valueOf(value), propClass); - } - else if (propClass.isArray()) - { - if (value instanceof Collection) - value = ((Collection) value).toArray(new String[0]); - else if (!value.getClass().isArray()) - value = new String[] {String.valueOf(value)}; - converted = ConvertUtils.convert((String[])value, propClass); - } - PropertyUtils.setProperty(command, propertyName, converted); - } - catch (ConversionException x) - { - errors.addError(new FieldError(commandName, propertyName, value, true, new String[] {"ConversionError", "typeMismatch"}, null, "Could not convert to value: " + value)); - } - catch (Exception x) - { - errors.addError(new ObjectError(commandName, new String[]{"Error"}, new Object[] {value}, x.getMessage())); - logger.error("unexpected error", x); - } - } - return errors; - } - - @Override - public boolean supports(Class clazz) - { - return getCommandClass().isAssignableFrom(clazz); - } - - - /* for TableViewForm, uses BeanUtils to work with DynaBeans */ - static public class BeanUtilsPropertyBindingResult extends BeanPropertyBindingResult - { - public BeanUtilsPropertyBindingResult(Object target, String objectName) - { - super(target, objectName); - } - - @Override - protected BeanWrapper createBeanWrapper() - { - return new BeanUtilsWrapperImpl((DynaBean)getTarget()); - } - } - - static public class BeanUtilsWrapperImpl extends AbstractPropertyAccessor implements BeanWrapper - { - private Object object; - private boolean autoGrowNestedPaths = false; - private int autoGrowCollectionLimit = 0; - - public BeanUtilsWrapperImpl() - { - // registerDefaultEditors(); - } - - public BeanUtilsWrapperImpl(DynaBean target) - { - this(); - object = target; - } - - @Override - public Object getPropertyValue(String propertyName) throws BeansException - { - try - { - return PropertyUtils.getProperty(object, propertyName); - } - catch (Exception e) - { - throw new NotReadablePropertyException(object.getClass(), propertyName); - } - } - - @Override - public void setPropertyValue(String propertyName, Object value) throws BeansException - { - try - { - PropertyUtils.setProperty(object, propertyName, value); - } - catch (Exception e) - { - throw new NotWritablePropertyException(object.getClass(), propertyName); - } - } - - @Override - public boolean isReadableProperty(String propertyName) - { - return true; - } - - @Override - public boolean isWritableProperty(String propertyName) - { - return true; - } - - @Override - public TypeDescriptor getPropertyTypeDescriptor(String s) throws BeansException - { - return null; - } - - public void setWrappedInstance(Object obj) - { - object = obj; - } - - @Override - public Object getWrappedInstance() - { - return object; - } - - @Override - public Class getWrappedClass() - { - return object.getClass(); - } - - @Override - public PropertyDescriptor[] getPropertyDescriptors() - { - throw new UnsupportedOperationException(); - } - - @Override - public PropertyDescriptor getPropertyDescriptor(String propertyName) throws BeansException - { - throw new UnsupportedOperationException(); - } - - @Override - public void setAutoGrowNestedPaths(boolean b) - { - this.autoGrowNestedPaths = b; - } - - @Override - public boolean isAutoGrowNestedPaths() - { - return this.autoGrowNestedPaths; - } - - @Override - public void setAutoGrowCollectionLimit(int i) - { - this.autoGrowCollectionLimit = i; - } - - @Override - public int getAutoGrowCollectionLimit() - { - return this.autoGrowCollectionLimit; - } - - @Override - public T convertIfNecessary(Object value, Class requiredType) throws TypeMismatchException - { - if (value == null) - return null; - return (T)ConvertUtils.convert(String.valueOf(value), requiredType); - } - - @Override - public T convertIfNecessary(Object value, Class requiredType, MethodParameter methodParam) throws TypeMismatchException - { - return convertIfNecessary(value, requiredType); - } - } - - /** - * @return a map from form element name to uploaded files - */ - protected Map getFileMap() - { - if (getViewContext().getRequest() instanceof MultipartHttpServletRequest) - return ((MultipartHttpServletRequest)getViewContext().getRequest()).getFileMap(); - return Collections.emptyMap(); - } - - protected List getAttachmentFileList() - { - return SpringAttachmentFile.createList(getFileMap()); - } - - public boolean isRobot() - { - return _robot; - } - - public boolean isPrint() - { - return _print; - } - - public boolean isDebug() - { - return _debug; - } - - public @NotNull Class getCommandClass() - { - if (null == _commandClass) - throw new IllegalStateException("NULL _commandClass in " + getClass().getName()); - return _commandClass; - } - - public void setCommandClass(@NotNull Class commandClass) - { - _commandClass = commandClass; - } - - protected final @NotNull Object createCommand() - { - return BeanUtils.instantiateClass(getCommandClass()); - } - - public void setCommandName(String commandName) - { - _commandName = commandName; - } - - public String getCommandName() - { - return _commandName; - } - - /** - * Cacheable resources can calculate a last modified timestamp to send to the browser. - */ - protected long getLastModified(FORM form) - { - return Long.MIN_VALUE; - } - - /** - * Cacheable resources can calculate an ETag header to send to the browser. - */ - protected String getETag(FORM form) - { - return null; - } -} +/* + * Copyright (c) 2008-2019 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.labkey.api.action; + +import jakarta.servlet.ServletRequest; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.apache.commons.beanutils.ConversionException; +import org.apache.commons.beanutils.ConvertUtils; +import org.apache.commons.beanutils.DynaBean; +import org.apache.commons.beanutils.PropertyUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.NotNull; +import org.labkey.api.attachments.AttachmentFile; +import org.labkey.api.attachments.SpringAttachmentFile; +import org.labkey.api.data.Container; +import org.labkey.api.data.ConvertHelper; +import org.labkey.api.security.User; +import org.labkey.api.util.HelpTopic; +import org.labkey.api.util.PageFlowUtil; +import org.labkey.api.util.logging.LogHelper; +import org.labkey.api.view.HttpView; +import org.labkey.api.view.template.PageConfig; +import org.labkey.api.writer.ContainerUser; +import org.springframework.beans.AbstractPropertyAccessor; +import org.springframework.beans.BeanUtils; +import org.springframework.beans.BeanWrapper; +import org.springframework.beans.BeansException; +import org.springframework.beans.InvalidPropertyException; +import org.springframework.beans.MutablePropertyValues; +import org.springframework.beans.NotReadablePropertyException; +import org.springframework.beans.NotWritablePropertyException; +import org.springframework.beans.PropertyAccessException; +import org.springframework.beans.PropertyValue; +import org.springframework.beans.PropertyValues; +import org.springframework.beans.TypeMismatchException; +import org.springframework.core.MethodParameter; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.lang.Nullable; +import org.springframework.validation.BeanPropertyBindingResult; +import org.springframework.validation.BindException; +import org.springframework.validation.BindingErrorProcessor; +import org.springframework.validation.BindingResult; +import org.springframework.validation.FieldError; +import org.springframework.validation.ObjectError; +import org.springframework.validation.Validator; +import org.springframework.web.bind.ServletRequestDataBinder; +import org.springframework.web.bind.ServletRequestParameterPropertyValues; +import org.springframework.web.multipart.MultipartFile; +import org.springframework.web.multipart.MultipartHttpServletRequest; +import org.springframework.web.servlet.ModelAndView; + +import java.beans.PropertyDescriptor; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.function.Predicate; + +public abstract class BaseViewAction extends PermissionCheckableAction implements Validator, HasPageConfig, ContainerUser +{ + protected static final Logger logger = LogHelper.getLogger(BaseViewAction.class, "BaseViewAction"); + + private PageConfig _pageConfig = null; + private PropertyValues _pvs; + private boolean _robot = false; // Is this request from GoogleBot or some other crawler? + private boolean _debug = false; + + protected boolean _print = false; + protected Class _commandClass; + protected String _commandName = "form"; + + protected BaseViewAction() + { + String methodName = getCommandClassMethodName(); + + if (null == methodName) + return; + + // inspect the action's *public* methods to determine form class + Class typeBest = null; + for (Method m : this.getClass().getMethods()) + { + if (methodName.equals(m.getName())) + { + Class[] types = m.getParameterTypes(); + if (types.length < 1) + continue; + Class typeCurrent = types[0]; + assert null == _commandClass || typeCurrent.equals(_commandClass); + + // Using templated classes to extend a base action can lead to multiple + // versions of a method with acceptable types, so take the most extended + // type we can find. + if (typeBest == null || typeBest.isAssignableFrom(typeCurrent)) + typeBest = typeCurrent; + } + } + if (typeBest != null) + 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) + { + PropertyValue pv = _pvs.getPropertyValue(key); + 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); + return pv == null ? null : pv.getValue(); + } + + public PropertyValues getPropertyValues() + { + return _pvs; + } + + + public static PropertyValues getPropertyValuesForFormBinding(PropertyValues pvs, @NotNull Predicate allowBind) + { + if (null == pvs) + return null; + MutablePropertyValues ret = new MutablePropertyValues(); + for (PropertyValue pv : pvs.getPropertyValues()) + { + if (allowBind.test(pv.getName())) + ret.addPropertyValue(pv); + } + return ret; + } + + static final String FORM_DATE_ENCODED_PARAM = "formDataEncoded"; + + /** + * When a double quote is encountered in a multipart/form-data context, it is encoded as %22 using URL-encoding by browsers. + * This process replaces the double quote with its hexadecimal equivalent in a URL-safe format, preventing it from being misinterpreted as the end of a value or a boundary. + * The consequence of such encoding is we can't distinguish '"' from the actual '%22' in parameter name. + * As a workaround, a client-side util `encodeFormDataQuote` is used to convert %22 to %2522 and " to %22 explicitly, while passing in an additional param formDataEncoded=true. + * This class converts those encoded param names back to its decoded form during PropertyValues binding. + * See Issue 52827, 52925 and 52119 for more information. + */ + static public class ViewActionParameterPropertyValues extends ServletRequestParameterPropertyValues + { + + public ViewActionParameterPropertyValues(ServletRequest request) { + this(request, null, null); + } + + public ViewActionParameterPropertyValues(ServletRequest request, @Nullable String prefix, @Nullable String prefixSeparator) + { + super(request, prefix, prefixSeparator); + if (isFormDataEncoded()) + { + for (int i = 0; i < getPropertyValues().length; i++) + { + PropertyValue formDataPropValue = getPropertyValues()[i]; + String propValueName = formDataPropValue.getName(); + String decoded = PageFlowUtil.decodeQuoteEncodedFormDataKey(propValueName); + if (!propValueName.equals(decoded)) + setPropertyValueAt(new PropertyValue(decoded, formDataPropValue.getValue()), i); + } + } + } + + private boolean isFormDataEncoded() + { + PropertyValue formDataPropValue = getPropertyValue(FORM_DATE_ENCODED_PARAM); + if (formDataPropValue != null) + { + Object v = formDataPropValue.getValue(); + String formDataPropValueStr = v == null ? null : String.valueOf(v); + if (StringUtils.isNotBlank(formDataPropValueStr)) + return (Boolean) ConvertUtils.convert(formDataPropValueStr, Boolean.class); + } + + return false; + } + } + + @Override + public ModelAndView handleRequest(@NotNull HttpServletRequest request, @NotNull HttpServletResponse response) throws Exception + { + if (null == getPropertyValues()) + setProperties(new ViewActionParameterPropertyValues(request)); + getViewContext().setBindPropertyValues(getPropertyValues()); + handleSpecialProperties(); + + return handleRequest(); + } + + + private void handleSpecialProperties() + { + _robot = PageFlowUtil.isRobotUserAgent(getViewContext().getRequest().getHeader("User-Agent")); + + // Special flag puts actions in "debug" mode, during which they should log extra information that would be + // helpful for testing or debugging problems + if (!_robot && hasStringValue("_debug")) + { + _debug = true; + } + + // SpringActionController.defaultPageConfig() has logic for isPrint, don't need to duplicate here + _print = PageConfig.Template.Print == HttpView.currentPageConfig().getTemplate(); + } + + private boolean hasStringValue(String propertyName) + { + Object o = getProperty(propertyName); + if (o == null) + { + return false; + } + if (o instanceof String s) + { + return !StringUtils.isBlank(s); + } + if (o instanceof String[] strings) + { + for (String s : strings) + { + if (!StringUtils.isBlank(s)) + { + return true; + } + } + } + return false; + } + + 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 + { + return c == null ? null : c.getConstructor().newInstance(); + } + catch (Exception x) + { + if (x instanceof RuntimeException) + throw ((RuntimeException)x); + else + throw new RuntimeException(x); + } + } + + + protected @NotNull FORM getCommand(HttpServletRequest request) throws Exception + { + FORM command = (FORM) createCommand(); + + if (command instanceof HasViewContext) + ((HasViewContext)command).setViewContext(getViewContext()); + + return command; + } + + + protected @NotNull FORM getCommand() throws Exception + { + return getCommand(getViewContext().getRequest()); + } + + + // + // PARAMETER BINDING + // + // don't assume parameters always come from a request, use PropertyValues interface + // + + public @NotNull BindException defaultBindParameters(FORM form, PropertyValues params) + { + return defaultBindParameters(form, getCommandName(), params); + } + + + public static @NotNull BindException defaultBindParameters(Object form, String commandName, PropertyValues params) + { + /* check for do-it-myself forms */ + if (form instanceof HasBindParameters) + { + return ((HasBindParameters)form).bindParameters(params); + } + + if (form instanceof DynaBean) + { + return simpleBindParameters(form, commandName, params); + } + else + { + return springBindParameters(form, commandName, params); + } + } + + public static @NotNull BindException springBindParameters(Object command, String commandName, PropertyValues params) + { + Predicate allow = command instanceof HasAllowBindParameter allowBP ? allowBP.allowBindParameter() : HasAllowBindParameter.getDefaultPredicate(); + ServletRequestDataBinder binder = new ServletRequestDataBinder(command, commandName); + + String[] fields = binder.getDisallowedFields(); + List fieldList = new ArrayList<>(fields != null ? Arrays.asList(fields) : Collections.emptyList()); + fieldList.addAll(Arrays.asList("class.*", "Class.*", "*.class.*", "*.Class.*")); + binder.setDisallowedFields(fieldList.toArray(new String[] {})); + + ConvertHelper.getPropertyEditorRegistrar().registerCustomEditors(binder); + BindingErrorProcessor defaultBEP = binder.getBindingErrorProcessor(); + binder.setBindingErrorProcessor(getBindingErrorProcessor(defaultBEP)); + binder.setFieldMarkerPrefix(SpringActionController.FIELD_MARKER); + try + { + // most paths probably called getPropertyValuesForFormBinding() already, but this is a public static method, so call it again + binder.bind(getPropertyValuesForFormBinding(params, allow)); + BindException errors = new NullSafeBindException(binder.getBindingResult()); + return errors; + } + catch (InvalidPropertyException x) + { + // 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()); + 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() + ")"); + 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() + ")"); + 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() + ")"); + return errors; + } + } + + + static BindingErrorProcessor getBindingErrorProcessor(final BindingErrorProcessor defaultBEP) + { + return new BindingErrorProcessor() + { + @Override + public void processMissingFieldError(String missingField, BindingResult bindingResult) + { + defaultBEP.processMissingFieldError(missingField, bindingResult); + } + + @Override + public void processPropertyAccessException(PropertyAccessException ex, BindingResult bindingResult) + { + Object newValue = ex.getPropertyChangeEvent().getNewValue(); + if (newValue instanceof String) + newValue = StringUtils.trimToNull((String)newValue); + + // convert NULL conversion errors to required errors + if (null == newValue) + defaultBEP.processMissingFieldError(ex.getPropertyChangeEvent().getPropertyName(), bindingResult); + else + defaultBEP.processPropertyAccessException(ex, bindingResult); + } + }; + } + + + /* + * This binder doesn't have much to offer over the standard spring data binding except that it will + * handle DynaBeans. + */ + public static @NotNull BindException simpleBindParameters(Object command, String commandName, PropertyValues params) + { + Predicate allow = command instanceof HasAllowBindParameter allowBP ? allowBP.allowBindParameter() : HasAllowBindParameter.getDefaultPredicate(); + + BindException errors = new NullSafeBindException(command, "Form"); + + // unfortunately ObjectFactory and BeanObjectFactory are not good about reporting errors + // do this by hand + for (PropertyValue pv : params.getPropertyValues()) + { + String propertyName = pv.getName(); + Object value = pv.getValue(); + if (!allow.test(propertyName)) + continue; + + try + { + Object converted = value; + Class propClass = PropertyUtils.getPropertyType(command, propertyName); + if (null == propClass) + continue; + if (value == null) + { + /* */ + } + else if (propClass.isPrimitive()) + { + converted = ConvertUtils.convert(String.valueOf(value), propClass); + } + else if (propClass.isArray()) + { + if (value instanceof Collection) + value = ((Collection) value).toArray(new String[0]); + else if (!value.getClass().isArray()) + value = new String[] {String.valueOf(value)}; + converted = ConvertUtils.convert((String[])value, propClass); + } + PropertyUtils.setProperty(command, propertyName, converted); + } + catch (ConversionException x) + { + errors.addError(new FieldError(commandName, propertyName, value, true, new String[] {"ConversionError", "typeMismatch"}, null, "Could not convert to value: " + value)); + } + catch (Exception x) + { + errors.addError(new ObjectError(commandName, new String[]{"Error"}, new Object[] {value}, x.getMessage())); + logger.error("unexpected error", x); + } + } + return errors; + } + + @Override + public boolean supports(Class clazz) + { + return getCommandClass().isAssignableFrom(clazz); + } + + + /* for TableViewForm, uses BeanUtils to work with DynaBeans */ + static public class BeanUtilsPropertyBindingResult extends BeanPropertyBindingResult + { + public BeanUtilsPropertyBindingResult(Object target, String objectName) + { + super(target, objectName); + } + + @Override + protected BeanWrapper createBeanWrapper() + { + return new BeanUtilsWrapperImpl((DynaBean)getTarget()); + } + } + + static public class BeanUtilsWrapperImpl extends AbstractPropertyAccessor implements BeanWrapper + { + private Object object; + private boolean autoGrowNestedPaths = false; + private int autoGrowCollectionLimit = 0; + + public BeanUtilsWrapperImpl() + { + // registerDefaultEditors(); + } + + public BeanUtilsWrapperImpl(DynaBean target) + { + this(); + object = target; + } + + @Override + public Object getPropertyValue(String propertyName) throws BeansException + { + try + { + return PropertyUtils.getProperty(object, propertyName); + } + catch (Exception e) + { + throw new NotReadablePropertyException(object.getClass(), propertyName); + } + } + + @Override + public void setPropertyValue(String propertyName, Object value) throws BeansException + { + try + { + PropertyUtils.setProperty(object, propertyName, value); + } + catch (Exception e) + { + throw new NotWritablePropertyException(object.getClass(), propertyName); + } + } + + @Override + public boolean isReadableProperty(String propertyName) + { + return true; + } + + @Override + public boolean isWritableProperty(String propertyName) + { + return true; + } + + @Override + public TypeDescriptor getPropertyTypeDescriptor(String s) throws BeansException + { + return null; + } + + public void setWrappedInstance(Object obj) + { + object = obj; + } + + @Override + public Object getWrappedInstance() + { + return object; + } + + @Override + public Class getWrappedClass() + { + return object.getClass(); + } + + @Override + public PropertyDescriptor[] getPropertyDescriptors() + { + throw new UnsupportedOperationException(); + } + + @Override + public PropertyDescriptor getPropertyDescriptor(String propertyName) throws BeansException + { + throw new UnsupportedOperationException(); + } + + @Override + public void setAutoGrowNestedPaths(boolean b) + { + this.autoGrowNestedPaths = b; + } + + @Override + public boolean isAutoGrowNestedPaths() + { + return this.autoGrowNestedPaths; + } + + @Override + public void setAutoGrowCollectionLimit(int i) + { + this.autoGrowCollectionLimit = i; + } + + @Override + public int getAutoGrowCollectionLimit() + { + return this.autoGrowCollectionLimit; + } + + @Override + public T convertIfNecessary(Object value, Class requiredType) throws TypeMismatchException + { + if (value == null) + return null; + return (T)ConvertUtils.convert(String.valueOf(value), requiredType); + } + + @Override + public T convertIfNecessary(Object value, Class requiredType, MethodParameter methodParam) throws TypeMismatchException + { + return convertIfNecessary(value, requiredType); + } + } + + /** + * @return a map from form element name to uploaded files + */ + protected Map getFileMap() + { + if (getViewContext().getRequest() instanceof MultipartHttpServletRequest) + return ((MultipartHttpServletRequest)getViewContext().getRequest()).getFileMap(); + return Collections.emptyMap(); + } + + protected List getAttachmentFileList() + { + return SpringAttachmentFile.createList(getFileMap()); + } + + public boolean isRobot() + { + return _robot; + } + + public boolean isPrint() + { + return _print; + } + + public boolean isDebug() + { + return _debug; + } + + public @NotNull Class getCommandClass() + { + if (null == _commandClass) + throw new IllegalStateException("NULL _commandClass in " + getClass().getName()); + return _commandClass; + } + + public void setCommandClass(@NotNull Class commandClass) + { + _commandClass = commandClass; + } + + protected final @NotNull Object createCommand() + { + return BeanUtils.instantiateClass(getCommandClass()); + } + + public void setCommandName(String commandName) + { + _commandName = commandName; + } + + public String getCommandName() + { + return _commandName; + } + + /** + * Cacheable resources can calculate a last modified timestamp to send to the browser. + */ + protected long getLastModified(FORM form) + { + return Long.MIN_VALUE; + } + + /** + * Cacheable resources can calculate an ETag header to send to the browser. + */ + protected String getETag(FORM form) + { + return null; + } +} From 1687b4d21d446d3defee062be64f2efa176bb2f3 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Tue, 28 Oct 2025 10:16:46 -0700 Subject: [PATCH 06/19] typo --- api/src/org/labkey/api/data/ConvertHelper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/data/ConvertHelper.java b/api/src/org/labkey/api/data/ConvertHelper.java index 078b807ae9a..ee0be861dfa 100644 --- a/api/src/org/labkey/api/data/ConvertHelper.java +++ b/api/src/org/labkey/api/data/ConvertHelper.java @@ -862,7 +862,7 @@ public Object convert(Class type, Object value) if (value instanceof String s) { // If the value is wrapped with { and }, let the beanutils converter tokenize the values. - // This let's us handle Issue 5340 while allowing multi-value strings to be parsed. + // This lets us handle Issue 5340 while allowing multi-value strings to be parsed. if (s.startsWith("{") && s.endsWith("}")) return _nested.convert(type, value); } From d6fd80397c38fb50d92becc9c507a1ca9db45c0d Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Wed, 29 Oct 2025 10:09:03 -0700 Subject: [PATCH 07/19] remove stray change --- api/src/org/labkey/api/util/HtmlString.java | 1 - 1 file changed, 1 deletion(-) diff --git a/api/src/org/labkey/api/util/HtmlString.java b/api/src/org/labkey/api/util/HtmlString.java index 9a669260f4f..54943a4d771 100644 --- a/api/src/org/labkey/api/util/HtmlString.java +++ b/api/src/org/labkey/api/util/HtmlString.java @@ -30,7 +30,6 @@ public final class HtmlString implements SafeToRender, DOM.Renderable, Comparabl { // Helpful constants for convenience (and efficiency) public static final HtmlString EMPTY_STRING = HtmlString.of(""); - public static final HtmlString SP = HtmlString.of(" "); public static final HtmlString NBSP = HtmlString.unsafe(" "); public static final HtmlString NDASH = HtmlString.unsafe("–"); public static final HtmlString BR = HtmlString.unsafe("
"); From 4032cc31e373ca5417a11fc06d63d8c6754d8138 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Wed, 29 Oct 2025 11:07:32 -0700 Subject: [PATCH 08/19] CR feedback use setValueToBind() in more places --- .../org/labkey/api/data/TableViewForm.java | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index 7321b7185cb..4f7c2d84800 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -73,8 +73,8 @@ public class TableViewForm extends ViewForm implements HasBindParameters { private static final Logger _log = LogHelper.getLogger(TableViewForm.class, "Table operation warnings"); - // This is called "stringValues" as this is expected to come from a form POST (but it was never just string valure) - // However, it can also be String[] abd other types + // This is called "stringValues" as this is expected to come from a form POST (but it was never just a string value) + // However, it can also be String[] and other types protected Map _stringValues = new CaseInsensitiveHashMap<>(); protected Map _values = null; protected Object _oldValues; @@ -301,8 +301,8 @@ public void setPkVals(String s) } else { - // TODO replace with new splitStringToValues - // setPkVals(PageFlowUtil.splitStringToValuesForImport(s)); + // CONSIDER We should support PK column names with commas. We should replace with better parser. + // something like: setPkVals(PageFlowUtil.splitStringToValuesForImport(s)); setPkVals(s.split(",")); } } @@ -534,7 +534,7 @@ public boolean hasTypedValue(ColumnInfo column) public void setTypedValue(String propName, Object val) { getTypedValues().put(propName, val); - // setValueToBind() but w/o invalidate. + // We don't use setValueToBind() here because we want to avoid its side effect of clearing _values // To convert or not to convert??? _stringValues.put(propName, val); } @@ -649,8 +649,9 @@ public void setTypedValues(Map values, boolean merge) public void setValuesToBind(Map strings) { _stringValues.clear(); - _stringValues.putAll(strings); _values = null; + for (Map.Entry e : strings.entrySet()) + setValueToBind(e.getKey(), e.getValue()); } public Map getValuesToBind() @@ -658,24 +659,18 @@ public Map getValuesToBind() return _stringValues; } -// public boolean contains(ColumnInfo col) -// { -// return _stringValues.containsKey(getFormFieldName(col)); -// } - public boolean contains(DisplayColumn col, RenderContext ctx) { return _stringValues.containsKey(col.getFormFieldName(ctx)); } - public String getAsString(String propName) + public @Nullable String getAsString(@NotNull String propName) { Object value = _stringValues.get(propName); if (value == null || value instanceof String) return (String)value; - if (value instanceof String[]) + if (value instanceof String[] arr) { - String[] arr = (String[]) value; if (arr.length == 0) return null; } @@ -839,7 +834,7 @@ public void setViewContext(@NotNull ViewContext context) for (PropertyValue pv : params.getPropertyValues()) { - _stringValues.put(pv.getName(), pv.getValue()); + setValueToBind(pv.getName(), pv.getValue()); } BindException errors = new NullSafeBindException(new BaseViewAction.BeanUtilsPropertyBindingResult(this, "form")); From 164a550c13c73d035c099dc76c12fbe0cf572ea3 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Wed, 29 Oct 2025 11:42:40 -0700 Subject: [PATCH 09/19] CR feedback Collections.unmodifiableMap() --- api/src/org/labkey/api/data/TableViewForm.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index 4f7c2d84800..c622d0e0fd5 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -533,7 +533,9 @@ public boolean hasTypedValue(ColumnInfo column) public void setTypedValue(String propName, Object val) { - getTypedValues().put(propName, val); + // call _populate() if necessary + getTypedValues(); + _values.put(propName, val); // We don't use setValueToBind() here because we want to avoid its side effect of clearing _values // To convert or not to convert??? _stringValues.put(propName, val); @@ -552,7 +554,7 @@ public Map getTypedValues() if (null == _values) populateValues(null); - return _values; + return Collections.unmodifiableMap(_values); } /** @@ -656,7 +658,7 @@ public void setValuesToBind(Map strings) public Map getValuesToBind() { - return _stringValues; + return Collections.unmodifiableMap(_stringValues); } public boolean contains(DisplayColumn col, RenderContext ctx) From f4b52c3ae1a01e964b367f65e88d8a3f53eac2b4 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Wed, 29 Oct 2025 12:02:43 -0700 Subject: [PATCH 10/19] CR feedback StrintUtils.trimToNull() --- api/src/org/labkey/api/data/TableViewForm.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index c622d0e0fd5..8c7855ae686 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -388,7 +388,7 @@ public Object getValueToBind(String propName) if (null == value) return null; if (value instanceof String str) - return StringUtils.isEmpty(str) ? null : str; + return StringUtils.trimToNull(str); return value; } From 55188978fa2a8f643f9027afb349eceed4f030a9 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Wed, 29 Oct 2025 18:11:44 -0700 Subject: [PATCH 11/19] string array parsing/formating ala Google Sheets --- api/src/org/labkey/api/util/PageFlowUtil.java | 202 +++++++++++++++++- 1 file changed, 195 insertions(+), 7 deletions(-) diff --git a/api/src/org/labkey/api/util/PageFlowUtil.java b/api/src/org/labkey/api/util/PageFlowUtil.java index c135c775455..feaf2515c73 100644 --- a/api/src/org/labkey/api/util/PageFlowUtil.java +++ b/api/src/org/labkey/api/util/PageFlowUtil.java @@ -24,6 +24,7 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Logger; import org.apache.tika.detect.DefaultDetector; import org.apache.tika.io.TikaInputStream; @@ -123,6 +124,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.text.ParsePosition; import java.util.ArrayList; import java.util.Arrays; import java.util.BitSet; @@ -146,7 +148,6 @@ import java.util.stream.Stream; import static org.apache.commons.lang3.StringUtils.isNotBlank; -import static org.apache.commons.lang3.StringUtils.startsWith; import static org.labkey.api.data.DataRegion.LAST_FILTER_PARAM; import static org.labkey.api.util.DOM.A; import static org.labkey.api.util.DOM.Attribute.height; @@ -649,7 +650,7 @@ public static String wafDecode(@Nullable String encoded) if (StringUtils.isBlank(encoded)) return null; - if (StringUtils.startsWith(encoded, WAF_PREFIX)) + if (Strings.CS.startsWith(encoded, WAF_PREFIX)) { encoded = encoded.substring(WAF_PREFIX.length()); if (!Pattern.matches("[A-Za-z0-9+/=]*", encoded)) @@ -707,7 +708,7 @@ public static String wafEncode(@Nullable String plain) return ""; String encoded = URLEncoder.encode(s, StringUtilsLabKey.DEFAULT_CHARSET); - encoded = StringUtils.replace(encoded, "+", "%20"); + encoded = Strings.CS.replace(encoded, "+", "%20"); if (decodeUnreservedMarks) { @@ -715,7 +716,7 @@ public static String wafEncode(@Nullable String plain) // does not. Here we decode these marks so that we match encodeURIComponent() encoding. // See https://stackoverflow.com/a/607403 for (var entry : DECODE_UNRESERVED_MARKS.entrySet()) - encoded = StringUtils.replace(encoded, entry.getValue(), entry.getKey()); + encoded = Strings.CS.replace(encoded, entry.getValue(), entry.getKey()); } return encoded; @@ -940,7 +941,7 @@ public static void prepareResponseForFile(HttpServletResponse response, Map>> 32)); + result = 31 * result + Long.hashCode(modified); return result; } } @@ -2053,6 +2054,7 @@ public static String validateHtml(String html, Collection errors, boolea } /** validate an html fragment */ + @SuppressWarnings("HttpUrlsUsage") public static String validateHtml(String html, Collection errors, Collection scriptWarnings) { if (!errors.isEmpty() || (null != scriptWarnings && !scriptWarnings.isEmpty())) @@ -2587,6 +2589,121 @@ public static String joinValuesToString(@NotNull List values, char delim .collect(Collectors.joining(String.valueOf(delimiter))); } + + private static char peek(ParsePosition p, char[] s) + { + int i=p.getIndex(); + return i < s.length-1 ? s[i+1] : '\0'; + } + + private static char next(ParsePosition p, char[] s) + { + int i=p.getIndex(); + if (i >= s.length-1) + return '\0'; + p.setIndex(i+1); + return s[i+1]; + } + + public static List splitStringToValuesForImport(String str) + { + enum STATE {BEFORETOKEN, INTOKEN, INQUOTEDTOKEN, AFTERTOKEN} + + if (str == null) + return null; + List result = new ArrayList<>(); + StringBuilder currentToken = new StringBuilder(); + STATE state = STATE.BEFORETOKEN; + char[] charArray = str.toCharArray(); + ParsePosition pos = new ParsePosition(-1); + + char c; + do + { + c = next(pos, charArray); + switch (state) + { + case BEFORETOKEN -> + { + assert currentToken.isEmpty(); + if (Character.isWhitespace(c)) + continue; + if (c == '"') + state = STATE.INQUOTEDTOKEN; + else if (c == ',' || c == '\0') + result.add(currentToken.toString()); + else + { + currentToken.append(c); + state = STATE.INTOKEN; + } + } + case AFTERTOKEN -> + { + assert currentToken.length() == 0; + if (Character.isWhitespace(c)) + continue; + if (c == ',' || c == '\0') + state = STATE.BEFORETOKEN; + else + throw new ConversionException("Badly formatted list of strings"); + } + case INTOKEN -> + { + if (c == ',' || c == '\0') + { + result.add(currentToken.toString().trim()); + currentToken.setLength(0); + state = STATE.BEFORETOKEN; + } + else + { + currentToken.append(c); + } + } + case INQUOTEDTOKEN -> + { + if (c == '\0') + throw new ConversionException("Unterminated quoted string"); + else if (c != '"') + { + currentToken.append(c); + } + else if (peek(pos, charArray) == '"') + { + next(pos, charArray); + currentToken.append('"'); + } + else + { + result.add(currentToken.toString()); + currentToken.setLength(0); + state = STATE.AFTERTOKEN; + } + } + } + } while (0 != c); + return result; + } + + // Google Sheets compatible version of joinValuesToString() + public static String joinValuesToStringForExport(@NotNull List values) + { + return values.stream() + .map(value -> null==value ? "" : shouldEscapeForExport(value) ? "\"" + Strings.CS.replace(value,"\"", "\"\"") + "\"": value) + .collect(Collectors.joining(", ")); + } + + private static boolean shouldEscapeForExport(@NotNull String value) + { + if (value.isEmpty()) + return true; + if (Character.isWhitespace(value.charAt(0)) || Character.isWhitespace(value.charAt(value.length()-1))) + return true; + return StringUtils.containsAny(value,",\""); + } + + /** * Issue 52925: App export to csv/tsv ignores filter with column containing double quote * Issue 52119: App issues with assay run properties with special characters @@ -2759,6 +2876,7 @@ public void testEncodeURIComponent() @Test public void testRobot() { + @SuppressWarnings("HttpUrlsUsage") List bots = Arrays.asList( "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html", "Mozilla/5.0 (compatible; bingbot/2.0; +http://www.bing.com/bingbot.htm)", @@ -2912,6 +3030,76 @@ public void testJoinAndSplit() } @Test + public void testGoogleSheetMultiValue() + { + var googleMultiChoice = """ + Option 1, Option 2, "A""quote", "B,comma", C\\backslash + """.trim(); + var expectedList = List.of("Option 1", "Option 2", "A\"quote", "B,comma", "C\\backslash"); + var list = splitStringToValuesForImport(googleMultiChoice); + assertEquals(expectedList.size(), list.size()); + for (int i=0 ; i null or List.of() + assertNull(splitStringToValuesForImport(null)); + // should "" -> List.of() or List.of("") + assertEquals(List.of(""), splitStringToValuesForImport("")); + assertEquals(List.of("",""), splitStringToValuesForImport(",")); + assertEquals(List.of("","",""), splitStringToValuesForImport(", ,")); + + List> quickTests = Arrays.asList( + List.of(""), + List.of("",""), + List.of("","",""), + List.of(" A", "B ", "C D"), + List.of(",A", "B,", "C,D"), + List.of("\tA", "B\t", "C\tD"), + List.of("\"A", "B\"", "C\"D") + ); + for (List test : quickTests) + assertEquals(test, splitStringToValuesForImport(joinValuesToStringForExport(test))); + } + + @Test + public void testGoogleSheetMultiValueBad() + { + try + { + splitStringToValuesForImport("\"A\"dreck,B,C"); // unescaped " + fail("Expected exception"); + } + catch (ConversionException e) + { + // expected + } + + try + { + splitStringToValuesForImport("A,\"B,C"); // unterminated " + fail("Expected exception"); + } + catch (ConversionException e) + { + // expected + } + } + + + @Test public void encodePath() { assertEquals("a/b/c", PageFlowUtil.encodePath("a/b/c")); From 3429dbbc1cf5f100281e5991841af4330e75696f Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Thu, 30 Oct 2025 12:02:56 -0700 Subject: [PATCH 12/19] StringArrayConverter uses splitStringToValuesForImport() Support [] as a form field suffix --- .../org/labkey/api/data/ConvertHelper.java | 20 +-- .../org/labkey/api/data/TableViewForm.java | 65 ++++++-- .../core/view/TableViewFormTestCase.java | 157 ++++++++++++++++-- 3 files changed, 203 insertions(+), 39 deletions(-) diff --git a/api/src/org/labkey/api/data/ConvertHelper.java b/api/src/org/labkey/api/data/ConvertHelper.java index ee0be861dfa..a8918d2bc9a 100644 --- a/api/src/org/labkey/api/data/ConvertHelper.java +++ b/api/src/org/labkey/api/data/ConvertHelper.java @@ -59,6 +59,7 @@ import org.labkey.api.settings.LookAndFeelProperties; import org.labkey.api.util.DateUtil; import org.labkey.api.util.GUID; +import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.ReturnURLString; import org.labkey.api.util.SimpleTime; import org.labkey.api.util.SkipMothershipLogging; @@ -71,7 +72,6 @@ import org.springframework.beans.PropertyEditorRegistrar; import org.springframework.beans.PropertyEditorRegistry; -import java.awt.*; import java.beans.PropertyEditorSupport; import java.io.File; import java.math.BigDecimal; @@ -83,7 +83,9 @@ import java.sql.Timestamp; import java.util.Calendar; import java.util.Date; +import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; @@ -146,7 +148,7 @@ protected void register() _register(new NullSafeConverter(new CharacterConverter()), Character.class); _register(new CharacterConverter(), Character.TYPE); _register(new NullSafeConverter(new ClassConverter()), Class.class); - _register(new ColorConverter(), Color.class); + _register(new ColorConverter(), java.awt.Color.class); _register(new ContainerConverter(), Container.class); _register(new GuidConverter(), GUID.class); _register(new InfDoubleConverter(), Double.TYPE); @@ -838,11 +840,11 @@ public static class ColorConverter implements Converter public Object convert(Class type, Object value) { if (value == null) - return Color.WHITE; + return java.awt.Color.WHITE; if (value.getClass() == type) return value; String s = value.toString(); - return Color.decode(s); + return java.awt.Color.decode(s); } } @@ -859,14 +861,10 @@ public Object convert(Class type, Object value) return null; if (value instanceof String[]) return value; + if (value instanceof List l) + return l.stream().map(v -> Objects.toString(v, null)).toArray(String[]::new); if (value instanceof String s) - { - // If the value is wrapped with { and }, let the beanutils converter tokenize the values. - // This lets us handle Issue 5340 while allowing multi-value strings to be parsed. - if (s.startsWith("{") && s.endsWith("}")) - return _nested.convert(type, value); - } - + return PageFlowUtil.splitStringToValuesForImport(s).toArray(String[]::new); // Otherwise, treat it as a single element string array. return new String[] {String.valueOf(value)}; } diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index 8c7855ae686..6facc652b81 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -20,7 +20,6 @@ import org.apache.commons.beanutils.ConversionException; import org.apache.commons.beanutils.ConvertUtils; import org.apache.commons.beanutils.PropertyUtils; -import org.apache.commons.collections4.IteratorUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; @@ -44,6 +43,7 @@ import org.labkey.api.view.UnauthorizedException; import org.labkey.api.view.ViewContext; import org.labkey.api.view.ViewForm; +import org.springframework.beans.MutablePropertyValues; import org.springframework.beans.PropertyValue; import org.springframework.beans.PropertyValues; import org.springframework.validation.BindException; @@ -63,6 +63,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; /** * Basic form for handling posts into views. @@ -803,25 +804,53 @@ public void setViewContext(@NotNull ViewContext context) } } - @Override - public @NotNull BindException bindParameters(PropertyValues params) + /** Handle @ prefix and [] suffix + * "@field" indicates that if "field" is missing, it should be treated as "field=0" + * "@field[] indicates that value should be treated as an array even if only one value is present + *
+ * client _could_ post both "myfield=" and "myfield[]=", but that's a client bug + */ + PropertyValues preprocessPropertyValues(PropertyValues params) { - /* - * Checkboxes are weird. If set to FALSE they don't post - * at all. So impossible to tell difference between values - * that weren't on the html form at all and ones that were set to false - * by the user. - * To fix this each checkbox posts its name in a hidden field - * We set them all to false and spring will overwrite with true - * if they are set. - */ - HttpServletRequest request = getRequest(); + // we can usually just return params + if (params.stream().noneMatch(e -> e.getName().endsWith("[]") || e.getName().startsWith(SpringActionController.FIELD_MARKER))) + return params; - // handle Spring style markers - IteratorUtils.asIterator(request.getParameterNames()).forEachRemaining(name -> { - if (name.startsWith(SpringActionController.FIELD_MARKER)) - setValueToBind(name.substring(SpringActionController.FIELD_MARKER.length()), "0"); - }); + Set names = params.stream().map(PropertyValue::getName).collect(Collectors.toSet()); + var ret = new MutablePropertyValues(); + for (var orig : params) + { + var copy = orig; + if (orig.getName().startsWith(SpringActionController.FIELD_MARKER)) + { + if (names.contains(orig.getName().substring(1))) + continue; + copy = new PropertyValue(orig.getName().substring(1), "0"); + } + else if (orig.getName().endsWith("[]") && orig.getValue()!=null) + { + var value = orig.getValue(); + var convertedValue = value; + if (List.class.isAssignableFrom(value.getClass())) + { + convertedValue = ((List) value).toArray(new Object[0]); + } + if (!value.getClass().isArray()) + { + convertedValue = Array.newInstance(value.getClass(), 1); + Array.set(convertedValue, 0, value); + } + copy = new PropertyValue(orig.getName().substring(0, orig.getName().length() - 2), convertedValue); + } + ret.addPropertyValue(copy); + } + return ret; + } + + @Override + public @NotNull BindException bindParameters(PropertyValues paramsIn) + { + var params = preprocessPropertyValues(paramsIn); // handle binding of base class ReturnURLForm PropertyValue pvReturn = params.getPropertyValue(ActionURL.Param.returnUrl.toString()); diff --git a/core/src/org/labkey/core/view/TableViewFormTestCase.java b/core/src/org/labkey/core/view/TableViewFormTestCase.java index 0ee5366a76e..5fa07adeec3 100644 --- a/core/src/org/labkey/core/view/TableViewFormTestCase.java +++ b/core/src/org/labkey/core/view/TableViewFormTestCase.java @@ -19,6 +19,8 @@ import org.junit.Assert; import org.junit.Test; import org.labkey.api.action.NullSafeBindException; +import org.labkey.api.action.SpringActionController; +import org.labkey.api.data.BeanViewForm; import org.labkey.api.data.Container; import org.labkey.api.data.TableViewForm; import org.labkey.api.data.TestSchema; @@ -27,11 +29,15 @@ import org.labkey.api.util.TestContext; import org.labkey.api.view.HttpView; import org.labkey.api.view.ViewContext; +import org.springframework.beans.MutablePropertyValues; import org.springframework.validation.BindException; import java.sql.SQLException; import java.sql.Timestamp; import java.util.Date; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; public class TableViewFormTestCase extends Assert { @@ -39,7 +45,6 @@ public class TableViewFormTestCase extends Assert public void testBasic() { TestForm tf = new TestForm(); - tf.setViewContext(HttpView.currentContext()); TestContext ctx = TestContext.get(); Assert.assertEquals(ctx.getRequest().getUserPrincipal(), tf.getUser()); @@ -47,7 +52,7 @@ public void testBasic() //Test date handling tf.setValueToBind("datetimeNotNull", "2004-06-20"); Date dt = (Date) tf.getTypedValue("datetimeNotNull"); - Assert.assertTrue("Date get", dt.equals(new Timestamp(DateUtil.parseISODateTime("2004-06-20")))); + Assert.assertEquals("Date get", dt, new Timestamp(DateUtil.parseISODateTime("2004-06-20"))); //Should turn empty strings into nulls tf.setValueToBind("text", ""); @@ -61,11 +66,141 @@ public void testBasic() Assert.assertEquals(20, tf.getTypedValue("rowId")); } + @SuppressWarnings("unused") + public static class BindBean + { + private String[] strArray; + private Boolean boolValue; + + public String[] getStrArray() + { + return strArray; + } + + public void setStrArray(String[] strArray) + { + this.strArray = strArray; + } + + public Boolean getBoolValue() + { + return boolValue; + } + + public void setBoolValue(Boolean boolValue) + { + this.boolValue = boolValue; + } + } + + @Test + public void testArray() + { + // actually using a BeanVieForm because SQL Server + BeanViewForm form; + MutablePropertyValues mpv; + Map typed; + String[] strArray; + + // test comma parsing + form = new BeanViewForm<>(BindBean.class, null); + mpv = new MutablePropertyValues(); + mpv.add("strArray", "Option 1, Option 2"); + form.bindParameters(mpv); + typed = form.getTypedValues(); + assertEquals(1, typed.size()); + assertTrue(typed.get("strArray") instanceof String[]); + strArray = (String[]) typed.get("strArray"); + assertEquals(2, strArray.length); + assertEquals("Option 1", strArray[0]); + assertEquals("Option 2", strArray[1]); + + // test disambiguate one select + // this is explicitly an array of size 1, so no parsing + form = new BeanViewForm<>(BindBean.class, null); + mpv = new MutablePropertyValues(); + mpv.add("strArray[]", "Option 1, Option 2"); + form.bindParameters(mpv); + typed = form.getTypedValues(); + assertEquals(1, typed.size()); + assertTrue(typed.get("strArray") instanceof String[]); + strArray = (String[]) typed.get("strArray"); + assertEquals(1, strArray.length); + assertEquals("Option 1, Option 2", strArray[0]); + } + + + @Test + public void testBoolean() + { + TestForm form; + MutablePropertyValues mpv; + Map typed; + + form = new TestForm(); + mpv = new MutablePropertyValues(); + mpv.add("other", "1"); + form.bindParameters(mpv); + typed = form.getTypedValues(); + assertFalse(typed.containsKey("bitNull")); + + form = new TestForm(); + mpv = new MutablePropertyValues(); + mpv.add("bitNull", "1"); + form.bindParameters(mpv); + typed = form.getTypedValues(); + assertEquals(1, typed.size()); + assertTrue(typed.get("bitNull") instanceof Boolean); + assertTrue((Boolean) typed.get("bitNull")); + + form = new TestForm(); + mpv = new MutablePropertyValues(); + mpv.add("bitNull", "0"); + form.bindParameters(mpv); + typed = form.getTypedValues(); + assertEquals(1, typed.size()); + assertTrue(typed.get("bitNull") instanceof Boolean); + assertFalse((Boolean) typed.get("bitNull")); + + form = new TestForm(); + mpv = new MutablePropertyValues(); + mpv.add(SpringActionController.FIELD_MARKER+"bitNull", "1"); + form.bindParameters(mpv); + typed = form.getTypedValues(); + assertEquals(1, typed.size()); + assertTrue(typed.get("bitNull") instanceof Boolean); + assertFalse((Boolean) typed.get("bitNull")); + + form = new TestForm(); + mpv = new MutablePropertyValues(); + mpv.add(SpringActionController.FIELD_MARKER+"bitNull", "1"); + mpv.add("bitNull", "1"); + form.bindParameters(mpv); + typed = form.getTypedValues(); + assertEquals(1, typed.size()); + assertTrue(typed.get("bitNull") instanceof Boolean); + assertTrue((Boolean) typed.get("bitNull")); + } + + + @Test public void testErrorHandling() { TestForm tf = new TestForm(); - tf.setViewContext(HttpView.currentContext()); //Should be invalid because of null fields. //BUG: Not differentiating between insert & update cases. @@ -97,12 +232,10 @@ public void testDbOperations() throws SQLException { Container test = JunitUtil.getTestContainer(); - ViewContext ctx = new ViewContext(HttpView.currentContext()); + ViewContext ctx = new ViewContext(Objects.requireNonNull(HttpView.currentContext())); ctx.setContainer(test); TestForm tf = new TestForm(); - tf.setViewContext(ctx); - tf.setValueToBind("datetimeNotNull", "2004-06-20"); tf.setValueToBind("bitNotNull", "1"); tf.setValueToBind("intNotNull", "20"); @@ -116,9 +249,12 @@ public void testDbOperations() throws SQLException Date createdDate = (Date) tf.getTypedValue("created"); //Make sure date->string->date comes out right... - tf.setValueToBind("datetimeNotNull", tf.getAsString("created")); - tf.setValueToBind("text", "Second test record"); - tf.getValuesToBind().remove("rowId"); + Map copy = new HashMap<>(tf.getValuesToBind()); + copy.remove("rowid"); + copy.put("datetimeNotNull", tf.getAsString("created")); + copy.put("text", "Second test record"); + tf = new TestForm(); + tf.setValuesToBind(copy); tf.doInsert(); Assert.assertEquals("Date time roundtrip: ", createdDate.getTime(), ((Date) tf.getTypedValue("datetimeNotNull")).getTime()); tf.doDelete(); @@ -129,7 +265,7 @@ public void testDbOperations() throws SQLException tf.doDelete(); tf.forceReselect(); - Assert.assertTrue("deleted", 1 == tf.getTypedValues().size()); + Assert.assertEquals("deleted", 1, tf.getTypedValues().size()); } public static class TestForm extends TableViewForm @@ -137,6 +273,7 @@ public static class TestForm extends TableViewForm public TestForm() { super(TestSchema.getInstance().getTableInfoTestTable()); + setViewContext(Objects.requireNonNull(HttpView.currentContext())); } } } From 40f0588833078b283a09ac1fd44c7b65204d857b Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Thu, 30 Oct 2025 12:20:05 -0700 Subject: [PATCH 13/19] public static --- api/src/org/labkey/api/data/TableViewForm.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index 6facc652b81..3f82dc0cc88 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -810,7 +810,7 @@ public void setViewContext(@NotNull ViewContext context) *
* client _could_ post both "myfield=" and "myfield[]=", but that's a client bug */ - PropertyValues preprocessPropertyValues(PropertyValues params) + public static PropertyValues preprocessPropertyValues(PropertyValues params) { // we can usually just return params if (params.stream().noneMatch(e -> e.getName().endsWith("[]") || e.getName().startsWith(SpringActionController.FIELD_MARKER))) From 98dac589b367ddddc34efa2902b0e6a4970329a8 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Thu, 30 Oct 2025 17:42:22 -0700 Subject: [PATCH 14/19] PropertyType.MULTI_CHOICE --- api/src/org/labkey/api/data/ColumnInfo.java | 5 +- .../api/data/ColumnRenderProperties.java | 32 +- .../api/data/ColumnRenderPropertiesImpl.java | 13 +- api/src/org/labkey/api/data/DataColumn.java | 30 +- .../org/labkey/api/data/DisplayColumn.java | 9 +- api/src/org/labkey/api/data/JdbcType.java | 4 +- api/src/org/labkey/api/data/MultiChoice.java | 510 ++++++++++++++++++ api/src/org/labkey/api/data/SQLFragment.java | 5 + api/src/org/labkey/api/data/Table.java | 2 +- .../org/labkey/api/data/TableViewForm.java | 10 +- api/src/org/labkey/api/exp/PropertyType.java | 51 ++ .../labkey/api/exp/property/DomainKind.java | 1 + api/src/org/labkey/api/util/HtmlString.java | 1 + .../api/property/PropertyServiceImpl.java | 5 +- .../api/property/TextChoiceValidator.java | 38 +- 15 files changed, 667 insertions(+), 49 deletions(-) create mode 100644 api/src/org/labkey/api/data/MultiChoice.java diff --git a/api/src/org/labkey/api/data/ColumnInfo.java b/api/src/org/labkey/api/data/ColumnInfo.java index c2eb03e754a..77da3bd84a3 100644 --- a/api/src/org/labkey/api/data/ColumnInfo.java +++ b/api/src/org/labkey/api/data/ColumnInfo.java @@ -31,7 +31,6 @@ import org.labkey.api.util.StringExpression; import org.labkey.data.xml.ColumnType; -import java.beans.Introspector; import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; @@ -56,6 +55,10 @@ else if (colInfo.getPropertyType() == PropertyType.ATTACHMENT) { return new AttachmentDisplayColumn(colInfo); } + if (JdbcType.ARRAY == colInfo.getJdbcType() && PropertyType.MULTI_CHOICE == colInfo.getPropertyType()) + { + return new MultiChoice.DisplayColumn(colInfo); + } DataColumn dataColumn = new DataColumn(colInfo); if (colInfo.getPropertyType() == PropertyType.MULTI_LINE) diff --git a/api/src/org/labkey/api/data/ColumnRenderProperties.java b/api/src/org/labkey/api/data/ColumnRenderProperties.java index 57b2a8ec140..75070c71a0c 100644 --- a/api/src/org/labkey/api/data/ColumnRenderProperties.java +++ b/api/src/org/labkey/api/data/ColumnRenderProperties.java @@ -35,6 +35,7 @@ import java.math.BigDecimal; import java.text.DecimalFormatSymbols; import java.util.Date; +import java.util.List; import java.util.Set; import java.util.function.Function; @@ -168,8 +169,9 @@ else if (java.sql.Date.class.isAssignableFrom(javaClass)) return "Date"; else if (Date.class.isAssignableFrom(javaClass)) return "Date and Time"; - else - return "Other"; + else if (List.class.isAssignableFrom(javaClass) || javaClass.isArray()) + return "Array"; + return "Other"; } /** Don't return TYPEs just real java objects */ @@ -381,18 +383,20 @@ static Function getDefaultConvertFn(ColumnRenderProperties col) final var defaultUnit = col.getDisplayUnit(); final @NotNull var jdbcType = col.getJdbcType(); - if (null == defaultUnit) + if (null != defaultUnit) + return defaultUnit::convert; + + if (PropertyType.MULTI_CHOICE == col.getPropertyType()) + return MultiChoice.Converter.getInstance(); + + return (value) -> { - return (value) -> - { - // quick check for unnecessary conversion - if (value == null || javaClass == value.getClass()) - return value; - if (value instanceof CharSequence) - ConvertUtils.convert(value.toString(), javaClass); - return jdbcType.convert(value); - }; - } - return defaultUnit::convert; + // quick check for unnecessary conversion + if (value == null || javaClass == value.getClass()) + return value; + if (value instanceof CharSequence) + ConvertUtils.convert(value.toString(), javaClass); + return jdbcType.convert(value); + }; } } diff --git a/api/src/org/labkey/api/data/ColumnRenderPropertiesImpl.java b/api/src/org/labkey/api/data/ColumnRenderPropertiesImpl.java index 43b7b35651e..aa30afeb5d1 100644 --- a/api/src/org/labkey/api/data/ColumnRenderPropertiesImpl.java +++ b/api/src/org/labkey/api/data/ColumnRenderPropertiesImpl.java @@ -791,19 +791,22 @@ public Class getJavaClass(boolean isNullable) public static Class defaultJavaClass(ColumnRenderProperties col, boolean isNullable) { - Class ret; - boolean isNumeric; PropertyType pt = col.getPropertyType(); + JdbcType jdbcType = col.getJdbcType(); + boolean isNumeric; + Class ret; + if (pt != null) { - ret = pt.getJavaType(); + if (JdbcType.ARRAY == jdbcType && PropertyType.MULTI_CHOICE == pt) + return MultiChoice.Array.class; isNumeric = pt.getJdbcType().isNumeric(); + ret = pt.getJavaType(); } else { - JdbcType jdbcType = col.getJdbcType(); - ret =jdbcType.getJavaClass(isNullable); isNumeric = jdbcType.isNumeric(); + ret = jdbcType.getJavaClass(isNullable); } if (isNumeric) { diff --git a/api/src/org/labkey/api/data/DataColumn.java b/api/src/org/labkey/api/data/DataColumn.java index 7245fbbdb25..377392d4664 100644 --- a/api/src/org/labkey/api/data/DataColumn.java +++ b/api/src/org/labkey/api/data/DataColumn.java @@ -61,6 +61,8 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; +import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Objects; @@ -710,7 +712,7 @@ else if (_inputType.equalsIgnoreCase("checkbox")) { IPropertyValidator textChoiceValidator = PropertyService.get().getValidatorForColumn(_boundColumn, PropertyValidatorType.TextChoice); if (textChoiceValidator != null) - renderTextChoiceFormInput(out, formFieldName, value, strVal, disabledInput, textChoiceValidator); + renderTextChoiceFormInput(out, formFieldName, value, List.of(strVal), disabledInput, textChoiceValidator); else renderTextFormInput(out, formFieldName, value, strVal, disabledInput); } @@ -735,7 +737,8 @@ else if (_inputType.equalsIgnoreCase("checkbox")) return ctx.getForm() == null || col == null ? HtmlString.EMPTY_STRING : ctx.getErrors(col); } - private void renderSelectFormInput(HtmlWriter out, String formFieldName, Object value, String strVal, boolean disabledInput, NamedObjectList entryList) + + private void renderSelectFormInput(HtmlWriter out, String formFieldName, Object value, List strValues, boolean disabledInput, NamedObjectList entryList) { SelectBuilder select = new SelectBuilder() .disabled(disabledInput) @@ -747,12 +750,15 @@ private void renderSelectFormInput(HtmlWriter out, String formFieldName, Object // add empty option options.add(new OptionBuilder().build()); + Set selectedValues = strValues.isEmpty() ? Set.of() : + strValues.size()==1 ? (null == strValues.get(0) ? Set.of() : Set.of(strValues.get(0))) : + new HashSet<>(strValues); for (NamedObject entry : entryList) { String entryName = entry.getName(); OptionBuilder option = new OptionBuilder() - .selected(isSelectInputSelected(entryName, value, strVal)) - .value(entryName); + .selected(selectedValues.contains(entryName)) + .value(entryName); if (null != entry.getObject()) option.label(getSelectInputDisplayValue(entry)); @@ -767,22 +773,18 @@ private void renderSelectFormInput(HtmlWriter out, String formFieldName, Object renderHiddenFormInput(out, formFieldName, value); } - private void renderTextChoiceFormInput(HtmlWriter out, String formFieldName, Object value, String strVal, boolean disabledInput, IPropertyValidator textChoiceValidator) + protected void renderTextChoiceFormInput(HtmlWriter out, String formFieldName, Object value, List strValues, boolean disabledInput, IPropertyValidator textChoiceValidator) { - NamedObjectList options = new NamedObjectList(); - List choices = PropertyService.get().getTextChoiceValidatorOptions(textChoiceValidator); + LinkedHashSet choices = new LinkedHashSet<>(PropertyService.get().getTextChoiceValidatorOptions(textChoiceValidator)); // if the already saved strVal is not in the current choice set, add it (as it seems wrong to remove a value that the user hasn't explicitly touched) - if (!StringUtils.isEmpty(strVal) && !choices.contains(strVal)) - { - choices = new ArrayList<>(choices); - choices.add(strVal); - } + choices.addAll(strValues); + NamedObjectList options = new NamedObjectList(); for (String choice : choices) options.put(new SimpleNamedObject(choice, choice)); - renderSelectFormInput(out, formFieldName, value, strVal, disabledInput, options); + renderSelectFormInput(out, formFieldName, value, strValues, disabledInput, options); } protected void renderSelectFormInputFromFk(RenderContext ctx, HtmlWriter out, String formFieldName, Object value, String strVal, boolean disabledInput) @@ -807,7 +809,7 @@ protected void renderSelectFormInputFromFk(RenderContext ctx, HtmlWriter out, St } else { - renderSelectFormInput(out, formFieldName, value, strVal, disabledInput, entryList); + renderSelectFormInput(out, formFieldName, value, List.of(Objects.toString(value)), disabledInput, entryList); } } diff --git a/api/src/org/labkey/api/data/DisplayColumn.java b/api/src/org/labkey/api/data/DisplayColumn.java index 2d673af6256..d531f2bba51 100644 --- a/api/src/org/labkey/api/data/DisplayColumn.java +++ b/api/src/org/labkey/api/data/DisplayColumn.java @@ -58,6 +58,7 @@ import java.text.DecimalFormatSymbols; import java.text.Format; import java.util.ArrayList; +import java.util.Collection; import java.util.Date; import java.util.HashSet; import java.util.LinkedHashSet; @@ -631,7 +632,11 @@ public String getJsonTypeName() public static String getJsonTypeName(Class valueClass) { - if (String.class.isAssignableFrom(valueClass)) + if (Map.class.isAssignableFrom(valueClass)) + return "object"; + else if (valueClass.isArray() || Collection.class.isAssignableFrom(valueClass)) + return "array"; + else if (String.class.isAssignableFrom(valueClass)) return "string"; else if (Boolean.class.isAssignableFrom(valueClass) || boolean.class.isAssignableFrom(valueClass)) return "boolean"; @@ -1166,7 +1171,7 @@ protected Object getInputValue(RenderContext ctx) val = viewForm.getAsString(formFieldName); } else if (ctx.getRow() != null) - val = col.getValue(ctx); + val = getValue(ctx); } return val; diff --git a/api/src/org/labkey/api/data/JdbcType.java b/api/src/org/labkey/api/data/JdbcType.java index 361456f21c1..e427d25382a 100644 --- a/api/src/org/labkey/api/data/JdbcType.java +++ b/api/src/org/labkey/api/data/JdbcType.java @@ -30,6 +30,7 @@ import java.math.BigDecimal; import java.math.BigInteger; import java.nio.ByteBuffer; +import java.sql.Array; import java.sql.Time; import java.sql.Timestamp; import java.sql.Types; @@ -299,6 +300,8 @@ protected Collection getSqlTypes() } }, + ARRAY(Types.ARRAY, Array.class), + NULL(Types.NULL, Object.class), OTHER(Types.OTHER, Object.class); @@ -398,7 +401,6 @@ protected void addSqlTypes(Collection sqlTypes) public static JdbcType valueOf(int type) { JdbcType jt = sqlTypeMap.get(type); - return null != jt ? jt : OTHER; } diff --git a/api/src/org/labkey/api/data/MultiChoice.java b/api/src/org/labkey/api/data/MultiChoice.java new file mode 100644 index 00000000000..9ab3b921712 --- /dev/null +++ b/api/src/org/labkey/api/data/MultiChoice.java @@ -0,0 +1,510 @@ +package org.labkey.api.data; + +import org.apache.commons.beanutils.ConvertUtils; +import org.apache.commons.lang3.StringUtils; +import org.jetbrains.annotations.NotNull; +import org.labkey.api.collections.CaseInsensitiveHashSet; +import org.labkey.api.exp.property.IPropertyValidator; +import org.labkey.api.exp.property.PropertyService; +import org.labkey.api.gwt.client.model.PropertyValidatorType; +import org.labkey.api.ontology.Unit; +import org.labkey.api.util.DOM; +import org.labkey.api.util.HtmlString; +import org.labkey.api.util.PageFlowUtil; +import org.labkey.api.writer.HtmlWriter; + +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Types; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.ListIterator; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.function.BiConsumer; +import java.util.function.BinaryOperator; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Collector; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; + +import static org.labkey.api.util.DOM.Attribute.style; +import static org.labkey.api.util.DOM.DIV; +import static org.labkey.api.util.DOM.SPAN; +import static org.labkey.api.util.DOM.at; + +public class MultiChoice +{ + public static final String ARRAY_MARKER = "[]"; + public static class DisplayColumn extends DataColumn + { + public DisplayColumn(ColumnInfo col) + { + super(col, false); + } + + @Override + public Object getValue(RenderContext ctx) + { + Object v = super.getValue(ctx); + if (!(v instanceof java.sql.Array array)) + return Array.from(new Object[]{v}); + return Array.from(array); + } + + @Override + public Object getDisplayValue(RenderContext ctx) + { + return getValue(ctx); + } + + @Override + protected Object getInputValue(RenderContext ctx) + { + return super.getInputValue(ctx); + } + + @Override + protected String getStringValue(Object value, Unit unit, boolean disabledInput) + { + // Because MultiChoice.Array implements Collection ConvertUtils will return convert(Array.get(0).toString())) + if (value instanceof Array) + return value.toString(); + return super.getStringValue(value, unit, disabledInput); + } + + @Override + public void renderInputHtml(RenderContext ctx, HtmlWriter out, Object value) + { + MultiChoice.Array array; + if (null == value) + array = new MultiChoice.Array(new String[]{}); + else if (value instanceof MultiChoice.Array mca) + array = mca; + else + array = _converter.convert(MultiChoice.Array.class, value); + + boolean disabledInput = isDisabledInput(ctx); + String formFieldName = getFormFieldName(ctx); + ColumnInfo boundColumn = getBoundColumn(); + IPropertyValidator textChoiceValidator = PropertyService.get().getValidatorForColumn(boundColumn, PropertyValidatorType.TextChoice); + + if (textChoiceValidator != null) + { + setInputType("select.multiple"); + renderTextChoiceFormInput(out, formFieldName+ARRAY_MARKER, array, array, disabledInput, textChoiceValidator); + } + else + { + renderTextFormInput(out, formFieldName, array, array.toString(), disabledInput); + } + } + + @Override + public void renderInputCell(RenderContext ctx, HtmlWriter out) + { + super.renderInputCell(ctx, out); + } + + @Override + public @NotNull HtmlString getFormattedHtml(RenderContext ctx) + { + Array array = (Array) getValue(ctx); + + if (null == array || array.isEmpty()) + return HtmlString.EMPTY_STRING; + + return HtmlString.of( + DIV(array.stream().map(v -> SPAN(at(style,"border:solid 1px black; border-radius:3px;"), v)) + .collect(new JoinRenderable(HtmlString.SP)))); + } + } + + + /* could also use .flatMap() */ + static class JoinRenderable implements Collector,List> + { + private final DOM.Renderable separator; + + JoinRenderable(DOM.Renderable separator) + { + this.separator = separator; + } + + @Override + public BiConsumer, DOM.Renderable> accumulator() + { + return (l, r) -> { + if (null == r) + return; + if (!l.isEmpty()) + l.add(separator); + l.add(r);}; + } + + @Override + public Supplier> supplier() + { + return ArrayList::new; + } + + @Override + public BinaryOperator> combiner() + { + return (l1,l2) -> {l1.addAll(l2); return l1;}; + } + + @Override + public Function, List> finisher() + { + return l->l; + } + + @Override + public Set characteristics() + { + return Set.of(); + } + } + + + // LK impl to help with conversions + public static class Array implements List, java.sql.Array + { + final String[] array; + List list = null; + + protected Array(Stream str) + { + CaseInsensitiveHashSet set = new CaseInsensitiveHashSet(); + array = str.filter(Objects::nonNull) + .map(s -> StringUtils.trimToNull(s.toString())) + .filter(Objects::nonNull) + .toArray(String[]::new); + } + + protected Array(Object[] array) + { + this(Stream.of(array)); + } + + public String[] getStringArray() + { + return array; + } + + private List getList() + { + if (null == list) + list = Collections.unmodifiableList(Arrays.asList(array)); + return list; + } + + @Override + public String toString() + { + return PageFlowUtil.joinValuesToStringForExport(this); + } + + public static Array from(Object @NotNull [] values) + { + return new Array(Arrays.stream(values)); + } + + public static Array from(@NotNull org.json.JSONArray array) + { + return new Array(StreamSupport.stream(array.spliterator(), false)); + } + + public static Array from(@NotNull String s) + { + List split = PageFlowUtil.splitStringToValuesForImport(s); + return from(split.toArray()); + } + + public static Array from(@NotNull java.sql.Array sqlArray) + { + try + { + // JDBC spec says java.sql.Array can return a primitive array! + Object o = sqlArray.getArray(); + Object[] array; + if (!o.getClass().getComponentType().isPrimitive()) + { + array = (Object[]) o; + } + else + { + array = new Object[java.lang.reflect.Array.getLength(o)]; + for (int i = 0; i < array.length; i++) + array[i] = java.lang.reflect.Array.get(o, i); + } + return new Array(array); + } + catch (SQLException x) + { + throw new RuntimeException(x); + } + } + + // + // implements List + // + + @Override + public boolean add(String s) + { + throw new UnsupportedOperationException(); + } + + @Override + public int size() + { + return array.length; + } + + @Override + public boolean isEmpty() + { + return array.length == 0; + } + + @Override + public boolean contains(Object o) + { + return getList().contains(o); + } + + @Override + public @NotNull Iterator iterator() + { + return getList().iterator(); + } + + @Override + public @NotNull Object[] toArray() + { + return array; + } + + @Override + public @NotNull T[] toArray(@NotNull T[] a) + { + if (a.length == 0 && a.getClass().getComponentType().isAssignableFrom(String.class)) + return (T[])array.clone(); + return getList().toArray(a); + } + + @Override + public boolean remove(Object o) + { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsAll(@NotNull Collection c) + { + return getList().containsAll(c); + } + + @Override + public boolean addAll(@NotNull Collection c) + { + throw new UnsupportedOperationException(); + } + + @Override + public boolean addAll(int index, @NotNull Collection c) + { + throw new UnsupportedOperationException(); + } + + @Override + public boolean removeAll(@NotNull Collection c) + { + throw new UnsupportedOperationException(); + } + + @Override + public boolean retainAll(@NotNull Collection c) + { + throw new UnsupportedOperationException(); + } + + @Override + public void clear() + { + throw new UnsupportedOperationException(); + } + + @Override + public String get(int index) + { + return array[index]; + } + + @Override + public String set(int index, String element) + { + throw new UnsupportedOperationException(); + } + + @Override + public void add(int index, String element) + { + throw new UnsupportedOperationException(); + } + + @Override + public String remove(int index) + { + throw new UnsupportedOperationException(); + } + + @Override + public int indexOf(Object o) + { + return getList().indexOf(o); + } + + @Override + public int lastIndexOf(Object o) + { + return getList().lastIndexOf(o); + } + + @Override + public @NotNull ListIterator listIterator() + { + return getList().listIterator(); + } + + @Override + public @NotNull ListIterator listIterator(int index) + { + return getList().listIterator(index); + } + + @Override + public @NotNull List subList(int fromIndex, int toIndex) + { + return getList().subList(fromIndex, toIndex); + } + + + // + // implements Array + // + + @Override + public void free() throws SQLException + { + + } + + @Override + public String getBaseTypeName() throws SQLException + { + return "VARCHAR"; + } + + @Override + public int getBaseType() throws SQLException + { + return Types.VARCHAR; + } + + @Override + public Object getArray() throws SQLException + { + return toArray(new String[size()]); + } + + @Override + public Object getArray(Map> map) throws SQLException + { + return toArray(new String[size()]); + } + + @Override + public Object getArray(long index, int count) throws SQLException + { + return subList((int) index, (int) index + count).toArray(new String[0]); + } + + @Override + public Object getArray(long index, int count, Map> map) throws SQLException + { + return subList((int) index, (int) index + count).toArray(new String[0]); + } + + @Override + public ResultSet getResultSet() throws SQLException + { + throw new UnsupportedOperationException(); + } + + @Override + public ResultSet getResultSet(Map> map) throws SQLException + { + throw new UnsupportedOperationException(); + } + + @Override + public ResultSet getResultSet(long index, int count) throws SQLException + { + throw new UnsupportedOperationException(); + } + + @Override + public ResultSet getResultSet(long index, int count, Map> map) throws SQLException + { + throw new UnsupportedOperationException(); + } + } + + private static final Converter _converter = new Converter(); + static + { + ConvertUtils.register(_converter, Array.class); + } + + + public static class Converter implements org.apache.commons.beanutils.Converter, Function + { + private Converter() + { + } + + public static Converter getInstance() + { + return _converter; + } + + @Override + public T convert(Class aClass, Object o) + { + if (null == o) + return (T) List.of(); + if (o instanceof String s) + return (T) Array.from(s); + if (o instanceof org.json.JSONArray json) + return (T) Array.from(json); + if (o.getClass().isArray()) + return (T) Array.from((Object[]) o); + return (T) Array.from(o.toString()); + } + + @Override + final public Object apply(Object o) + { + return convert(MultiChoice.Array.class, o); + } + } +} \ No newline at end of file diff --git a/api/src/org/labkey/api/data/SQLFragment.java b/api/src/org/labkey/api/data/SQLFragment.java index f0fe937f876..acde96b77c6 100644 --- a/api/src/org/labkey/api/data/SQLFragment.java +++ b/api/src/org/labkey/api/data/SQLFragment.java @@ -684,6 +684,11 @@ public SQLFragment add(Object p) return this; } + public SQLFragment add(Object p, JdbcType type) + { + getMutableParams().add(new Parameter.TypedValue(p, type)); + return this; + } /** Adds the objects as JDBC parameter values */ public SQLFragment addAll(Collection l) diff --git a/api/src/org/labkey/api/data/Table.java b/api/src/org/labkey/api/data/Table.java index ab70c00cfdb..7c30344cc96 100644 --- a/api/src/org/labkey/api/data/Table.java +++ b/api/src/org/labkey/api/data/Table.java @@ -1015,7 +1015,7 @@ else if (pkVals instanceof Map) whereSQL.append(whereAND); whereSQL.appendIdentifier(col.getSelectIdentifier()); whereSQL.append("=?"); - whereSQL.add(keys.get(col.getName())); + whereSQL.add(keys.get(col.getName()), col.getJdbcType()); whereAND = " AND "; } diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index 3f82dc0cc88..d693ddc244d 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -65,6 +65,9 @@ import java.util.Set; import java.util.stream.Collectors; +import static org.labkey.api.action.SpringActionController.FIELD_MARKER; +import static org.labkey.api.data.MultiChoice.ARRAY_MARKER; + /** * Basic form for handling posts into views. * Supports insert, update, delete functionality with a minimum of fuss @@ -644,7 +647,6 @@ public void setTypedValues(Map values, boolean merge) if (Character.isUpperCase(propName.charAt(0))) propName = Introspector.decapitalize(propName); setTypedValue(propName, e.getValue()); - // TODO MultiChoice To convert or not to convert??? _stringValues.put(propName, e.getValue()); } } @@ -813,7 +815,7 @@ public void setViewContext(@NotNull ViewContext context) public static PropertyValues preprocessPropertyValues(PropertyValues params) { // we can usually just return params - if (params.stream().noneMatch(e -> e.getName().endsWith("[]") || e.getName().startsWith(SpringActionController.FIELD_MARKER))) + if (params.stream().noneMatch(e -> e.getName().endsWith(ARRAY_MARKER) || e.getName().startsWith(FIELD_MARKER))) return params; Set names = params.stream().map(PropertyValue::getName).collect(Collectors.toSet()); @@ -821,13 +823,13 @@ public static PropertyValues preprocessPropertyValues(PropertyValues params) for (var orig : params) { var copy = orig; - if (orig.getName().startsWith(SpringActionController.FIELD_MARKER)) + if (orig.getName().startsWith(FIELD_MARKER)) { if (names.contains(orig.getName().substring(1))) continue; copy = new PropertyValue(orig.getName().substring(1), "0"); } - else if (orig.getName().endsWith("[]") && orig.getValue()!=null) + else if (orig.getName().endsWith(ARRAY_MARKER) && orig.getValue()!=null) { var value = orig.getValue(); var convertedValue = value; diff --git a/api/src/org/labkey/api/exp/PropertyType.java b/api/src/org/labkey/api/exp/PropertyType.java index 9fa928cfac4..401da603679 100644 --- a/api/src/org/labkey/api/exp/PropertyType.java +++ b/api/src/org/labkey/api/exp/PropertyType.java @@ -26,6 +26,7 @@ import org.labkey.api.attachments.AttachmentFile; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.JdbcType; +import org.labkey.api.data.MultiChoice; import org.labkey.api.data.NameGenerator; import org.labkey.api.exp.OntologyManager.PropertyRow; import org.labkey.api.reader.ExcelFactory; @@ -41,6 +42,7 @@ import java.text.SimpleDateFormat; import java.util.Date; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.TimeZone; @@ -155,6 +157,7 @@ public Object getPreviewValue(@Nullable String prefix) return prefix + "Value"; } }, + // NOT an XMLSchema type uri??? MULTI_LINE("http://www.w3.org/2001/XMLSchema#multiLine", "MultiLine", 's', JdbcType.VARCHAR, 4000, "textarea", CellType.STRING, String.class) { @Override @@ -202,6 +205,53 @@ public Object getPreviewValue(@Nullable String prefix) return prefix + "Value"; } }, + MULTI_CHOICE("http://cpas.fhcrc.org/exp/xml#multiChoice", "MultiChoice", '?' /* unsupported in exp.PropertyValues */, JdbcType.ARRAY, 0, "textarea", CellType.STRING, List.class) + { + @Override + protected Object convertExcelValue(Cell cell) throws ConversionException + { + return ConvertUtils.convert(cell.getStringCellValue(), MultiChoice.Array.class); + } + + @Override + public Object convert(Object value) throws ConversionException + { + if (value instanceof String) + return value; + else + return ConvertUtils.convert(value); + } + + @Override + public SimpleTypeNames.Enum getXmlBeanType() + { + return SimpleTypeNames.STRING; + } + + @Override + protected void init(PropertyRow row, Object value) + { + row.stringValue = (String)value; + } + + @Override + protected void setValue(ObjectProperty property, Object value) + { + property.stringValue = value == null ? null : value.toString(); + } + + @Override + protected Object getValue(ObjectProperty property) + { + return property.getStringValue(); + } + + @Override + public Object getPreviewValue(@Nullable String prefix) + { + return prefix + "Value"; + } + }, RESOURCE("http://www.w3.org/2000/01/rdf-schema#Resource", "PropertyURI", 's', JdbcType.VARCHAR, 4000, null, CellType.STRING, Identifiable.class) { @Override @@ -372,6 +422,7 @@ public Object getPreviewValue(@Nullable String prefix) return Integer.valueOf(3); } }, + // NOT an XMLSchema type uri??? BINARY("http://www.w3.org/2001/XMLSchema#binary", "Binary", 'f', JdbcType.BINARY, 10, null, CellType.NUMERIC, ByteBuffer.class) { @Override diff --git a/api/src/org/labkey/api/exp/property/DomainKind.java b/api/src/org/labkey/api/exp/property/DomainKind.java index 508ef06f025..f2a8ff5aad9 100644 --- a/api/src/org/labkey/api/exp/property/DomainKind.java +++ b/api/src/org/labkey/api/exp/property/DomainKind.java @@ -323,6 +323,7 @@ public boolean matchesTemplateXML(String templateName, DomainTemplateType templa public boolean allowAttachmentProperties() { return false; } public boolean allowFlagProperties() { return true; } public boolean allowTextChoiceProperties() { return true; } + public boolean allowMultiTextChoiceProperties() { return false; } public boolean allowSampleSubjectProperties() { return true; } public boolean allowTimepointProperties() { return false; } public boolean allowUniqueConstraintProperties() { return false; } diff --git a/api/src/org/labkey/api/util/HtmlString.java b/api/src/org/labkey/api/util/HtmlString.java index 54943a4d771..2075039ac44 100644 --- a/api/src/org/labkey/api/util/HtmlString.java +++ b/api/src/org/labkey/api/util/HtmlString.java @@ -30,6 +30,7 @@ public final class HtmlString implements SafeToRender, DOM.Renderable, Comparabl { // Helpful constants for convenience (and efficiency) public static final HtmlString EMPTY_STRING = HtmlString.of(""); + public static final HtmlString SP = HtmlString.unsafe(" "); public static final HtmlString NBSP = HtmlString.unsafe(" "); public static final HtmlString NDASH = HtmlString.unsafe("–"); public static final HtmlString BR = HtmlString.unsafe("
"); diff --git a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java index 545600dcee5..9559df5f69d 100644 --- a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java @@ -323,7 +323,10 @@ public void registerValidatorKind(ValidatorKind validatorKind) if (_validatorTypes.containsKey(validatorKind.getTypeURI())) throw new IllegalArgumentException("Validator type : " + validatorKind.getTypeURI() + " is already registered"); - _validatorTypes.put(validatorKind.getTypeURI(), validatorKind); + var uri = validatorKind.getTypeURI(); + _validatorTypes.put(uri, validatorKind); + _validatorTypes.put(uri.substring(uri.lastIndexOf(':')+1), validatorKind); + _validatorTypes.put(validatorKind.getName(), validatorKind); } @Override diff --git a/experiment/src/org/labkey/experiment/api/property/TextChoiceValidator.java b/experiment/src/org/labkey/experiment/api/property/TextChoiceValidator.java index 603982a5a3d..da946740ab8 100644 --- a/experiment/src/org/labkey/experiment/api/property/TextChoiceValidator.java +++ b/experiment/src/org/labkey/experiment/api/property/TextChoiceValidator.java @@ -17,6 +17,7 @@ import org.jetbrains.annotations.NotNull; import org.labkey.api.data.ColumnRenderProperties; +import org.labkey.api.data.MultiChoice; import org.labkey.api.exp.property.IPropertyValidator; import org.labkey.api.exp.property.PropertyService; import org.labkey.api.exp.property.ValidatorContext; @@ -24,7 +25,11 @@ import org.labkey.api.gwt.client.model.PropertyValidatorType; import org.labkey.api.query.ValidationError; +import java.util.Collection; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Objects; +import java.util.Set; public class TextChoiceValidator extends RegExValidator implements ValidatorKind { @@ -52,18 +57,39 @@ public boolean validate(IPropertyValidator validator, ColumnRenderProperties fie { assert value != null : "Shouldn't be validating a null value"; - List validValues = (List)validatorCache.get(TextChoiceValidator.class, validator.getExpressionValue()); + Set validValues = (Set)validatorCache.get(TextChoiceValidator.class, validator.getExpressionValue()); if (validValues == null) { - validValues = PropertyService.get().getTextChoiceValidatorOptions(validator); + validValues = new LinkedHashSet<>(PropertyService.get().getTextChoiceValidatorOptions(validator)); // Cache the validValues so that it can be reused validatorCache.put(TextChoiceValidator.class, validator.getExpressionValue(), validValues); } - if (validValues.contains(value)) - return true; + Object errorValue = null; - createErrorMessage(validator, field, value, errors); - return false; + if (value instanceof String) + { + if (!validValues.contains(value)) + errorValue = value; + } + else if (value instanceof Collection col) + { + for (Object item : col) + { + if (null == item || !validValues.contains(Objects.toString(item))) + { + errorValue = item; + break; + } + } + } + else + { + errorValue = value; + } + + if (null != errorValue) + createErrorMessage(validator, field, value, errors); + return null == errorValue; } } \ No newline at end of file From 14256bcd454efc58df2fe47ded1b3bd31d4ba51d Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Mon, 3 Nov 2025 13:11:12 -0800 Subject: [PATCH 15/19] Update api/src/org/labkey/api/data/ColumnRenderProperties.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- api/src/org/labkey/api/data/ColumnRenderProperties.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/data/ColumnRenderProperties.java b/api/src/org/labkey/api/data/ColumnRenderProperties.java index 75070c71a0c..a282ddfa5bf 100644 --- a/api/src/org/labkey/api/data/ColumnRenderProperties.java +++ b/api/src/org/labkey/api/data/ColumnRenderProperties.java @@ -395,7 +395,7 @@ static Function getDefaultConvertFn(ColumnRenderProperties col) if (value == null || javaClass == value.getClass()) return value; if (value instanceof CharSequence) - ConvertUtils.convert(value.toString(), javaClass); + return ConvertUtils.convert(value.toString(), javaClass); return jdbcType.convert(value); }; } From af54995f1e575f000a3190e99e77e479e5e85c51 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Mon, 3 Nov 2025 15:35:03 -0800 Subject: [PATCH 16/19] undo --- .../labkey/experiment/api/property/PropertyServiceImpl.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java index 9559df5f69d..545600dcee5 100644 --- a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java @@ -323,10 +323,7 @@ public void registerValidatorKind(ValidatorKind validatorKind) if (_validatorTypes.containsKey(validatorKind.getTypeURI())) throw new IllegalArgumentException("Validator type : " + validatorKind.getTypeURI() + " is already registered"); - var uri = validatorKind.getTypeURI(); - _validatorTypes.put(uri, validatorKind); - _validatorTypes.put(uri.substring(uri.lastIndexOf(':')+1), validatorKind); - _validatorTypes.put(validatorKind.getName(), validatorKind); + _validatorTypes.put(validatorKind.getTypeURI(), validatorKind); } @Override From 68405076b181ffeb6eb9288aa5b60a178a35f452 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Mon, 3 Nov 2025 15:52:26 -0800 Subject: [PATCH 17/19] List is probably better here. Collection is pretty broad. --- api/src/org/labkey/api/data/DisplayColumn.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/data/DisplayColumn.java b/api/src/org/labkey/api/data/DisplayColumn.java index d531f2bba51..a9918cf3cd5 100644 --- a/api/src/org/labkey/api/data/DisplayColumn.java +++ b/api/src/org/labkey/api/data/DisplayColumn.java @@ -634,7 +634,7 @@ public static String getJsonTypeName(Class valueClass) { if (Map.class.isAssignableFrom(valueClass)) return "object"; - else if (valueClass.isArray() || Collection.class.isAssignableFrom(valueClass)) + else if (valueClass.isArray() || List.class.isAssignableFrom(valueClass)) return "array"; else if (String.class.isAssignableFrom(valueClass)) return "string"; From 82652d5143f6831dcde97b8b4f1a6b88531f60ed Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Mon, 3 Nov 2025 16:52:09 -0800 Subject: [PATCH 18/19] CR fixes --- api/src/org/labkey/api/data/MultiChoice.java | 2 +- api/src/org/labkey/api/exp/PropertyType.java | 13 +++++-------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/api/src/org/labkey/api/data/MultiChoice.java b/api/src/org/labkey/api/data/MultiChoice.java index 9ab3b921712..ad0862ebf1a 100644 --- a/api/src/org/labkey/api/data/MultiChoice.java +++ b/api/src/org/labkey/api/data/MultiChoice.java @@ -39,7 +39,7 @@ import static org.labkey.api.util.DOM.SPAN; import static org.labkey.api.util.DOM.at; -public class MultiChoice +public class MultiChoice { public static final String ARRAY_MARKER = "[]"; public static class DisplayColumn extends DataColumn diff --git a/api/src/org/labkey/api/exp/PropertyType.java b/api/src/org/labkey/api/exp/PropertyType.java index 401da603679..a30ac08695f 100644 --- a/api/src/org/labkey/api/exp/PropertyType.java +++ b/api/src/org/labkey/api/exp/PropertyType.java @@ -216,10 +216,7 @@ protected Object convertExcelValue(Cell cell) throws ConversionException @Override public Object convert(Object value) throws ConversionException { - if (value instanceof String) - return value; - else - return ConvertUtils.convert(value); + return MultiChoice.Converter.getInstance().convert(MultiChoice.Array.class, value); } @Override @@ -231,25 +228,25 @@ public SimpleTypeNames.Enum getXmlBeanType() @Override protected void init(PropertyRow row, Object value) { - row.stringValue = (String)value; + throw new UnsupportedOperationException("TODO MultiChoice"); } @Override protected void setValue(ObjectProperty property, Object value) { - property.stringValue = value == null ? null : value.toString(); + throw new UnsupportedOperationException("TODO MultiChoice"); } @Override protected Object getValue(ObjectProperty property) { - return property.getStringValue(); + throw new UnsupportedOperationException("TODO MultiChoice"); } @Override public Object getPreviewValue(@Nullable String prefix) { - return prefix + "Value"; + return "Option 1, Option 2"; } }, RESOURCE("http://www.w3.org/2000/01/rdf-schema#Resource", "PropertyURI", 's', JdbcType.VARCHAR, 4000, null, CellType.STRING, Identifiable.class) From caeed0467542b9f6d856e8f9000bdad8c17900ce Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Tue, 4 Nov 2025 12:43:39 -0800 Subject: [PATCH 19/19] add List support to Converter.convert() basic Converter junit test --- api/src/org/labkey/api/data/MultiChoice.java | 82 +++++++++++++++++++- core/src/org/labkey/core/CoreModule.java | 2 + 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/data/MultiChoice.java b/api/src/org/labkey/api/data/MultiChoice.java index ad0862ebf1a..a694c3f53bd 100644 --- a/api/src/org/labkey/api/data/MultiChoice.java +++ b/api/src/org/labkey/api/data/MultiChoice.java @@ -3,16 +3,21 @@ import org.apache.commons.beanutils.ConvertUtils; import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.NotNull; +import org.json.JSONArray; +import org.junit.Assert; +import org.junit.Test; import org.labkey.api.collections.CaseInsensitiveHashSet; import org.labkey.api.exp.property.IPropertyValidator; import org.labkey.api.exp.property.PropertyService; import org.labkey.api.gwt.client.model.PropertyValidatorType; import org.labkey.api.ontology.Unit; +import org.labkey.api.reader.TabLoader; import org.labkey.api.util.DOM; import org.labkey.api.util.HtmlString; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.writer.HtmlWriter; +import java.io.StringBufferInputStream; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Types; @@ -186,6 +191,7 @@ protected Array(Stream str) array = str.filter(Objects::nonNull) .map(s -> StringUtils.trimToNull(s.toString())) .filter(Objects::nonNull) + .filter(set::add) .toArray(String[]::new); } @@ -206,6 +212,20 @@ private List getList() return list; } + @Override + public boolean equals(Object obj) + { + if (!(obj instanceof Array arr)) + return false; + return Arrays.equals(array, arr.array); + } + + @Override + public int hashCode() + { + return Arrays.hashCode(array); + } + @Override public String toString() { @@ -491,13 +511,17 @@ public static Converter getInstance() public T convert(Class aClass, Object o) { if (null == o) - return (T) List.of(); + return (T) Array.from(new String[]{}); + if (o instanceof MultiChoice.Array arr) + return (T)arr; if (o instanceof String s) return (T) Array.from(s); - if (o instanceof org.json.JSONArray json) - return (T) Array.from(json); if (o.getClass().isArray()) return (T) Array.from((Object[]) o); + if (o instanceof org.json.JSONArray json) + return (T) Array.from(json); + if (o instanceof List list) + return (T) new Array(list.stream()); return (T) Array.from(o.toString()); } @@ -507,4 +531,56 @@ final public Object apply(Object o) return convert(MultiChoice.Array.class, o); } } + + + + public static class TestCase extends Assert + { + @Test + public void testConvert() throws Exception + { + Array expected = Array.from(new String[]{"a,","b\"","c "}); + + assertEquals(expected, _converter.convert(Array.class, expected)); + assertEquals(expected, _converter.convert(Array.class, "\"a,\",\"b\"\"\",\"c \"")); + assertEquals(expected, _converter.convert(Array.class, new String[]{"a,","b\"","c "})); + assertEquals(expected, _converter.convert(Array.class, List.of("a,","b\"","c "))); + assertEquals(expected, _converter.convert(Array.class, new JSONArray(List.of("a,","b\"","c ")))); + } + + @Test + public void testCSV() throws Exception + { + Array expected = Array.from(new String[]{"a,", "b\\", "c,d", "e\"f"}); + // toString() == "a,", b\, "c,d", "e""f" + assertEquals("\"a,\", b\\, \"c,d\", \"e\"\"f\"", expected.toString()); + + // csv/tsv with double double-quote escaping + // add " around entire value, and double the " + // (need to use some \" to avoid ending the """ """ block) + String oneMultiValueColumn = """ + column + ""\"a,"", b\\, ""c,d"", ""e""\""f""\" + """; + + try (var csvLoader = new TabLoader.CsvFactory().createLoader(new StringBufferInputStream(oneMultiValueColumn), true)) + { + var maps = csvLoader.load(); + assertEquals(1, maps.size()); + Map map = maps.get(0); + assertTrue(map.get("column") instanceof String); + String value = (String) map.get("column"); + assertEquals(expected, Array.from(value)); + } + try (var tsvLoader = new TabLoader.TsvFactory().createLoader(new StringBufferInputStream(oneMultiValueColumn), true)) + { + var maps = tsvLoader.load(); + assertEquals(1, maps.size()); + Map map = maps.get(0); + assertTrue(map.get("column") instanceof String); + String value = (String) map.get("column"); + assertEquals(expected, Array.from(value)); + } + } + } } \ No newline at end of file diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index e35275d1395..6b1629758a4 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -60,6 +60,7 @@ import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbScope; import org.labkey.api.data.FileSqlScriptProvider; +import org.labkey.api.data.MultiChoice; import org.labkey.api.data.MvUtil; import org.labkey.api.data.NormalContainerType; import org.labkey.api.data.OutOfRangeDisplayColumn; @@ -1454,6 +1455,7 @@ public TabDisplayMode getTabDisplayMode() ApiJsonWriter.TestCase.class, ClassLoaderTestCase.class, CopyFileRootPipelineJob.TestCase.class, + MultiChoice.TestCase.class, OutOfRangeDisplayColumn.TestCase.class, PostgreSqlVersion.TestCase.class, ScriptEngineManagerImpl.TestCase.class,