Add KingfisherTransformMiddleware and update affected spiders

[yolile]

closes #329, closes #449, closes #450, closes #471, closes #573, closes #574

(this will also solve getting only samples for compressed files and also being able to pluck them)

@jpmckinney this is a draft, and before updating the documentation and tests I would like to read your opinion.

[jpmckinney]

Looks good! Some comments.

The data_pointer logic is doing something similar to root_path. Should we switch it to root_path?

[jpmckinney]

Do we need both file_format and compressed_file_format or can we collapse them?

[yolile]

I added that just for digiwhist_base that currently doesn't extend from CompressedFileSpider

[jpmckinney]

Okay, we can maybe look at combining them in a follow-up PR.

[jpmckinney]

Follow-up issue is here: #574

[yolile]

@jpmckinney I ended up removing it now

[jpmckinney]

I reviewed my earlier comments. I'll read the diff in full later.

[jpmckinney]

If there are many items in result in process_spider_output, then I guess we'll still do extra processing, but it will at least be limited.

[jpmckinney]

I've read the diff now. Looks good! Just those few comments left.

[jpmckinney]

One comment to refactor to make code easier to analyze.

[jpmckinney]

I think this comment isn't quite right. One or more of the following is true: data_type is release or record, or compressed_file_format is release_package, or root_path is non-empty.

As such, I think the method name should also be changed (or maybe we should have different methods).

For example, is afghanistan_records/releases processed correctly? https://ocds.ageops.net/api/record/5ed2a62c4192f32c8c74a4e5 returns a single record. We can wrap it in a simple package, and then return it as a file, instead of as a file item. This can be done in a separate, simpler method.

The _parse_json_array method is very hard to analyze, because it also gets called from _parse_json_lines. I think we should have multiple methods for specific stages. For example, maybe:

• items = _split_data(...): if it's json lines, yields each line, otherwise yields data back
• items = _apply_root_path(items, ...): if root_path is set, calls ijson.items and yields each value, otherwise yields each item back
• items = _add_package(items, ...): if data_type is release or record, adds a simple package to each item, otherwise yields each item back
• _resize_packages: does the self.MAX_RELEASES_PER_PACKAGE logic if needed (I think only absolutely necessary when compressed_file_format = 'release_package')
• finally, yields each item as a File (if the items generator contains one item) or a FileItem

[jpmckinney]

Actually, maybe we should have separate, simpler middlewares (similar idea to the above)?

[yolile]

@jpmckinney thank you for your changes and improvements!

[jpmckinney]

Note that I had to change test_data_types - please review

@yolile Can you check the logic in test_data_types that was changed in this commit? b2beb4d

[jpmckinney]

@yolile More specifically, I was wondering about: b2beb4d#diff-1071f4e84a30adb8a2b145f878256523e64f9aa931b5a41dceec4320f9d3420fL75-R78

Originally, the response_mock's data key was changed, and then submitted along with the original item to add_package_middleware. In the new code, the response_mock is not changed, and the item returned by the root_path_middleware middleware is submitted to add_package_middleware. I just wanted to check that this was correct. I wasn't sure why the response_mock was changed in the old code.

[yolile]

Originally, the response_mock's data key was changed, and then submitted along with the original item to add_package_middleware. In the new code, the response_mock is not changed, and the item returned by the root_path_middleware middleware is submitted to add_package_middleware. I just wanted to check that this was correct. I wasn't sure why the response_mock was changed in the old code.

@jpmckinney I think that was redundant and it is ok to remove it, as actually we don't use the response in the middleware, just result

[jpmckinney]

@yolile Thanks! I've removed it, so I'll merge this once CI is done. 🎉

[yolile]

@jpmckinney awesome, I'm so happy right now, look at all these closed issues, it's beautiful!

[jpmckinney]

Same! ☺️