⚡ Optimize Guild Loading N+1 Query

[acsoto]

💡 What:
Optimized JdbcGuildRepository.getAllGuilds() to fetch all members in a single query (SELECT * FROM player_guild) instead of executing a separate query for each guild inside a loop.

🎯 Why:
The previous implementation suffered from the N+1 query problem. If there were N guilds, it executed N+1 database queries. This causes significant latency as the number of guilds grows.

📊 Measured Improvement:

• Baseline: Validated N+1 behavior with a new test case (JdbcGuildRepositoryTest). For 5 guilds, it executed 6 prepareStatement calls.
• Optimized: The same test case now executes exactly 2 prepareStatement calls (1 for guilds, 1 for members) regardless of the number of guilds.
• Verified data correctness: Members are correctly assigned to their respective guilds in the resulting map.

Other Changes:

• Fixed a compilation error in GuildCommandS.java (missing GuildManager import) to allow the project to compile for testing.
• Updated pom.xml to include junit and mockito-core for testing.
• Updated pom.xml to use com.github.PlaceholderAPI:PlaceholderAPI (Jitpack) instead of me.clip:placeholderapi to resolve build dependency errors.

---

PR created automatically by Jules for task 12318912467186978337 started by @acsoto

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[Copilot]

Pull request overview

This PR optimizes the JdbcGuildRepository.getAllGuilds() method to eliminate the N+1 query problem by fetching all guild members in a single query instead of executing a separate query for each guild.

Changes:

• Replaced the N+1 query pattern in getAllGuilds() with a single batch query to fetch all members
• Added a new SQL command GET_ALL_MEMBERS to support the optimization
• Added test coverage to verify the optimization reduces database calls from N+1 to 2
• Fixed a missing import in GuildCommandS.java
• Updated dependencies: added JUnit and Mockito for testing, switched PlaceholderAPI to JitPack repository

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
JdbcGuildRepository.java	Removed N+1 query pattern by fetching all members in a single batch query and distributing them to guilds
SqlCommand.java	Added GET_ALL_MEMBERS command for the batch member query
JdbcGuildRepositoryTest.java	Added test to verify optimization reduces prepareStatement calls from N+1 to 2
GuildCommandS.java	Added missing GuildManager import required for compilation
pom.xml	Added test dependencies (JUnit, Mockito) and updated PlaceholderAPI to use JitPack

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The test only mocks the guild_id field from the ResultSet, but the actual getAllGuilds() method retrieves multiple fields for each guild (guild_name, guild_chairman, guild_vice_chairman_1, guild_vice_chairman_2, guild_level, guild_points, guild_cash, guild_has_residence, guild_has_changed_name). This will cause NullPointerExceptions or default values when those getters are called on the ResultSet. You should add mock return values for all these fields to ensure the test accurately reflects the actual behavior.

Suggested change

	when(mockResultSetGuilds.getString("guild_id")).thenReturn("g1", "g2", "g3", "g4", "g5");
	when(mockResultSetGuilds.getString("guild_id")).thenReturn("g1", "g2", "g3", "g4", "g5");
	when(mockResultSetGuilds.getString("guild_name")).thenReturn("Guild1", "Guild2", "Guild3", "Guild4", "Guild5");
	when(mockResultSetGuilds.getString("guild_chairman")).thenReturn("Chairman1", "Chairman2", "Chairman3", "Chairman4", "Chairman5");
	when(mockResultSetGuilds.getString("guild_vice_chairman_1")).thenReturn("Vice1_1", "Vice1_2", "Vice1_3", "Vice1_4", "Vice1_5");
	when(mockResultSetGuilds.getString("guild_vice_chairman_2")).thenReturn("Vice2_1", "Vice2_2", "Vice2_3", "Vice2_4", "Vice2_5");
	when(mockResultSetGuilds.getInt("guild_level")).thenReturn(1, 2, 3, 4, 5);
	when(mockResultSetGuilds.getInt("guild_points")).thenReturn(10, 20, 30, 40, 50);
	when(mockResultSetGuilds.getInt("guild_cash")).thenReturn(100, 200, 300, 400, 500);
	when(mockResultSetGuilds.getBoolean("guild_has_residence")).thenReturn(false, false, false, false, false);
	when(mockResultSetGuilds.getBoolean("guild_has_changed_name")).thenReturn(false, false, false, false, false);

[Copilot]

The comment states "Members return nothing for now" but the code immediately below it sets up 2 members with player_id, guild_id, player_contribution, and player_is_advanced values. This comment is misleading and should be updated to reflect the actual test setup, such as "Setup 2 members for guilds g1 and g2".

Suggested change

	// Members return nothing for now
	// Setup 2 members for guilds g1 and g2

[Copilot]

The import statement for Mockito is unused. The test uses static imports for the mock methods (mock, when, verify, times, anyString, contains) from Mockito, but doesn't directly reference the Mockito class itself. Consider removing this unused import.

Suggested change

import org.mockito.Mockito;

[Copilot]

When a member's guild_id doesn't match any existing guild in the guilds map, the member is silently ignored (the null check prevents adding it to any guild). While this is defensive programming, it may hide data integrity issues where members reference non-existent guilds. Consider logging a warning when this occurs to help detect orphaned member records in the database.

Suggested change

	g.getMembers().add(m);
	g.getMembers().add(m);
	} else {
	System.err.println("[JdbcGuildRepository] Orphaned member record: player_id=" + m.getPlayerID()
	+ " references non-existent guild_id=" + m.getGuildID());