clouway / twig-persist

twig-persist
0 stars 3 forks source link

Add option for reset of type converters #17

Closed georgievJon closed 7 years ago

georgievJon commented 7 years ago

This option can be used in tests.

mgenov commented 7 years ago

Could you elaborate ?

mgenov commented 7 years ago

Introduction of such method will force callers to use it in tests which doesn't sounds good.

georgievJon commented 7 years ago

Type converters are registered in static object only once

    // volatile to allow double checked locking
    private volatile static ConverterRegistry registry;

    private TypeConverter converter;

    @Override
    public TypeConverter getTypeConverter()
    {
        if (converter == null)
        {
            // TODO problem if more than one subclass - set converters in constructor
            if (registry == null)
            {
                synchronized (this)
                {
                    if (registry == null)
                    {
                        registry = createStaticConverterRegistry();
                    }
                }
            }

            // these are added for every OD instance
            ChainedTypeConverter chain = new ChainedTypeConverter();
            chain.add(registry);
            chain.add(new CollectionTypeConverter(chain));
            chain.add(new IterableToArray(chain));
            chain.add(new ArrayToIterable(chain));
            chain.add(new IterableToFirstElement());

            converter = chain;
        }
        return converter;
    }

In test we wont to register different converters for different modules. But which tests are run first their converters are used for all next test.

With this method we can reset converters for every test.

mlesikov commented 7 years ago

it will be in the DatastoreRule, there is a static type convertor register that improves the instantiation in production but in the tests, when we use it in two different modules with separate converters it does not clears when we use it in the second module

mgenov commented 7 years ago

As I know converters are not keeping any state. Why they have to be registered again ?

mlesikov commented 7 years ago

no the problem is that we have another AdmObjectDatastore in the bss-app that extends the AdmObjectDatastore form the core. actaully we have type converter for a Llist which belongs to the bss-app module and it is not visible in the core, so we need to register it in the twig object datastore in the bss-app(that's why I need that new ObjectDatastore).

During the single wun for the module bss-app, we instantiate only the bss-app's ObjectDatastore, but on the jenkins the tests of the core were ran firs and the static convertere register was filled only with the converters of the core module - that makes the tests failing, because there was no converters for them when running tests for the bss=app after the core's tests

is that make sense?

I plan to use that reset method only in the tests, because in the production there will be no problem because we use only the bss-app's ObjectaDatastore that has it's own and the core's converters. after all the adaptees are moved from the core to bss-app this will be no need any more and the core's (parent) ObjectDatastore can be deleted

mgenov commented 7 years ago

Actually the problem is that the core module is using twig-persist. I suppose it should be considered instead of changing the API.