Hi Felix, I've taken into account your first remarks. I let you fix the remaining ones.
Thanks for your feedback! Regards On Wed, Nov 22, 2017 at 7:36 AM, Felix Schumacher < felix.schumac...@internetallee.de> wrote: > > > Am 20. November 2017 20:50:51 MEZ schrieb pmoua...@apache.org: > >Author: pmouawad > >Date: Mon Nov 20 19:50:51 2017 > >New Revision: 1815838 > > > >URL: http://svn.apache.org/viewvc?rev=1815838&view=rev > >Log: > >Bug 61759 - New __changeCase function > >Contributed by Orimarko > >Bugzilla Id: 61759 > > > >Added: > >jmeter/trunk/src/functions/org/apache/jmeter/functions/ChangeCase.java > > (with props) > >jmeter/trunk/test/src/org/apache/jmeter/functions/TestChangeCase.java > >(with props) > >Modified: > > jmeter/trunk/xdocs/changes.xml > > > jmeter/trunk/xdocs/usermanual/functions.xml > > > >Added: > >jmeter/trunk/src/functions/org/apache/jmeter/functions/ChangeCase.java > >URL: > >http://svn.apache.org/viewvc/jmeter/trunk/src/functions/ > org/apache/jmeter/functions/ChangeCase.java?rev=1815838&view=auto > >=========================================================== > =================== > >--- > >jmeter/trunk/src/functions/org/apache/jmeter/functions/ChangeCase.java > >(added) > >+++ > >jmeter/trunk/src/functions/org/apache/jmeter/functions/ChangeCase.java > >Mon Nov 20 19:50:51 2017 > >@@ -0,0 +1,175 @@ > >+/* > >+ * Licensed to the Apache Software Foundation (ASF) under one or more > >+ * contributor license agreements. See the NOTICE file distributed > >with > >+ * this work for additional information regarding copyright ownership. > >+ * The ASF licenses this file to You 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.apache.jmeter.functions; > >+ > >+import java.util.Collection; > >+import java.util.EnumSet; > >+import java.util.LinkedList; > >+import java.util.List; > >+import java.util.regex.Pattern; > >+ > >+import org.apache.commons.lang3.StringUtils; > >+import org.apache.jmeter.engine.util.CompoundVariable; > >+import org.apache.jmeter.samplers.SampleResult; > >+import org.apache.jmeter.samplers.Sampler; > >+import org.apache.jmeter.util.JMeterUtils; > >+import org.slf4j.Logger; > >+import org.slf4j.LoggerFactory; > >+ > >+/** > >+ * Change Case Function > >+ * > >+ * Support String manipulations of: > >+ * <ul> > >+ * <li>upper case</li> > >+ * <li>lower case</li> > >+ * <li>capitalize</li> > >+ * <li>camel cases</li> > >+ * <li></li> > >+ * > >+ * > >+ * @since 4.0 > >+ * > >+ */ > >+public class ChangeCase extends AbstractFunction { > >+ > >+ private static final Pattern NOT_ALPHANUMERIC_REGEX = > >+ Pattern.compile("[^a-zA-Z]"); > > The regex doesn't include numeric, so it probably should be called not > alpha. > yes > > Would not it be better to allow groups of non wanted characters by adding > a +? > Yes > > But it might be nicer to split on non word chars. That would be \W+. > > >+ private static final Logger LOGGER = > >LoggerFactory.getLogger(ChangeCase.class); > >+ private static final List<String> DESC = new LinkedList<>(); > >+ private static final String KEY = "__changeCase"; > >+ > >+ private static final int MIN_PARAMETER_COUNT = 1; > >+ private static final int MAX_PARAMETER_COUNT = 3; > >+ > >+ static { > >+ DESC.add(JMeterUtils.getResString("change_case_string")); > >+ DESC.add(JMeterUtils.getResString("change_case_mode")); > >+ DESC.add(JMeterUtils.getResString("function_name_paropt")); > >+ } > >+ > >+ private CompoundVariable[] values; > >+ > >+ @Override > >+ public String execute(SampleResult previousResult, Sampler > >currentSampler) throws InvalidVariableException { > >+ String originalString = values[0].execute(); > >+ String mode = ChangeCaseMode.UPPER.getName(); // default > >+ if (values.length > 1) { > >+ mode = values[1].execute(); > >+ } > >+ String targetString = changeCase(originalString, mode); > >+ addVariableValue(targetString, values, 2); > >+ return targetString; > >+ } > >+ > >+ protected String changeCase(String originalString, String mode) { > >+ String targetString = originalString; > >+ // mode is case insensitive, allow upper for example > >+ ChangeCaseMode changeCaseMode = > >ChangeCaseMode.typeOf(mode.toUpperCase()); > >+ if (changeCaseMode != null) { > >+ switch (changeCaseMode) { > >+ case UPPER: > >+ targetString = StringUtils.upperCase(originalString); > >+ break; > >+ case LOWER: > >+ targetString = StringUtils.lowerCase(originalString); > >+ break; > >+ case CAPITALIZE: > >+ targetString = StringUtils.capitalize(originalString); > >+ break; > >+ case CAMEL_CASE: > >+ targetString = camel(originalString, false); > >+ break; > >+ case CAMEL_CASE_FIRST_LOWER: > >+ targetString = camel(originalString, true); > >+ break; > >+ default: > >+ // default not doing nothing to string > >+ } > >+ } else { > >+ LOGGER.error("Unknown mode {}, returning {}Â unchanged", > > A strange char seems to have crept in. Probably a white space on Mac. > > >mode, targetString); > >+ } > >+ return targetString; > >+ } > >+ > >+ @Override > >+ public void setParameters(Collection<CompoundVariable> parameters) > >throws InvalidVariableException { > >+ checkParameterCount(parameters, MIN_PARAMETER_COUNT, > >MAX_PARAMETER_COUNT); > >+ values = parameters.toArray(new > >CompoundVariable[parameters.size()]); > >+ } > >+ > >+ @Override > >+ public String getReferenceKey() { > >+ return KEY; > >+ } > >+ > >+ @Override > >+ public List<String> getArgumentDesc() { > >+ return DESC; > >+ } > >+ > >+ private static String camel(String str, boolean > >isFirstCapitalized) { > >+ StringBuilder builder = new StringBuilder(str.length()); > >+ String[] tokens = NOT_ALPHANUMERIC_REGEX.split(str); > >+ for (int i = 0; i < tokens.length; i++) { > >+ if(i == 0) { > >+ builder.append(isFirstCapitalized ? tokens[0]: > > I am surprised by the value of isFirstCapitalized, as it means to me - > what is the current state. > Changed this after taking into account your first remarks > > I think it would be better named upperCaseFirstChar or something along > that line. > yes > > >+ StringUtils.capitalize(tokens[i])); > >+ } else { > >+ builder.append(StringUtils.capitalize(tokens[i])); > >+ } > >+ } > >+ return builder.toString(); > >+ } > >+ > >+ /** > >+ * ChangeCase Modes > >+ * > >+ * Modes for different cases > >+ * > >+ */ > >+ public enum ChangeCaseMode { > >+ UPPER("UPPER"), LOWER("LOWER"), CAPITALIZE("CAPITALIZE"), > >CAMEL_CASE("CAMEL_CASE"), CAMEL_CASE_FIRST_LOWER( > >+ "CAMEL_CASE_FIRST_LOWER"); > >+ private String mode; > >+ > >+ private ChangeCaseMode(String mode) { > >+ this.mode = mode; > >+ } > >+ > >+ public String getName() { > >+ return this.mode; > >+ } > >+ > >+ /** > >+ * Get ChangeCaseMode by mode > >+ * > >+ * @param mode > >+ * @return relevant ChangeCaseMode > >+ */ > >+ public static ChangeCaseMode typeOf(String mode) { > >+ EnumSet<ChangeCaseMode> allOf = > >EnumSet.allOf(ChangeCaseMode.class); > >+ for (ChangeCaseMode zs : allOf) > > Why is this variable named zs? > Fixed > > Regards > Felix > > -- Cordialement. Philippe Mouawad.
