PO Cache Changes

[Heng Sin Low]

Hi All,

When changes for https://idempiere.atlassian.net/browse/IDEMPIERE-4287 Cache API not thread safe and inconsistent with context parameter landed to the master branch, it will bring the following significant changes to global/static PO cache implementation:

1. To make global cache for PO thread safe, most of the cache is changed to store and return immutable PO ( for e.g MProduct.get(Properties ctx, int M_Product_ID) ). For get method that take trxName parameter, the trxName is only used to retrieve the PO from db (if it is not in cache yet) and the PO return is immutable and without trxName.
2. The immutable cache is implemented using the newly added ImmutableIntPOCache (if cache key is integer) and ImmutablePOCache class (extends CCache). To work with the 2 cache implementation, the model class must implement the newly added ImmutablePOSupport interface (see MProduct for reference). For better efficiency, the model class should provide a copy constructor as well (see MProduct for reference).
3. For compatibility concern, some cache is implemented using copy on read and write approach ( for e.g MPrintTableFormat.get(Properties ctx, int AD_PrintTableFormat_ID, Font standard_font) ). This is implemented using the newly added IntPOCopyCache (if cache key is integer) and POCopyCache class
4. For some model class, additional getCopy method is added to facilitate the retrieval of an updateable copy of the model instance from cache, for e.g MProduct.getCopy(Properties ctx, int M_Product_ID, String trxName).
5. Most of iDempiere's cache is coded following the public static ModelClass get(Properties ctx, int ModelClass_ID) pattern. However, this is undesirable for system level, global cache, the ctx shouldn't be part of the conversation here. For e.g, most of the call to MProduct.get(Properties ctx, int M_Product_ID) would just end up as MProduct.get(Env.getCtx(), MProduct_ID) making the first parameter, Env.getCtx() unnecessary. For this reason, a version of the get method without the context part is being added to many model class. Due to the large number of classes affected, I didn't change existing call but new code should use the version that doesn't has the ctx parameter (i.e use MProduct.get(M_Product_ID) instead of MProduct.get(ctx, M_Product_ID)).
6. A bad usage pattern of cache in some of the existing code in core (perhaps in some of the plugin codes out there as well) is to get a PO instance from cache and perform an update with it. For e.g, MProduct p = MProduct.get(Env.getCtx(), M_Product_ID); ... p.saveEx(). This is bad for the following reasons (note that with the new immutable cache, this is not supported anymore) :
1. This is just not a good use of cache since PO cache will be reset after the update call. It is even worse if the PO is not in cache yet as it is just a pure waste of some CPU time - get PO from DB, add PO to cache, update PO, update trigger reset of cache and PO is removed from cache.
2. Doing an update with PO instance from cache means potentially you are performing an update with stall/outdated data. This can sometimes lead to corrupted data.
7. New PO cache should be implemented using ImmutableIntPOCache (if key of cache is integer) or ImmutablePOCache. Unless there's a specific need for it, the cache should only have a get method that takes a single id parameter. For e.g, if model class is MyModel, static cache would be private final static ImmutableIntPOCache<Integer, MyModel> s_cache = new ImmutableIntPOCache<>(MyModel.Table_Name, 100) and 1 get method of public MyModel get(int MyModel_ID).
8. For existing code, if you are concerned with breaking existing caller, you can use IntPOCopyCache or POCopyCache. This copy on read and write cache is thread safe and wouldn't break existing code. The price to pay is some performance and memory overhead.
9. If you encounter the "PO is immutable: ModelClassName" exception, this is due to the violation of the immutable cache rule, i.e trying to make changes to a PO retrieve from cache. If the code is from core, please report it at the jira ticket above. If it is your custom code, you should consider getting your PO from db instead of from cache, i.e replace YourModelClass.get(...) with new YourModelClass(ctx, id, trxName).

Regards,

Low