(ofbiz-framework) branch trunk updated: Keep configurable product selections out of the shared product config cache

[mr...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mridulpathak pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 50d548501d Keep configurable product selections out of the shared product config cache
50d548501d is described below

commit 50d548501d6504b29e8bf9cb7b3370c8e5d6bdfa
Author: Mridul Pathak <mr...@gmail.com>
AuthorDate: Fri Oct 30 14:36:34 2026 +0530

    Keep configurable product selections out of the shared product config cache
    
    ProductConfigWorker.getProductConfigWrapper returned the cached ProductConfigWrapper itself on a cache miss, so the first customer's selections and comments were written into the cache and copied to every later visitor. It now always returns a copy. Copies were not independent either: a copied ConfigOption kept the source option's parent ConfigItem and shared its componentOptions map, so copied options now belong to the copied item and get their own map.
---
 .../ofbiz/product/config/ProductConfigWorker.java    | 20 ++++++++++----------
 .../ofbiz/product/config/ProductConfigWrapper.java   | 10 +++++++---
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWorker.java b/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWorker.java
index 08ea4d2e17..c972b82699 100644
--- a/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWorker.java
+++ b/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWorker.java
@@ -68,17 +68,17 @@ public final class ProductConfigWorker {
             Delegator delegator = (Delegator) request.getAttribute("delegator");
             String cacheKey = productId + SEPARATOR + productStoreId + SEPARATOR + catalogId + SEPARATOR + webSiteId + SEPARATOR
                     + currencyUomId + SEPARATOR + delegator;
-            configWrapper = PRODUCT_CONFIG_CACHE.get(cacheKey);
-            if (configWrapper == null) {
-                configWrapper = new ProductConfigWrapper((Delegator) request.getAttribute("delegator"),
-                                                         (LocalDispatcher) request.getAttribute("dispatcher"),
-                                                         productId, productStoreId, catalogId, webSiteId,
-                                                         currencyUomId, UtilHttp.getLocale(request),
-                                                         autoUserLogin);
-                configWrapper = PRODUCT_CONFIG_CACHE.putIfAbsentAndGet(cacheKey, new ProductConfigWrapper(configWrapper));
-            } else {
-                configWrapper = new ProductConfigWrapper(configWrapper);
+            ProductConfigWrapper cachedWrapper = PRODUCT_CONFIG_CACHE.get(cacheKey);
+            if (cachedWrapper == null) {
+                cachedWrapper = PRODUCT_CONFIG_CACHE.putIfAbsentAndGet(cacheKey,
+                        new ProductConfigWrapper((Delegator) request.getAttribute("delegator"),
+                                                 (LocalDispatcher) request.getAttribute("dispatcher"),
+                                                 productId, productStoreId, catalogId, webSiteId,
+                                                 currencyUomId, UtilHttp.getLocale(request),
+                                                 autoUserLogin));
             }
+            // callers fill in the customer's selections, so never hand out the cached instance itself
+            configWrapper = new ProductConfigWrapper(cachedWrapper);
         } catch (Exception e) {
             Debug.logWarning(e.getMessage(), MODULE);
             configWrapper = null;
diff --git a/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWrapper.java b/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWrapper.java
index a9795b5325..f9d0230744 100644
--- a/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWrapper.java
+++ b/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWrapper.java
@@ -518,7 +518,7 @@ public class ProductConfigWrapper implements Serializable {
             configItemAssoc = GenericValue.create(ci.configItemAssoc);
             options = new LinkedList<>();
             for (ConfigOption co: ci.options) {
-                options.add(new ConfigOption(co));
+                options.add(new ConfigOption(co, this));
             }
             first = ci.first;
             content = ci.content; // FIXME: this should be cloned
@@ -815,13 +815,17 @@ public class ProductConfigWrapper implements Serializable {
         }
 
         public ConfigOption(ConfigOption co) {
+            this(co, co.parentConfigItem);
+        }
+
+        private ConfigOption(ConfigOption co, ConfigItem parentConfigItem) {
             configOption = GenericValue.create(co.configOption);
             componentList = new LinkedList<>();
             for (GenericValue component: co.componentList) {
                 componentList.add(GenericValue.create(component));
             }
-            parentConfigItem = co.parentConfigItem;
-            componentOptions = co.componentOptions;
+            this.parentConfigItem = parentConfigItem;
+            componentOptions = co.componentOptions == null ? null : new HashMap<>(co.componentOptions);
             optionListPrice = co.optionListPrice;
             optionPrice = co.optionPrice;
             available = co.available;