From fc408125c87df527e8368138105b14fb5be67873 Mon Sep 17 00:00:00 2001
From: Hung Nguyen <hungnv@google.com>
Date: Thu, 19 Oct 2023 09:35:03 +0100
Subject: [PATCH] Fix MemoizedCallable in PropertiesBuildService

In commit 5ceebec, the MemoizedCallable instance was created inside the
Provider, which means that the MemoizedCallable is not reused the next
time the Provider is resolved. This was not the intended effect.

In this commit, we will create the MemoizedCallable instance up front so
that each time the Provider is resolved, it will reuse the same
MemoizedCallable and therefore avoid recomputing the value.

Test: Manually verified on the large project in KT-62496
^KT-62496 Fixed
---
 .../gradle/plugin/PropertiesBuildService.kt   | 24 ++++++++++---------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/PropertiesBuildService.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/PropertiesBuildService.kt
index a68099ecffd..616ee8610b4 100644
--- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/PropertiesBuildService.kt
+++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/PropertiesBuildService.kt
@@ -65,22 +65,24 @@ private class PropertiesManager(private val project: Project, private val localP
     fun property(propertyName: String): Provider<String> {
         // Note: The same property may be read many times (KT-62496). Therefore,
         //   - Use a map to create only one Provider per property.
-        //   - Use MemoizedCallable to resolve the Provider only once
+        //   - Use MemoizedCallable to resolve the Provider only once.
         return properties.computeIfAbsent(propertyName) {
-            project.provider { computeValue(propertyName) }
-        }
-    }
-
-    private fun computeValue(propertyName: String): String? {
-        return project.extraProperties.getOrNull(propertyName) as String? // We don't memoize extraProperties as they can still change
-            ?: MemoizedCallable {
+            // We need to create the MemoizedCallable instance up front so that each time the Provider is resolved, it will reuse the same
+            // MemoizedCallable.
+            val valueFromGradleAndLocalProperties = MemoizedCallable {
                 project.providers.gradleProperty(propertyName).usedAtConfigurationTime(configurationTimePropertiesAccessor).orNull
                     ?: localProperties[propertyName]
-            }.call()
+            }
+            project.provider {
+                // FIXME(KT-62684): We currently don't memoize extraProperties as they may still change, we'll fix this later.
+                project.extraProperties.getOrNull(propertyName) as String?
+                    ?: valueFromGradleAndLocalProperties.call()
+            }
+        }
     }
 }
 
 private class MemoizedCallable<T>(valueResolver: Callable<T>) : Callable<T> {
-    private val value by lazy { valueResolver.call() }
-    override fun call(): T = value
+    private val value: T? by lazy { valueResolver.call() }
+    override fun call(): T? = value
 }