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

fix: resolved inconsistency for empty string csv input #4873

Merged
merged 3 commits into from
Oct 11, 2023
Merged

fix: resolved inconsistency for empty string csv input #4873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d70aab5
fix: resolved inconsistency for empty string csv input
sfiorani Sep 29, 2023
09de175
Updated order of checkboxed and added new checkbox description
sfiorani Oct 4, 2023
794c580
Implemented suggestions
sfiorani Oct 9, 2023
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
12 changes: 12 additions & 0 deletions ...eb2/src/main/java/org/eclipse/kura/web/client/ui/drivers/assets/AssetConfigurationUi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,16 @@ interface AssetConfigurationUiBinder extends UiBinder<Widget, AssetConfiguration
@UiField
CheckBox appendCheck;
@UiField
CheckBox emptyStringCheck;
@UiField
Hidden xsrfTokenField;
@UiField
Hidden assetPidField;
@UiField
Hidden driverPidField;
@UiField
Hidden emptyStringCheckField;
@UiField
Hidden appendCheckField;
@UiField
Paragraph emptyTableLabel;
Expand Down Expand Up @@ -229,6 +233,8 @@ public void setPageStart(int index) {
.getParameterValue(AssetConstants.ASSET_DRIVER_PROP.value()));
AssetConfigurationUi.this.appendCheckField
.setValue(AssetConfigurationUi.this.appendCheck.getValue().toString());
AssetConfigurationUi.this.emptyStringCheckField
.setValue(AssetConfigurationUi.this.emptyStringCheck.getValue().toString());
AssetConfigurationUi.this.uploadForm.submit();
AssetConfigurationUi.this.uploadModal.hide();
AssetConfigurationUi.this.formSubmitCallback = context.callback(completeEvent -> {
Expand Down Expand Up @@ -680,6 +686,12 @@ private void uploadAndApply() {
this.appendCheckField.setName("doReplace");
this.appendCheckField.setValue("");

this.emptyStringCheck.setName("emptyStringCheck");
this.emptyStringCheck.setValue(false);
this.emptyStringCheckField.setID("doEmptyStringConversion");
this.emptyStringCheckField.setName("doEmptyStringConversion");
this.emptyStringCheckField.setValue("");

}

@Override
Expand Down
3 changes: 3 additions & 0 deletions ...2/src/main/java/org/eclipse/kura/web/client/ui/drivers/assets/AssetConfigurationUi.ui.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@
<b:FormGroup>
<b:FormLabel>File</b:FormLabel>
<g:FileUpload ui:field="filePath"></g:FileUpload>
<b:CheckBox ui:field="emptyStringCheck" text="{msgs.wiresEmptyStringCheck}"/>
<b:HelpBlock text="{msgs.wiresEmptyStringCheckHelpText}"/>
<b:CheckBox ui:field="appendCheck" text="{msgs.wiresAppendChannelsCheck}"/>
<b:HelpBlock text="{msgs.wiresAppendChannelsHelpText}"/>
</b:FormGroup>
Expand All @@ -115,6 +117,7 @@
<g:Hidden ui:field="assetPidField"></g:Hidden>
<g:Hidden ui:field="driverPidField"></g:Hidden>
<g:Hidden ui:field="appendCheckField"></g:Hidden>
<g:Hidden ui:field="emptyStringCheckField"></g:Hidden>
</b:FieldSet>
</b:Form>
</b:Container>
Expand Down
3 changes: 2 additions & 1 deletion .../org.eclipse.kura.web2/src/main/java/org/eclipse/kura/web/server/servlet/FileServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,9 +450,10 @@ private void doPostAsset(HttpServletRequest req, HttpServletResponse resp) throw
String csvString = new String(data, StandardCharsets.UTF_8);
String assetPid = formFields.get("assetPid");
String driverPid = formFields.get("driverPid");
Boolean nullValueAsEmptyString = Boolean.parseBoolean(formFields.get("doEmptyStringConversion"));

List<GwtConfigParameter> parametersFromCsv = AssetConfigValidator.get().validateCsv(csvString, driverPid,
errors);
errors, nullValueAsEmptyString);

final GwtConfigComponent config = new GwtConfigComponent();
config.setParameters(parametersFromCsv);
Expand Down
44 changes: 31 additions & 13 deletions ...clipse.kura.web2/src/main/java/org/eclipse/kura/web/server/util/AssetConfigValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.apache.commons.csv.CSVParser;
import org.apache.commons.csv.CSVRecord;
import org.eclipse.kura.configuration.metatype.Option;
import org.eclipse.kura.configuration.metatype.Scalar;
import org.eclipse.kura.core.configuration.metatype.Tad;
import org.eclipse.kura.driver.Driver;
import org.eclipse.kura.internal.wire.asset.WireAssetChannelDescriptor;
Expand Down Expand Up @@ -87,10 +88,10 @@ private class LineScanner {
private final List<Tad> adsByIndex;
private final List<GwtConfigParameter> defaultValues;

public LineScanner(final List<Tad> fullChannelMetatype, final List<String> columnHeaders)
throws ValidationException {
public LineScanner(final List<Tad> fullChannelMetatype, final List<String> columnHeaders,
Boolean nullValueAsEmptyString) throws ValidationException {
this.adsByIndex = probeAdsByIndex(fullChannelMetatype, columnHeaders);
this.defaultValues = probeDefaultValues(fullChannelMetatype, this.adsByIndex);
this.defaultValues = probeDefaultValues(fullChannelMetatype, this.adsByIndex, nullValueAsEmptyString);
}

private <T> Optional<T> findLast(final Collection<T> collection, final Predicate<T> predicate) {
Expand Down Expand Up @@ -169,7 +170,7 @@ private List<Tad> probeAdsByIndex(final List<Tad> fullChannelMetatype, final Lis
}

private List<GwtConfigParameter> probeDefaultValues(final List<Tad> fullChannelMetatype,
final List<Tad> boundAds) {
final List<Tad> boundAds, Boolean nullValueAsEmptyString) {
final List<Tad> missing = fullChannelMetatype.stream()
.filter(c -> boundAds.stream().noneMatch(b -> b.getId().equals(c.getId())))
.collect(Collectors.toList());
Expand All @@ -183,7 +184,8 @@ private List<GwtConfigParameter> probeDefaultValues(final List<Tad> fullChannelM
}

try {
final Object defaultValue = validate(ad, ad.getDefault(), new ArrayList<>(), 0);
final Object defaultValue = validate(ad, ad.getDefault(), new ArrayList<>(), 0,
nullValueAsEmptyString);
result.add(GwtServerUtil.toGwtConfigParameter(ad, defaultValue));
} catch (final Exception e) {
logger.warn("Ad default value is probably not valid", e);
Expand All @@ -194,7 +196,7 @@ private List<GwtConfigParameter> probeDefaultValues(final List<Tad> fullChannelM
}

public String scan(final CSVRecord line, final Consumer<GwtConfigParameter> parameterConsumer,
List<String> errors) {
List<String> errors, Boolean nullValueAsEmptyString) {
String channelName = "";
boolean errorInChannel = false;
if (line.size() != adsByIndex.size()) {
Expand All @@ -212,7 +214,7 @@ public String scan(final CSVRecord line, final Consumer<GwtConfigParameter> para
channelName = token;
}
final Tad ad = adsByIndex.get(i);
final Object value = validate(ad, token, errors, lineNumber);
final Object value = validate(ad, token, errors, lineNumber, nullValueAsEmptyString);

parameterConsumer.accept(GwtServerUtil.toGwtConfigParameter(ad, value));
} catch (Exception ex) {
Expand All @@ -230,8 +232,8 @@ public String scan(final CSVRecord line, final Consumer<GwtConfigParameter> para
}
}

public List<GwtConfigParameter> validateCsv(String csv, String driverPid, List<String> errors)
throws ServletException {
public List<GwtConfigParameter> validateCsv(String csv, String driverPid, List<String> errors,
Boolean nullValueAsEmptyString) throws ServletException {

try (CSVParser parser = CSVParser.parse(csv, CSVFormat.RFC4180)) {
errors.clear();
Expand All @@ -249,11 +251,11 @@ public List<GwtConfigParameter> validateCsv(String csv, String driverPid, List<S
final List<String> header = StreamSupport.stream(lines.remove(0).spliterator(), false)
.collect(Collectors.toList());

final LineScanner scanner = new LineScanner(fullChannelMetatype, header);
final LineScanner scanner = new LineScanner(fullChannelMetatype, header, nullValueAsEmptyString);

lines.forEach(record -> {
final List<GwtConfigParameter> params = new ArrayList<>();
String channelName = scanner.scan(record, params::add, errors);
String channelName = scanner.scan(record, params::add, errors, nullValueAsEmptyString);
if (!channelName.isEmpty() && !channels.contains(channelName)) {
channels.add(channelName);
for (final GwtConfigParameter param : params) {
Expand Down Expand Up @@ -301,16 +303,18 @@ private String errorString(int lineNumber, Tad field, String value) {
}

// Validates all the entered values
protected Object validate(Tad field, String value, List<String> errors, int lineNumber) throws ValidationException {
protected Object validate(Tad field, String value, List<String> errors, int lineNumber,
Boolean nullValueAsEmptyString) throws ValidationException {

String trimmedValue = value.trim();
final boolean isEmpty = trimmedValue.isEmpty();
final boolean convertNullToEmptyString = nullToEmptyStringValueManagement(field, nullValueAsEmptyString);

if (field.isRequired() && isEmpty) {
throw new ValidationException();
}

if (!isEmpty) {
if (validateTrimmedValue(isEmpty, convertNullToEmptyString)) {
// Validate "Options" field first. Data type will be taken care of next.
if (!field.getOption().isEmpty()) {
boolean foundEqual = false;
Expand Down Expand Up @@ -361,6 +365,20 @@ protected Object validate(Tad field, String value, List<String> errors, int line
return null;
}

private boolean nullToEmptyStringValueManagement(Tad field, Boolean nullValueAsEmptyString) {

boolean returnValue = false;

if (nullValueAsEmptyString.booleanValue()) {
returnValue = field.getType().equals(Scalar.STRING);
}
return returnValue;
}

private boolean validateTrimmedValue(boolean isEmpty, boolean convertNullToEmptyString) {
return !isEmpty || convertNullToEmptyString;
}

protected interface ValidationErrorConsumer {

public void addError(String errorMsg);
Expand Down
4 changes: 3 additions & 1 deletion ...pse.kura.web2/src/main/resources/org/eclipse/kura/web/client/messages/Messages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,8 +771,10 @@ wiresDownloadChannels=Download Channels
wiresUploadChannels=Upload Channels
wiresNewChannel=New Channel
wiresDeleteChannel=Delete Channel
wiresEmptyStringCheck=Force import empty string policy.
wiresEmptyStringCheckHelpText=If checked, empty values contained in the csv will be parsed as empty strings, for all those fields whose type is <String>.
Copy link
Contributor

Choose a reason for hiding this comment

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

If checked, empty values contained in the csv will be parsed as empty strings, for all those fields whose type is

Maybe better like this: "If checked, all typed channel fields that have an empty value in the input csv will be parsed as empty strings."

wiresAppendChannelsCheck=Replace current channels.
wiresAppendChannelsHelpText=If checked, channels will be replaced by the imported ones. Otherwise, existing Asset configuration will be merged with the imported one, parameters values of imported channels will override the existing ones. The change will not be applied until Asset configuration is saved.
wiresAppendChannelsHelpText=If checked, all typed channel fields that have an empty value in the input csv will be parsed as empty strings.

wiresEmitters=Emitters
wiresReceivers=Receivers
Expand Down