Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

XWIKI-15776: The groups displayer store values with a coma at the end #1800

Merged
merged 4 commits into from
Mar 18, 2022
Merged

XWIKI-15776: The groups displayer store values with a coma at the end #1800

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5f8e7e8
XWIKI-15576: The groups displayer store values with a coma at the end
surli Mar 18, 2022
e83ede1
XWIKI-15576: The groups displayer store values with a coma at the end
surli Mar 18, 2022
c094260
XWIKI-15576: The groups displayer store values with a coma at the end
surli Mar 18, 2022
2631642
XWIKI-15776: The groups displayer store values with a coma at the end
surli Mar 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions ...-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/classes/GroupsClass.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import java.util.List;
import java.util.Map;

import org.apache.commons.lang3.StringUtils;
import org.dom4j.Element;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -52,6 +51,8 @@ public class GroupsClass extends ListClass
/** Logging helper object. */
private static final Logger LOGGER = LoggerFactory.getLogger(GroupsClass.class);

private static final String COMMA = ",";

/**
* The meta property that specifies if the list box that is used to select the groups should be filled with all the
* available groups. This property should not be set when the number of groups is very large.
Expand Down Expand Up @@ -80,6 +81,12 @@ public GroupsClass()
this(null);
}

@Override
protected String getFirstSeparator()
{
return COMMA;
}

@Override
public List<String> getList(XWikiContext context)
{
Expand Down Expand Up @@ -145,7 +152,7 @@ public BaseProperty fromStringArray(String[] strings)
List<String> list = Arrays.asList(strings);

BaseProperty prop = newProperty();
fromList(prop, list);
fromList(prop, list, true);
return prop;
}

Expand All @@ -170,7 +177,7 @@ public String getText(String value, XWikiContext context)
*/
public static List<String> getListFromString(String value)
{
return getListFromString(value, ",", false);
return getListFromString(value, COMMA, false, true);
}

@Override
Expand All @@ -194,16 +201,6 @@ public List<String> toList(BaseProperty<?> property)
return selectlist;
}

@Override
public void fromList(BaseProperty<?> property, List<String> list)
{
if (isMultiSelect()) {
property.setValue(list != null ? StringUtils.join(list, ',') : null);
} else {
property.setValue(list != null && !list.isEmpty() ? list.get(0) : null);
}
}

