feat: Add filter registration option for random teleport locations

[vLuckyyy]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces a flexible filtering system for random teleport locations, allowing external plugins to add custom validation logic via a new RandomTeleportLocationFilter interface and API methods. A critical security concern was identified: the current filter unregistration process allows any plugin to unregister any filter by name, which could be exploited to bypass security constraints. It is recommended to update the API to require the filter instance for unregistration to ensure better isolation and security. Additionally, a circular dependency between RandomTeleportServiceImpl and RandomTeleportSafeLocationService needs to be resolved, and an improvement in the filter registration logic is suggested for robustness.

[gemini-code-assist]

Injecting RandomTeleportServiceImpl here introduces a circular dependency, as RandomTeleportServiceImpl already depends on RandomTeleportSafeLocationService. This is a significant design flaw that can lead to issues with object instantiation and makes the code harder to understand, maintain, and test.

To resolve this, you should break the cycle. A good approach is to pass the required data (the collection of filters) from RandomTeleportServiceImpl to the methods of RandomTeleportSafeLocationService that need it, instead of injecting the entire service.

For example, you could modify getSafeRandomLocation:

// In RandomTeleportSafeLocationService
public CompletableFuture<Location> getSafeRandomLocation(World world, RandomTeleportRadius radius, int attemptCount, Collection<RandomTeleportLocationFilter> filters) {
    // ...
    if (this.isSafeLocation(chunk, generatedLocation, filters)) {
        // ...
    }
    // ...
}

private boolean isSafeLocation(Chunk chunk, Location location, Collection<RandomTeleportLocationFilter> filters) {
    // ...
    for (RandomTeleportLocationFilter filter : filters) {
        if (!filter.isValid(location)) {
            return false;
        }
    }
    return true;
}

Then, RandomTeleportServiceImpl would pass the filters when calling the method:

// In RandomTeleportServiceImpl
@Override
public CompletableFuture<Location> getSafeRandomLocation(World world, int attemptCount) {
    // ...
    return this.safeLocationService.getSafeRandomLocation(world, radius, attemptCount, this.registeredFilters.values());
}

This refactoring would eliminate the circular dependency and also allow you to remove the package-private getRegisteredFiltersInternal() method.

[gemini-code-assist]

The unregisterFilter method allows any caller with access to the RandomTeleportService to remove any registered filter by its name. Since filter names are discoverable via getRegisteredFilters(), a malicious or poorly-behaved plugin can unregister filters registered by other plugins. This can lead to a bypass of critical security checks, such as those preventing teleportation into protected regions or claimed plots. It is recommended to modify the API to require the RandomTeleportLocationFilter instance itself for unregistration, ensuring that only a caller with a reference to the filter object can remove it.

[gemini-code-assist]

The current implementation for checking if a filter is already registered and then adding it is not an atomic operation. While ConcurrentHashMap is thread-safe, using putIfAbsent is a more idiomatic and robust way to perform this check-then-act operation atomically. This makes the code's intent clearer and prevents potential race conditions if multiple threads try to register a filter with the same name concurrently.

Suggested change

	if (this.registeredFilters.containsKey(filterName)) {
	throw new IllegalArgumentException("Filter with name '" + filterName + "' is already registered");
	}
	this.registeredFilters.put(filterName, filter);
	if (this.registeredFilters.putIfAbsent(filterName, filter) != null) {
	throw new IllegalArgumentException("Filter with name '" + filterName + "' is already registered");
	}