Fix overlapping data issues in stateful reviews
Previously, when reviewing multiple projects in stateful mode, ChatGPT
assistant data retrieval for different projects would overwrite each
other. To address this, `PluginDataHandlerProvider` has been decoupled
from `PluginDataHandlerBaseProvider` and now supports multiple project
scopes. Additionally, a new 'project' scope has been introduced to
separately manage persistent data for each project.
Change-Id: I1279f86c181109640498e286e04d20a8b71fcb25
Signed-off-by: Patrizio <patrizio.gelosi@amarulasolutions.com>
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/data/PluginDataHandlerBaseProvider.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/data/PluginDataHandlerBaseProvider.java
new file mode 100644
index 0000000..15344a9
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/data/PluginDataHandlerBaseProvider.java
@@ -0,0 +1,29 @@
+package com.googlesource.gerrit.plugins.chatgpt.data;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+import java.nio.file.Path;
+
+@Singleton
+public class PluginDataHandlerBaseProvider implements Provider<PluginDataHandler> {
+ private static final String PATH_SUFFIX = ".data";
+ private static final String PATH_GLOBAL = "global";
+
+ private final Path defaultPluginDataPath;
+
+ @Inject
+ public PluginDataHandlerBaseProvider(@com.google.gerrit.extensions.annotations.PluginData Path defaultPluginDataPath) {
+ this.defaultPluginDataPath = defaultPluginDataPath;
+ }
+
+ public PluginDataHandler get(String path) {
+ return new PluginDataHandler(defaultPluginDataPath.resolve(path + PATH_SUFFIX));
+ }
+
+ @Override
+ public PluginDataHandler get() {
+ return get(PATH_GLOBAL);
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/data/PluginDataHandlerProvider.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/data/PluginDataHandlerProvider.java
index f7366ca..ca52d51 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/data/PluginDataHandlerProvider.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/data/PluginDataHandlerProvider.java
@@ -3,24 +3,28 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritChange;
import java.nio.file.Path;
@Singleton
-public class PluginDataHandlerProvider implements Provider<PluginDataHandler> {
- private final Path defaultPluginDataPath;
+public class PluginDataHandlerProvider extends PluginDataHandlerBaseProvider implements Provider<PluginDataHandler> {
+ private final String projectName;
@Inject
- public PluginDataHandlerProvider(@com.google.gerrit.extensions.annotations.PluginData Path defaultPluginDataPath) {
- this.defaultPluginDataPath = defaultPluginDataPath;
+ public PluginDataHandlerProvider(
+ @com.google.gerrit.extensions.annotations.PluginData Path defaultPluginDataPath,
+ GerritChange change
+ ) {
+ super(defaultPluginDataPath);
+ projectName = change.getProjectName();
}
- public PluginDataHandler get(Path configPath) {
- return new PluginDataHandler(configPath);
+ public PluginDataHandler getGlobalScope() {
+ return super.get();
}
- @Override
- public PluginDataHandler get() {
- return get(defaultPluginDataPath.resolve("plugin.config"));
+ public PluginDataHandler getProjectScope() {
+ return super.get(projectName);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateful/client/api/chatgpt/ChatGptAssistant.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateful/client/api/chatgpt/ChatGptAssistant.java
index 7fc146c..e2548d5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateful/client/api/chatgpt/ChatGptAssistant.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateful/client/api/chatgpt/ChatGptAssistant.java
@@ -29,7 +29,7 @@
private final ChatGptHttpClient httpClient = new ChatGptHttpClient();
private final GerritChange change;
private final GitRepoFiles gitRepoFiles;
- private final PluginDataHandler pluginDataHandler;
+ private final PluginDataHandler projectDataHandler;
public ChatGptAssistant(
Configuration config,
@@ -40,16 +40,16 @@
super(config);
this.change = change;
this.gitRepoFiles = gitRepoFiles;
- this.pluginDataHandler = pluginDataHandlerProvider.get();
+ this.projectDataHandler = pluginDataHandlerProvider.getProjectScope();
}
public void setupAssistant() {
- String assistantId = pluginDataHandler.getValue(KEY_ASSISTANT_ID);
+ String assistantId = projectDataHandler.getValue(KEY_ASSISTANT_ID);
if (assistantId == null || config.getForceCreateAssistant()) {
String fileId = uploadRepoFiles();
- pluginDataHandler.setValue(KEY_FILE_ID, fileId);
+ projectDataHandler.setValue(KEY_FILE_ID, fileId);
assistantId = createAssistant(fileId);
- pluginDataHandler.setValue(KEY_ASSISTANT_ID, assistantId);
+ projectDataHandler.setValue(KEY_ASSISTANT_ID, assistantId);
log.info("Project assistant created with ID: {}", assistantId);
}
else {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateful/client/api/chatgpt/ChatGptRun.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateful/client/api/chatgpt/ChatGptRun.java
index 466afa1..d078e67 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateful/client/api/chatgpt/ChatGptRun.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateful/client/api/chatgpt/ChatGptRun.java
@@ -28,7 +28,7 @@
private final ChatGptHttpClient httpClient = new ChatGptHttpClient();
private final String threadId;
- private final PluginDataHandler pluginDataHandler;
+ private final PluginDataHandler projectDataHandler;
private ChatGptResponse runResponse;
private ChatGptListResponse stepResponse;
@@ -36,7 +36,7 @@
public ChatGptRun(String threadId, Configuration config, PluginDataHandlerProvider pluginDataHandlerProvider) {
super(config);
this.threadId = threadId;
- this.pluginDataHandler = pluginDataHandlerProvider.get();
+ this.projectDataHandler = pluginDataHandlerProvider.getProjectScope();
}
public void createRun() {
@@ -81,7 +81,7 @@
log.debug("ChatGPT Create Run request URI: {}", uri);
ChatGptCreateRunRequest requestBody = ChatGptCreateRunRequest.builder()
- .assistantId(pluginDataHandler.getValue(KEY_ASSISTANT_ID))
+ .assistantId(projectDataHandler.getValue(KEY_ASSISTANT_ID))
.build();
return httpClient.createRequestFromJson(uri.toString(), config.getGptToken(), requestBody);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatefulTest.java b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatefulTest.java
index 02f16c5..267f968 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatefulTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatefulTest.java
@@ -66,7 +66,7 @@
.thenReturn(MODES.stateful.name());
// Mock the pluginDataHandlerProvider to return the mocked pluginDataHandler
- when(pluginDataHandlerProvider.get()).thenReturn(pluginDataHandler);
+ when(pluginDataHandlerProvider.getProjectScope()).thenReturn(pluginDataHandler);
}
protected void initConfig() {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java
index 393a87f..b91fddc 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java
@@ -2,9 +2,6 @@
import com.github.tomakehurst.wiremock.junit.WireMockRule;
import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.BranchNameKey;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.Project;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.extensions.api.GerritApi;
@@ -36,7 +33,6 @@
import com.googlesource.gerrit.plugins.chatgpt.data.PluginDataHandlerProvider;
import com.googlesource.gerrit.plugins.chatgpt.listener.EventHandlerTask;
import com.googlesource.gerrit.plugins.chatgpt.localization.Localizer;
-import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritChange;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritClient;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritClientComments;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritClientFacade;
@@ -76,7 +72,7 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
-public class ChatGptReviewTestBase {
+public class ChatGptReviewTestBase extends ChatGptTestBase {
protected static final Path basePath = Paths.get("src/test/resources");
protected static final int GERRIT_GPT_ACCOUNT_ID = 1000000;
protected static final String GERRIT_GPT_USERNAME = "gpt";
@@ -87,9 +83,6 @@
protected static final String GERRIT_USER_GROUP = "Test";
protected static final String GPT_TOKEN = "tk-test";
protected static final String GPT_DOMAIN = "http://localhost:9527";
- protected static final Project.NameKey PROJECT_NAME = Project.NameKey.parse("myProject");
- protected static final Change.Key CHANGE_ID = Change.Key.parse("myChangeId");
- protected static final BranchNameKey BRANCH_NAME = BranchNameKey.create(PROJECT_NAME, "myBranchName");
protected static final boolean GPT_STREAM_OUTPUT = true;
protected static final long TEST_TIMESTAMP = 1699270812;
private static final int GPT_USER_ACCOUNT_ID = 1000000;
@@ -137,10 +130,6 @@
initTest();
}
- protected GerritChange getGerritChange() {
- return new GerritChange(PROJECT_NAME, BRANCH_NAME, CHANGE_ID);
- }
-
protected void initGlobalAndProjectConfig() {
globalConfig = mock(PluginConfig.class);
Answer<Object> returnDefaultArgument = invocation -> {
@@ -161,7 +150,6 @@
.thenReturn(GPT_DOMAIN);
when(globalConfig.getString("gerritUserName")).thenReturn(GERRIT_GPT_USERNAME);
-
projectConfig = mock(PluginConfig.class);
// Mock the Project Config values
diff --git a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptTestBase.java b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptTestBase.java
new file mode 100644
index 0000000..b3488b4
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptTestBase.java
@@ -0,0 +1,16 @@
+package com.googlesource.gerrit.plugins.chatgpt;
+
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.Project;
+import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritChange;
+
+public class ChatGptTestBase {
+ protected static final Project.NameKey PROJECT_NAME = Project.NameKey.parse("myProject");
+ protected static final Change.Key CHANGE_ID = Change.Key.parse("myChangeId");
+ protected static final BranchNameKey BRANCH_NAME = BranchNameKey.create(PROJECT_NAME, "myBranchName");
+
+ protected GerritChange getGerritChange() {
+ return new GerritChange(ChatGptTestBase.PROJECT_NAME, ChatGptTestBase.BRANCH_NAME, ChatGptTestBase.CHANGE_ID);
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/PluginDataTest.java b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/PluginDataTest.java
index 5bdc08f..d3b687a 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/PluginDataTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/PluginDataTest.java
@@ -17,7 +17,7 @@
import org.mockito.junit.MockitoJUnitRunner;
@RunWith(MockitoJUnitRunner.class)
-public class PluginDataTest {
+public class PluginDataTest extends ChatGptTestBase {
@Rule
public TemporaryFolder tempFolder = new TemporaryFolder();
@@ -29,31 +29,36 @@
@Before
public void setUp() {
// Setup temporary folder for tests
- realPluginDataPath = tempFolder.getRoot().toPath().resolve("plugin.config");
+ realPluginDataPath = tempFolder.getRoot().toPath().resolve("global.data");
+ Path realProjectDataPath = tempFolder.getRoot().toPath().resolve(PROJECT_NAME + ".data");
// Mock the PluginData annotation behavior
- when(mockPluginDataPath.resolve("plugin.config")).thenReturn(realPluginDataPath);
+ when(mockPluginDataPath.resolve("global.data")).thenReturn(realPluginDataPath);
+ when(mockPluginDataPath.resolve(PROJECT_NAME + ".data")).thenReturn(realProjectDataPath);
}
@Test
public void testValueSetAndGet() {
- PluginDataHandlerProvider provider = new PluginDataHandlerProvider(mockPluginDataPath);
- PluginDataHandler handler = provider.get();
+ PluginDataHandlerProvider provider = new PluginDataHandlerProvider(mockPluginDataPath, getGerritChange());
+ PluginDataHandler globalHandler = provider.getGlobalScope();
+ PluginDataHandler projectHandler = provider.getProjectScope();
String key = "testKey";
String value = "testValue";
// Test set value
- handler.setValue(key, value);
+ globalHandler.setValue(key, value);
+ projectHandler.setValue(key, value);
// Test get value
- assertEquals("The value retrieved should match the value set.", value, handler.getValue(key));
+ assertEquals("The value retrieved should match the value set.", value, globalHandler.getValue(key));
+ assertEquals("The value retrieved should match the value set.", value, projectHandler.getValue(key));
}
@Test
public void testRemoveValue() {
- PluginDataHandlerProvider provider = new PluginDataHandlerProvider(mockPluginDataPath);
- PluginDataHandler handler = provider.get();
+ PluginDataHandlerProvider provider = new PluginDataHandlerProvider(mockPluginDataPath, getGerritChange());
+ PluginDataHandler handler = provider.getGlobalScope();
String key = "testKey";
String value = "testValue";
@@ -72,8 +77,8 @@
// Ensure the file doesn't exist before creating the handler
Files.deleteIfExists(realPluginDataPath);
- PluginDataHandlerProvider provider = new PluginDataHandlerProvider(mockPluginDataPath);
- provider.get();
+ PluginDataHandlerProvider provider = new PluginDataHandlerProvider(mockPluginDataPath, getGerritChange());
+ provider.getGlobalScope();
// The constructor should create the file if it doesn't exist
assertTrue("The config file should exist after initializing the handler.", Files.exists(realPluginDataPath));
