CachedInstanceLoader defaults to empty when import_id is missing

[DonQueso89]

Problem

CachedInstanceLoader tries to clean the import_id field, even when it is not present in the inbound dataset, resulting in a KeyError thrown in fields.py. When using ModelInstanceLoader, an absent import_id column is allowed and results in the entries all being new, which is (to my mind) the most desirable behavior.

Solution

Refactor CachedInstanceLoader to initialize with an empty cache when it gets a dataset without the import_id column. get_instance is then never called by the Resource since it checks whether the import_id_fields are present in the inbound dataset before calling get_instance.

Acceptance Criteria

Added a unit test which demonstrates the KeyError in the previous code and shows the correct initialization of CachedInstanceLoader in the new code.

Documentation is not necessary since this makes no change to a public API. Code using CachedInstanceLoader which has implemented a workaround for this problem should still work since most workarounds will probably have implemented some hook to guarantee the presence of the import_id field. In the unlikely event that someone is explicitly checking for a KeyError and basing their behavior on that, they will have to update their code.

[coveralls]

Coverage increased (+0.02%) to 96.754% when pulling 32a5c57 on DonQueso89:develop into ceb6bf0 on django-import-export:master.

[matthewhegarty]

I think you have done a good job here. This is a safer way of handling imports which don't include the pk field.

[DonQueso89]

I think you have done a good job here. This is a safer way of handling imports which don't include the pk field.

thank you 😄

[DonQueso89]

@matthewhegarty will this be merged for the next release?

[matthewhegarty]

Looping in @andrewgy8 as he will likely be reviewing changes for the next release.

[andrewgy8]

LGTM. I just released 2.5.0 so that we can get a clean release with this change! Thanks!