Repository navigation
Feat #8767 : CreditCardValidator: Add support for custom BIN validation via CSV file - #8783
figonugroho wants to merge 1 commit into
Conversation
CreditCardValidator: Add support for custom BIN validation via CSV file
| for (BinOutputField outputField : outputFields) { | ||
| String realName = variables.resolve(outputField.getName()); | ||
| if (!Utils.isEmpty(realName)) { | ||
| IValueMeta v = new ValueMetaString(realName); |
There was a problem hiding this comment.
[bug] Extra BIN output fields are always added as ValueMetaString. The dialog stores type, length, precision, and format on BinOutputField, and convertValue can return a Long, Double, or Date, but none of those properties are copied onto the row meta. Downstream steps then cast that object to String (ValueMetaString.getBoolean / getInteger) or create the wrong column type.
Suggestion: Create the meta with ValueMetaFactory.createValueMeta(realName, outputField.getType(), outputField.getLength(), outputField.getPrecision()) and copy the conversion mask, decimal/group/currency symbols, trim type, and origin. Cover it with a getFields test that an Integer mapping is not a String.
| valueMeta.setCurrencySymbol(field.getCurrency()); | ||
| valueMeta.setTrimType(field.getTrimType()); | ||
| return valueMeta.convertDataFromString( | ||
| value, new ValueMetaString(field.getName()), null, null, ValueMetaString.TRIM_TYPE_NONE); |
There was a problem hiding this comment.
[bug] The Trim column never affects the value. setTrimType is set on the target meta, but convertDataFromString trims with its last argument, which is hard-coded to TRIM_TYPE_NONE. The catch then returns the original String, so one row can hold a Long and the next row a String in the same field.
Suggestion: Pass field.getTrimType() into convertDataFromString. On a conversion failure, return null or route the row to error handling instead of a value of the wrong runtime type. Build the IValueMeta once per field in init, not on every row.
| } | ||
| } | ||
| } | ||
| } else { |
There was a problem hiding this comment.
[bug] With "Header row present" unchecked, extraIdx stays empty, so every extra output field is null for the whole run. The dialog still accepts CSV-column mappings, and readHeader always treats the first line as a header, ignoring that checkbox. Mappings created from Get columns therefore never load.
Suggestion: When there is no header, map extra fields by column index, or fail init with a clear message if a mapping needs a header name. Make Get columns honor the header checkbox.
| List<BinOutputField> outputFields = new ArrayList<>(); | ||
| int rows = wOutputFields.nrNonEmpty(); | ||
| for (int i = 0; i < rows; i++) { | ||
| String[] row = wOutputFields.getItem(i); |
There was a problem hiding this comment.
[bug] nrNonEmpty() skips blank rows, but the loop reads getItem(i), which is the raw table index. TableView only fills the non-empty index in nrNonEmpty() for a following getNonEmpty(i) call. A blank row above a filled one makes the last mapping fall outside 0..nrNonEmpty-1, so OK drops it.
Suggestion: Keep the nrNonEmpty() call, then read TableItem item = wOutputFields.getNonEmpty(i) and take the cells with item.getText(1) through item.getText(10).
| } | ||
| // add extra output fields? | ||
| for (BinOutputField outputField : meta.getOutputFields()) { | ||
| if (!Utils.isEmpty(outputField.getName())) { |
There was a problem hiding this comment.
[bug] getFields adds a column only when the resolved field name is non-empty, but this loop writes a column whenever the raw name is non-empty. A name such as ${BIN_FIELD} that resolves to empty still increments rowIndex and writes past the array allocated from data.outputRowMeta.size(). The same loop also runs when "Use BIN database" is off, so the extra columns stay in the stream as nulls.
Suggestion: Resolve the name once and use that both here and in getFields. Skip the loop unless meta.isUseBinDatabase() is true, in both places, so the schema and the row stay aligned.
| BaseMessages.getString( | ||
| PKG, "CreditCardValidator.Error.BinColumnNotFound", binCsvColumn)); | ||
| } | ||
| cardTypeIdx = resolveColumn(header, null, 1, "cardtype", "card_type", "card type"); |
There was a problem hiding this comment.
[suggestion] If the header has no cardtype, card_type, or card type column, resolveColumn returns index 1. The sample in the issue uses brand, which is not one of those names, so the card type becomes whatever is in column 2. The same positional fallback is applied to issuer (index 2) and country (index 3), so a different column order silently mis-labels the card.
Suggestion: Also accept brand, scheme, and type. When nothing matches, return -1 and leave the value empty instead of reading another column.
| String[] fields; | ||
| while ((fields = reader.readNext()) != null) { | ||
| String rawBin = field(fields, binIdx).trim(); | ||
| if (rawBin.isEmpty() || rawBin.length() > MAX_BIN_LENGTH) { |
There was a problem hiding this comment.
[suggestion] A BIN longer than 8 characters, or one that is not all digits, is skipped with no log. Those cards later fail lookup as "not valid" even though the row was in the file. Eight digits matches the current IIN length, but several BIN extracts use a longer prefix.
Suggestion: Count skipped rows and log that count from init. Either keep the prefix as a digit string so it can be longer than 8, or state the 8-digit limit on the BIN file control.
| String cardType = dedup(field(fields, cardTypeIdx)); | ||
| String issuer = field(fields, issuerIdx).trim(); | ||
| String country = dedup(field(fields, countryIdx)); | ||
| Map<String, String> extraValues = new HashMap<>(); |
There was a problem hiding this comment.
[suggestion] Every CSV row allocates its own HashMap, even when no extra columns are mapped, and the lookup table is constructed for 1,000,000 entries before the file is read. Issuer and country are stored on every record but are never written to the row unless the user maps those columns again as extra fields. Each transform copy loads its own copy during init.
Suggestion: Share one empty map when extraIdx is empty, size the lookup from the file instead of pre-allocating a million slots, and skip issuer and country unless an output field needs them.
2 participants
Feature #8767 CreditCardValidator: Add support for custom BIN validation via CSV file
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean install apache-rat:checkto make sure basic checks pass. A more thorough check will be performed on your pull request automatically.git rebase -i.addresses #123), if applicable.To make clear that you license your contribution under the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.