Potential problem with BatchUploader FK-lookup

[Isomorphic]

You seem to be trying to use BatchUploader to go directly to production-ready data with no intervening review step. The latter is a typical use case, and in that scenario, you don't want to throw error messages at the end user blocking them from importing data that doesn't perfectly match the DB.

So again, you have a way to be strict if you want, but it is not a bug for our product to have a permissive mode. It's, if anything, the more common use case.

[Blama]

Hi Isomorphic,

unfortunately I don't control the data for #1, so IMHO this is a bug, because a end user will reasonably expect such behavior you describe with importStrategy="displayRequired" and I must live with the chance that an external ID is numerical. Of course an overlap is unlikely, but still possible.

A thing like importStrategy="displayRequired" would solve #1 (and also #3 without the need for a hasRelatedRecord-validator).
Additionally I don't see an use case, where current importStrategy="display" would have an advantage over your suggested importStrategy="displayRequired", making this look like a bug to me.

Best regards
Blama

[Isomorphic]

So again, for #1, we would recommend setting up the data so you don't have an ambiguity between display value and ID, and for #3, we would recommend the hasRelatedRecord validator.

It's true there's room for additional settings here that could optimize your #3 case, or allow you to work with ambiguous data as in #1. Perhaps something like importStrategy="displayRequired" with the behavior of treating a failed lookup as a hard failure instead of just using the provided value. But this would be an enhancement, not a bug.

[Blama]

Hi Isomorphic,

this is true for my post #3. It's not true for #1, where you would upload randomly wrong data.

And for #3, without a framework code change there would be unnecessary DB workload, as you essentially do a:

SELECT id FROM lookuptable WHERE name = 'csvdata'; --Get Id
....
SELECT id FROM lookuptable WHERE id = previously_looked_up_id; --Check if record exists

Fixing the issue from #1 will also fix #3 without a need for a hasRelatedRecord validator.

Best regards
Blama

[Isomorphic]

Sorry, we still see this as a data problem, and not a framework bug. The framework is simply trying both ways to interpret the uploaded value, a good behavior. If you don't want that, you already understand how to avoid it via a hasRelatedRecord validator.

[Blama]

Hi Isomorphic,

this (or a similar thing with the same root cause) now happened for a customer.

They use an external ID as their logical name, this is looked up via importStrategy="display" to find the real internal Database ID and does not find anything.
This is now NOT flagged as an error in the Uploader GUI. It would, if I also define a hasRelatedRecord validator, but this is not correct IMHO and should not be necessary.

Also, if I use a Text in the column in the CSV-file, this is flagged as problem because of the not matching data type. This is kinda expected, but it is unclear, why two (logical) wrong (because they don't exist) cells result in a different outcome.

Now, the customer does not see any error and hits "Commit", causing a problem because of failing database requests (foreignKey errors).

The root cause here (just taking the text-value from the CSV file if the lookup fails) is the same as in #1.

If necessary, I can create an Oracle sequence-PK based testcase for this, but I hope you already see what the problem here is.
This update now is a milder version of #1, where the Batchuploader will add wrong data.

Best regards
Blama

[Isomorphic]

This is a potential problem if your dataset is not properly set up for the import. But it doesn't seem like something the framework should somehow handle for you.

[Blama]

Hi Isomorphic,

I noticed a potential problem in BatchUploader with FK-lookup (importStrategy="display"). I'll describe it before creating a testcase, because it's not really a bug.

Imagine a numberish string like supplyItem.SKU. supplyItem still has itemID as sequence-PK.

        <field name="supplyItem_ID" importStrategy="display" uploadFieldName="SKU" displayField="SKU" foreignKey="supplyItem.itemID"
            required="true">
            <title><fmt:message key="SKU" /></title>
            <validators>
                <validator type="hasRelatedRecord" />
            </validators>
        </field>

The data looks like this:

itemID SKU
0 "1"
1 "2"
2 "3"
...

Now I delete itemID=1

Now imagine I want to upload an item with SKU = "2" in the CSV, but it does not exist in the DB (because I deleted it). So the lookup for the SKU fails, and supplyItem_ID value is still 2 (=the value from the CSV).
Next, the DB-insert will be tried with supplyItem_ID=2, which does exist, so the insert will succeed, but the new row will link to a different item.

This problem can occur everywhere where the datatype of displayfield and datafield is the same or a cast succeeds.

Best regards
Blama