Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions modules/AcShop/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,18 @@
<type>jar</type>
<version>1.0.0</version>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.13.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>5.11.0</version>
Comment on lines +97 to +98
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mockito-core 5.11.0 requires a newer Java runtime than this module targets (<java.version>1.8</java.version>), so the build/tests can fail under Java 8. Also, BenchmarkTest uses Mockito.mockStatic(...), which requires the inline mock maker (typically org.mockito:mockito-inline) rather than plain mockito-core. Please align Mockito artifacts/versions with Java 8 (e.g., a Mockito 4.x line) and include the correct artifact for static mocking, or refactor the test to avoid static mocks.

Suggested change
<artifactId>mockito-core</artifactId>
<version>5.11.0</version>
<artifactId>mockito-inline</artifactId>
<version>4.11.0</version>

Copilot uses AI. Check for mistakes.
<scope>test</scope>
</dependency>
</dependencies>

</project>
24 changes: 18 additions & 6 deletions modules/AcShop/src/main/java/com/mcatk/acshop/FileOperation.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,35 @@

public class FileOperation {

private static final Gson gson = new GsonBuilder().setPrettyPrinting().create();
private final File shopsFile;
private final Gson gson;

public FileOperation() {
shopsFile = new File(AcShop.getPlugin().getDataFolder(), "shopsFile.json");
gson = new GsonBuilder().setPrettyPrinting().create();
this(AcShop.getPlugin().getDataFolder());
}

public FileOperation(File dataFolder) {
this.shopsFile = new File(dataFolder, "shopsFile.json");
}

public void saveShops(Shops shops) {
try {
FileWriter writer = new FileWriter(shopsFile);
try (FileWriter writer = new FileWriter(shopsFile)) {
gson.toJson(shops, writer);
writer.flush();
} catch (IOException e) {
e.printStackTrace();
}
}

public void saveShopsAsync(Shops shops) {
final String json = gson.toJson(shops);
org.bukkit.Bukkit.getScheduler().runTaskAsynchronously(AcShop.getPlugin(), () -> {
try (FileWriter writer = new FileWriter(shopsFile)) {
writer.write(json);
Comment on lines +33 to +37

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Prevent concurrent async writes to shops file

Because saveShopsAsync schedules a new async task on every call without any locking or sequencing, multiple admin commands in quick succession can run these writes concurrently. Bukkit’s async scheduler can execute tasks in parallel, so two writers can interleave and leave shopsFile.json partially written or with mixed content (last write not guaranteed to be the only writer). This is a regression from the prior synchronous write path. Consider serializing writes (e.g., a single-thread executor/queue) or writing to a temp file and atomically replacing.

Useful? React with 👍 / 👎.

} catch (IOException e) {
e.printStackTrace();
}
});
Comment on lines +33 to +41
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saveShopsAsync can schedule multiple async writes to the same shopsFile concurrently. Bukkit async tasks may run out-of-order and/or in parallel, which can lead to older snapshots overwriting newer ones or even file corruption if writes overlap. Consider serializing writes (single-flight/queue) and/or debouncing so only the latest snapshot is written; e.g., gate with a sequence number and skip stale tasks, or chain writes on a single-thread executor, or synchronize and write to a temp file then atomic move.

Copilot uses AI. Check for mistakes.
}

public Shops loadShops() {
Shops shops = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public boolean onCommand(CommandSender sender, Command command, String label, St
break;
default:
}
new FileOperation().saveShops(AcShop.getShops());
new FileOperation().saveShopsAsync(AcShop.getShops());
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The admin command now always triggers saveShopsAsync(...) even for reload and for unknown subcommands. This still does full JSON serialization on the main thread and schedules disk I/O tasks even when no data was modified, increasing background task load and amplifying the risk of overlapping saves. Consider only saving after mutating subcommands (e.g., add, setcmd) and returning early for reload/default cases.

Copilot uses AI. Check for mistakes.
return true;
}

Expand Down
70 changes: 70 additions & 0 deletions modules/AcShop/src/test/java/com/mcatk/acshop/BenchmarkTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package com.mcatk.acshop;

import com.mcatk.acshop.commodity.Item;
import com.mcatk.acshop.commodity.ItemType;
import com.mcatk.acshop.shop.Shop;
import com.mcatk.acshop.shop.Shops;
import org.bukkit.Bukkit;
import org.bukkit.scheduler.BukkitScheduler;
import org.bukkit.plugin.Plugin;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

@RunWith(MockitoJUnitRunner.class)
public class BenchmarkTest {

@Test
public void benchmarkSaveShops() throws IOException {
// Setup data
Shops shops = new Shops();
for (int i = 0; i < 50; i++) {
Shop shop = new Shop("Shop" + i);
for (int j = 0; j < 50; j++) {
shop.getItemHashMap().put("Item" + j, new Item(ItemType.ITEM_STACK, "Item" + j, 100, "sort", "id"));
}
shops.getShopsHashMap().put(shop.getId(), shop);
}

// Mock AcShop
File tempDir = Files.createTempDirectory("acshop_test").toFile();
tempDir.deleteOnExit();

AcShop mockPlugin = mock(AcShop.class);
when(mockPlugin.getDataFolder()).thenReturn(tempDir);

BukkitScheduler mockScheduler = mock(BukkitScheduler.class);
when(mockScheduler.runTaskAsynchronously(any(Plugin.class), any(Runnable.class))).thenReturn(null);
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runTaskAsynchronously(...) is stubbed to just return null without executing the submitted Runnable, so this benchmark never performs the actual file write. The measured time is therefore JSON serialization + scheduling overhead only, not end-to-end save-to-disk behavior. Consider capturing/executing the Runnable (and/or asserting file contents) or adjusting the benchmark name/output to make this scope explicit.

Suggested change
when(mockScheduler.runTaskAsynchronously(any(Plugin.class), any(Runnable.class))).thenReturn(null);
when(mockScheduler.runTaskAsynchronously(any(Plugin.class), any(Runnable.class))).thenAnswer(invocation -> {
Runnable task = invocation.getArgument(1);
task.run();
return null;
});

Copilot uses AI. Check for mistakes.

try (MockedStatic<AcShop> mockedAcShop = Mockito.mockStatic(AcShop.class);
MockedStatic<Bukkit> mockedBukkit = Mockito.mockStatic(Bukkit.class)) {

mockedAcShop.when(AcShop::getPlugin).thenReturn(mockPlugin);
mockedBukkit.when(Bukkit::getScheduler).thenReturn(mockScheduler);

// Warmup
new FileOperation().saveShopsAsync(shops);

// Benchmark
long startTime = System.nanoTime();
int iterations = 20;
for (int i = 0; i < iterations; i++) {
new FileOperation().saveShopsAsync(shops);
}
long endTime = System.nanoTime();
double avgTime = (endTime - startTime) / (double) iterations / 1_000_000.0;
System.out.println("Average save time (Async offload): " + avgTime + " ms");
}
}
}
Loading