@Override
public <T extends EntityReference> void mergeProperty(BaseProperty<T> currentProperty,
BaseProperty<T> previousProperty, BaseProperty<T> newProperty, MergeConfiguration configuration,
Expand Down
23 changes: 10 additions & 13 deletions ...-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/classes/LevelsClass.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public class LevelsClass extends ListClass

private static final String XCLASSNAME = "levelslist";

private static final String COMMA = ",";

public LevelsClass(PropertyMetaClass wclass)
{
super(XCLASSNAME, "Levels List", wclass);
Expand All @@ -61,6 +63,12 @@ public LevelsClass()
this(null);
}

@Override
protected String getFirstSeparator()
{
return COMMA;
}

@Override
public List<String> getList(XWikiContext context)
{
Expand Down Expand Up @@ -109,13 +117,8 @@ public BaseProperty fromString(String value)
public BaseProperty fromStringArray(String[] strings)
{
List<String> list = new ArrayList<String>();
for (int i = 0; i < strings.length; i++) {
if (!strings[i].trim().equals("")) {
list.add(strings[i]);
}
}
BaseProperty prop = newProperty();
fromList(prop, list);
fromList(prop, list, true);
return prop;
}

Expand All @@ -126,7 +129,7 @@ public String getText(String value, XWikiContext context)

public static List<String> getListFromString(String value)
{
return getListFromString(value, ",", false);
return getListFromString(value, COMMA, false, true);
}

@Override
Expand Down Expand Up @@ -194,12 +197,6 @@ public List<String> toList(BaseProperty<?> property)
return selectlist;
}

@Override
public void fromList(BaseProperty<?> property, List<String> list)
{
property.setValue(list != null ? StringUtils.join(list, ',') : null);
}

@Override
public <T extends EntityReference> void mergeProperty(BaseProperty<T> currentProperty,
BaseProperty<T> previousProperty, BaseProperty<T> newProperty, MergeConfiguration configuration,
Expand Down
77 changes: 71 additions & 6 deletions ...rm-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/classes/ListClass.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import org.apache.commons.lang3.StringUtils;
import org.apache.ecs.xhtml.input;
Expand Down Expand Up @@ -314,6 +315,21 @@ public static List<String> getListFromString(String value)
return getListFromString(value, null, true);
}

/**
* @return the first separator of the list of separators, or fallback on {@link #DEFAULT_SEPARATOR}.
* @since 14.2RC1
*/
protected String getFirstSeparator()
{
String separator;
if (!StringUtils.isEmpty(getSeparators())) {
separator = String.valueOf(getSeparators().charAt(0));
} else {
separator = DEFAULT_SEPARATOR;
}
return separator;
}

/**
* @param value the string holding a serialized list
* @param separators the separator characters (given as a string) used to delimit the list's items inside the input
Expand All @@ -324,6 +340,22 @@ public static List<String> getListFromString(String value)
* @return the list that was stored in the input string
*/
public static List<String> getListFromString(String value, String separators, boolean withMap)
{
return ListClass.getListFromString(value, separators, withMap, false);
}

/**
* @param value the string holding a serialized list
* @param separators the separator characters (given as a string) used to delimit the list's items inside the input
* string. These separators can also be present, in escaped ({@value #SEPARATOR_ESCAPE}) form, inside
* list items
* @param withMap set to true if the list's values contain map entries (key=value pairs) that should also be parsed.
* Only the keys are extracted from such list items
* @param filterEmptyValues {@code true} if the result should not contain any empty values.
* @return the list that was stored in the input string
*/
protected static List<String> getListFromString(String value, String separators, boolean withMap,
boolean filterEmptyValues)
{
List<String> list = new ArrayList<>();
if (StringUtils.isEmpty(value)) {
Expand Down Expand Up @@ -371,15 +403,19 @@ public static List<String> getListFromString(String value, String separators, bo
// in case of two consecutive identical characters different than a whitespace, then it means
// we want to record an empty value.
if (!inEscape && currentChar == previousSeparator && !StringUtils.isWhitespace(currentChar + "")) {
list.add("");
if (!filterEmptyValues) {
list.add("");
}
previousWasSeparator = false;
}
previousSeparator = currentChar;
continue;
// if we are finding a separator and we are not in escape mode, then we finished to parse one value
// we are adding the value to the result, and start a new value to parse
} else if (StringUtils.containsAny(separators, currentChar) && !inEscape) {
list.add(currentValue.toString());
if (!filterEmptyValues || !StringUtils.isEmpty(currentValue.toString())) {
list.add(currentValue.toString());
}
currentValue = new StringBuilder();
inMapValue = false;
previousWasSeparator = true;
Expand All @@ -404,7 +440,9 @@ public static List<String> getListFromString(String value, String separators, bo
}
}
// don't forget to add the latest value in the result.
list.add(currentValue.toString());
if (!filterEmptyValues || !StringUtils.isEmpty(currentValue.toString())) {
list.add(currentValue.toString());
}

return list;
}
Expand Down Expand Up @@ -536,6 +574,7 @@ public BaseProperty newProperty()
{
BaseProperty lprop;

// FIXME: this if is actually wrong: it means a multiselect static list cannot be stored with a large storage.
if (isRelationalStorage() && isMultiSelect()) {
lprop = new DBStringListProperty();
} else if (isMultiSelect()) {
Expand Down Expand Up @@ -974,10 +1013,36 @@ public List<String> toList(BaseProperty<?> property)
*/
public void fromList(BaseProperty<?> property, List<String> list)
{
if (property instanceof ListProperty) {
((ListProperty) property).setList(list);
fromList(property, list, false);
}

/**
* Set the passed {@link List} into the passed property.
*
* @param property the property to modify
* @param list the list to set
* @param filterEmptyValues if {@code true} filter out the empty values from the list.
* @since 14.2RC1
*/
protected void fromList(BaseProperty<?> property, List<String> list, boolean filterEmptyValues)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this new filterEmptyValues concept should be an actual configuration of the property class (that happen to be always true for group/user classes) instead of just a parameter in a protected method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be a global improvment to make indeed. Feels a bit too much for this PR though.

{
if (list == null && !(property instanceof ListProperty)) {
property.setValue(null);
} else {
property.setValue(list == null || list.isEmpty() ? null : list.get(0));
List<String> actualList;
if (filterEmptyValues && list != null) {
actualList = list.stream().filter(item -> !StringUtils.isEmpty(item)).collect(Collectors.toList());
} else {
actualList = list;
}

if (property instanceof ListProperty) {
((ListProperty) property).setList(actualList);
} else if (isMultiSelect()) {
property.setValue(getStringFromList(actualList, getFirstSeparator()));
} else {
property.setValue(actualList.isEmpty() ? null : actualList.get(0));
}
}
}

Expand Down
28 changes: 10 additions & 18 deletions ...m-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/classes/UsersClass.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import java.util.List;
import java.util.Map;

import org.apache.commons.lang3.StringUtils;
import org.dom4j.Element;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -57,6 +56,8 @@ public class UsersClass extends ListClass
*/
private static final String META_PROPERTY_USES_LIST = "usesList";

private static final String COMMA = ",";

/**
* Creates a new Users List property that is described by the given meta class.
*
Expand All @@ -79,6 +80,12 @@ public UsersClass()
this(null);
}

@Override
protected String getFirstSeparator()
{
return COMMA;
}

@Override
public List<String> getList(XWikiContext context)
{
Expand Down Expand Up @@ -158,13 +165,8 @@ public BaseProperty fromString(String value)
public BaseProperty fromStringArray(String[] strings)
{
List<String> list = new ArrayList<>();
for (int i = 0; i < strings.length; i++) {
if (!StringUtils.isBlank(strings[i])) {
list.add(strings[i]);
}
}
surli marked this conversation as resolved.
Show resolved Hide resolved
BaseProperty prop = newProperty();
prop.setValue(StringUtils.join(list, ','));
fromList(prop, list, true);
return prop;
}

Expand All @@ -186,7 +188,7 @@ public String getText(String value, XWikiContext context)
*/
public static List<String> getListFromString(String value)
{
return getListFromString(value, ",", false);
return getListFromString(value, COMMA, false, true);
}

@Override
Expand All @@ -210,16 +212,6 @@ public List<String> toList(BaseProperty<?> property)
return selectlist;
}

@Override
public void fromList(BaseProperty<?> property, List<String> list)
surli marked this conversation as resolved.
Show resolved Hide resolved
{
if (isMultiSelect()) {
property.setValue(list != null ? StringUtils.join(list, ',') : null);
} else {
property.setValue(list != null && !list.isEmpty() ? list.get(0) : null);
}
}

@Override
public <T extends EntityReference> void mergeProperty(BaseProperty<T> currentProperty,
BaseProperty<T> previousProperty, BaseProperty<T> newProperty, MergeConfiguration configuration,
Expand Down
Loading