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
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,18 @@
@ApplicationScoped
public class MapCache {

// simple - but enough - protection against too much entries in the cache
// this mainly targets ?query king of parameters
// simple - but enough - protection against too many entries in the cache
// this mainly targets ?query kind of parameters
public <A, B> void evictIfNeeded(final ConcurrentMap<A, B> cache, final int maxSize) {
if (maxSize < 0) {
if (maxSize <= 0) {
cache.clear();
return;
}
while (cache.size() > maxSize) {
final Iterator<Map.Entry<A, B>> iterator = cache.entrySet().iterator();
if (iterator.hasNext()) {
iterator.remove();
}
final Iterator<A> it = cache.keySet().iterator();
int excess = cache.size() - maxSize;
while (excess-- > 0 && it.hasNext()) {
it.next();
it.remove();
Comment on lines +34 to +38
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

java.util.Map is still imported in this class, but after switching to a keySet() iterator the type is no longer referenced. In Java, unused imports are compilation errors, so this will break the build; please remove the unused Map import (and any other now-unused imports).

Copilot uses AI. Check for mistakes.
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/**
* Copyright (C) 2006-2026 Talend Inc. - www.talend.com
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.talend.sdk.component.server.lang;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.stream.Stream;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

public class MapCacheTest {

@ParameterizedTest
@MethodSource("emptyCacheTestCases")
void shouldEmptyCache(final ConcurrentMap<String, Object> cache, final int maxSize) {
new MapCache().evictIfNeeded(cache, maxSize);
assertTrue(cache.isEmpty());
}

public static Stream<Arguments> emptyCacheTestCases() {
return Stream.of(
Arguments.of(new ConcurrentHashMap<>(Map.of("1", "2")), 0),
Arguments.of(new ConcurrentHashMap<>(Map.of("1", "2")), -1),
Arguments.of(new ConcurrentHashMap<>(Map.of("1", "2", "3", "4")), 0)
);
}

@ParameterizedTest
@MethodSource("updatedCacheTestCases")
void shouldKeepTheExpectedNumberOfItems(final ConcurrentMap<String, Object> cache, final int maxSize) {
new MapCache().evictIfNeeded(cache, maxSize);
assertEquals(maxSize, cache.size());
}

public static Stream<Arguments> updatedCacheTestCases() {
return Stream.of(
Arguments.of(new ConcurrentHashMap<>(Map.of("1", "2", "3", "4", "5", "6")), 1),
Arguments.of(new ConcurrentHashMap<>(Map.of("1", "2", "3", "4", "5", "6")), 2)
);
Comment on lines +55 to +58
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

Map.of("1","2","3","4","5","6") creates a map with 3 entries (pairs), not 6. This makes the eviction test much weaker than intended and can mask regressions; please build a map with 6 entries (e.g., Map.ofEntries(...) or explicit puts) so the starting size is actually > maxSize by a larger margin.

Copilot uses AI. Check for mistakes.
}

@ParameterizedTest
@MethodSource("unmodifiedCacheTestCases")
void shouldNotUpdateTheCache(final ConcurrentMap<String, Object> cache, final int maxSize) {
int initialSize = cache.size();
new MapCache().evictIfNeeded(cache, maxSize);
assertEquals(initialSize, cache.size());
}

public static Stream<Arguments> unmodifiedCacheTestCases() {
return Stream.of(
Arguments.of(new ConcurrentHashMap<>(Map.of("1", "2", "3", "4", "5", "6")), 3),
Arguments.of(new ConcurrentHashMap<>(Map.of("1", "2", "3", "4", "5", "6")), 4)
);
Comment on lines +70 to +73
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

Same issue here: Map.of("1","2","3","4","5","6") only creates 3 entries. If the intent is to validate that the cache is not modified when maxSize is comfortably above the current size, consider using a map with 6 actual entries and choosing maxSize accordingly to make this test assert what its name suggests.

Copilot uses AI. Check for mistakes.
}
}
Loading