Partial data import

[stmichael]

I started by decoupling the lookup tables from the definitions. It's not finished yet. There is still code that uses lookups on definitions.

@senny what do you think about it so far?

[stmichael]

Also references to lookup tables are still hold on the definitions. If we really want to make the lookup an independent feature, we have to put it somewhere else.

[senny]

We split out the Lookup to have a separate entity, that we can reason about. If you think in the terms of a LookupTable it does not care when a row was imported. The method on the lookup should have a more intention revealing name.

[stmichael]

I created an id mapping container which holds all id dictionaries. The container is held by the execution plan. I'm not happy the way it is used because I have to pass it to several initializations. @senny any idea on how to change that? May the container is in the wrong place.

The syntax to use the id mappings in the blocks is now:

id_mapping_for('definition name', 'id lookup name').lookup(old_value)
id_mapping_for('definition name', 'id lookup name').add(old_value, new_value)

[senny]

I think we can move this class outside the Definition namespace.

[senny]

@stmichael I think the IdMappingContainer belongs to the ExecutionPlan. I think the DSL objects should always have access to the plan, which is being built. They are coupled to the plan very tightly anyways because their only intention is to build the plan. So I suggest you pass along the whole plan in place of only the mapping container. I also think you should bridge the methods to add a new dictionary on the ExecutionPlan so that you don't need to reach through the object to grab the container.

[stmichael]

I updated the branch with the discussed changes. @senny If you agree I will go on to the implementation of the partial import

[stmichael]

This is a first version of a partial import. What do you think about it?

[stmichael]

The next step would be to extract the loading and storage of the settings into a separate class to keep it together. Also in the unit test of the partial import I directly write to the settings file as test setup. That's not flexible since it's closely tied to the implementation.

[senny]

row_importer typo

[stmichael]

What do you think about it now?

[senny]

Why don't we use rspec-mocks? Is it not relevant if save was called?

[senny]

@stmichael I'm a little short on review time at the moment. I have some general spots I'm not that happy with but don't have prepared suggestions how to improve:

1. The Naming seems a bit off. (eg. SettingsStore does not store settings but more progress related data, also the IdMappingsXXXX part does not communicate clearly to me. => very subjective comment)
2. I would not use Inheritance in the Migration Strategy but Composition.
3. I'm not sure if we want a two step process to lookup an ID, eg: id_mapping_for(...).lookup(...). This was the source that we had to deprecate the complete API and I don't think we should do that in the future.