Skip to content

Commit

Permalink
Improved cache
Browse files Browse the repository at this point in the history
  • Loading branch information
@idegtiarenko
idegtiarenko committed Jun 5, 2016
1 parent 60ceecf commit 4c1718f
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 73 deletions.
1 change: 1 addition & 0 deletions resources/META-INF/plugin.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
<li>Better syntax support for concordion expressions in md specs</li>
<li>Groovy test fixture support</li>
<li>Scala test fixture support</li>
<li>Improved cache</li>
</ul>
Version 0.8<br/>
<ul>
Expand Down
60 changes: 33 additions & 27 deletions src/org/concordion/plugin/idea/PsiElementCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,43 +4,40 @@
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.lang.ref.WeakReference;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.WeakHashMap;
import java.util.function.Function;
import java.util.function.Supplier;

public class PsiElementCache<T extends PsiElement> {

private final Function<T, String> identityFunction;
private final Map<String, CachedPsiElement<T>> cache;
@NotNull private final Function<T, String> identityFunction;
@NotNull private final Map<String, CachedPsiElement> cache = new HashMap<>();

public PsiElementCache(@NotNull Function<T, String> identityFunction) {
this(identityFunction, new WeakHashMap<>());
}

public PsiElementCache(@NotNull Function<T, String> identityFunction, @NotNull Map<String, CachedPsiElement<T>> cache) {
this.identityFunction = identityFunction;
this.cache = cache;
}

public void put(@NotNull String key, @Nullable T value) {
if (value != null) {
cache.put(key, new CachedPsiElement<>(identityFunction, value));
public void put(@NotNull String key, @Nullable T element) {
if (element != null) {
cache.put(key, new CachedPsiElement(element));
}
}

@Nullable
public T get(@NotNull String key) {
CachedPsiElement<T> cached = cache.get(key);
CachedPsiElement cached = cache.get(key);
if (cached == null) {
return null;
}
if (!cached.isValid()) {
T t = cached.getIfValid();
if (t == null) {
cache.remove(key);
return null;
}
return cached.get();
return t;
}

@Nullable
Expand All @@ -53,24 +50,33 @@ public T getOrCompute(@NotNull String key, @NotNull Supplier<T> supplier) {
return t;
}

protected static class CachedPsiElement<T extends PsiElement> {
public void clear() {
cache.clear();
}

public int size() {
return cache.size();
}

private final Function<T, String> identityFunction;
private final T element;
private final String identity;
private class CachedPsiElement {

public CachedPsiElement(Function<T, String> identityFunction, T element) {
this.identityFunction = identityFunction;
this.element = element;
this.identity = identityFunction.apply(element);
}
@NotNull private final WeakReference<T> element;
@Nullable private final String originalIdentity;

public boolean isValid() {
return Objects.equals(identity, identityFunction.apply(element)) && element.isValid();
public CachedPsiElement(@NotNull T element) {
this.element = new WeakReference<>(element);
this.originalIdentity = identityFunction.apply(element);
}

public T get() {
return element;
@Nullable
public T getIfValid() {
T elementRef = element.get();
if (elementRef == null
|| !Objects.equals(originalIdentity, identityFunction.apply(elementRef))
|| !elementRef.isValid()) {
return null;
}
return elementRef;
}
}
}
9 changes: 5 additions & 4 deletions src/org/concordion/plugin/idea/lang/psi/AbstractConcordionMember.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,20 @@

public abstract class AbstractConcordionMember extends AbstractConcordionPsiElement implements ConcordionMember {

protected PsiElementCache<PsiClass> containingClass = new PsiElementCache<>(PsiClass::getQualifiedName);
protected PsiElementCache<PsiMember> containingMember = new PsiElementCache<>(ConcordionPsiUtils::memberIdentity);
protected final PsiElementCache<PsiClass> containingClass = new PsiElementCache<>(PsiClass::getQualifiedName);
protected final PsiElementCache<PsiMember> containingMember = new PsiElementCache<>(ConcordionPsiUtils::memberIdentity);

public AbstractConcordionMember(@NotNull ASTNode node) {
super(node);
}

@Nullable
@Override
public PsiClass getContainingClass() {
return containingClass.getOrCompute(nullToEmpty(getName()), this::determineContainingClass);
}

@Nullable
@Override
public PsiMember getContainingMember() {
return containingMember.getOrCompute(nullToEmpty(getName()), this::determineContainingMember);
Expand All @@ -34,8 +36,7 @@ public PsiMember getContainingMember() {
@Nullable
protected PsiClass determineContainingClass() {
if (getParent() instanceof ConcordionOgnlExpressionStart) {
PsiFile htmlRunner = getTopLevelFile(this);
return ConcordionNavigationService.getInstance(getProject()).correspondingTestFixture(htmlRunner);
return ConcordionNavigationService.getInstance(getProject()).correspondingTestFixture(getTopLevelFile(this));
} else {
ConcordionPsiElement parent = getConcordionParent();
if (parent == null) {
Expand Down
3 changes: 0 additions & 3 deletions src/org/concordion/plugin/idea/lang/psi/AbstractConcordionPsiElement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@

public abstract class AbstractConcordionPsiElement extends ASTWrapperPsiElement implements ConcordionPsiElement {

//TODO find a good way to cache (no outdated, ok with renaming), use PsiCachedValuesFactory?
protected PsiType type;

public AbstractConcordionPsiElement(@NotNull ASTNode node) {
super(node);
}
Expand Down
72 changes: 33 additions & 39 deletions tests/org/concordion/plugin/idea/PsiElementCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,83 +2,77 @@

import com.intellij.psi.PsiClass;
import junit.framework.TestCase;
import org.concordion.plugin.idea.PsiElementCache;

import java.util.HashMap;
import java.util.Map;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class PsiElementCacheTest extends TestCase {

private final Map<String, PsiElementCache.CachedPsiElement<PsiClass>> spy = new HashMap<>();
private final PsiElementCache<PsiClass> testingInstance = new PsiElementCache<>(PsiClass::getName, spy);

private final String notAddedKey = "notAddedKey";
private final String notAddedName = "notAddedName";
private PsiClass notAddedPsiElement;
private final PsiElementCache<PsiClass> cache = new PsiElementCache<>(PsiClass::getName);

private final String testKey = "testKey";
private final String testName = "testName";
private PsiClass testPsiElement;
private PsiClass notInCacheElement;
private PsiClass inCacheElement;

@Override
protected void setUp() throws Exception {
notAddedPsiElement = mock(PsiClass.class);
when(notAddedPsiElement.getName()).thenReturn(notAddedName);
when(notAddedPsiElement.isValid()).thenReturn(true);

testPsiElement = mock(PsiClass.class);
when(testPsiElement.getName()).thenReturn(testName);
when(testPsiElement.isValid()).thenReturn(true);
testingInstance.put(testKey, testPsiElement);
notInCacheElement = mock(PsiClass.class);
when(notInCacheElement.getName()).thenReturn("notInCacheElement");
when(notInCacheElement.isValid()).thenReturn(true);

inCacheElement = mock(PsiClass.class);
when(inCacheElement.getName()).thenReturn("inCacheElement");
when(inCacheElement.isValid()).thenReturn(true);
//WeakReference in cache will not lose this element as it is stored in inCacheElement link and in mockito internals
cache.put("inCache", inCacheElement);
}

@Override
public void tearDown() {
spy.clear();
cache.clear();
}

public void testMissingElementShouldBeNull() {
assertThat(testingInstance.get(notAddedKey)).isNull();
assertThat(cache.get("notInCache")).isNull();
}

public void testAddedElementShouldBeCached() {
assertThat(testingInstance.get(testKey)).isNotNull().isSameAs(testPsiElement);
assertThat(cache.get("inCache")).isNotNull().isSameAs(inCacheElement);
}

public void testAddedNullShouldNotBeCached() {
int initialSize = spy.size();
testingInstance.put(notAddedKey, null);
assertThat(spy.size()).isEqualTo(initialSize);
int initialSize = cache.size();
cache.put("notInCache", null);
assertThat(cache.size()).isEqualTo(initialSize);
}

public void testRenamedElementShouldBeEvicted() {
when(testPsiElement.getName()).thenReturn("newTestName");
assertThat(testingInstance.get(testKey)).isNull();
assertThat(spy.get(testKey)).isNull();
int initialSize = cache.size();
when(inCacheElement.getName()).thenReturn("newInCacheElement");
assertThat(cache.get("inCache")).isNull();
assertThat(cache.size()).isEqualTo(initialSize - 1);
}

public void testInvalidElementShouldBeEvicted() {
when(testPsiElement.isValid()).thenReturn(false);
assertThat(testingInstance.get(testKey)).isNull();
assertThat(spy.get(testKey)).isNull();
int initialSize = cache.size();
when(inCacheElement.isValid()).thenReturn(false);
assertThat(cache.get("inCache")).isNull();
assertThat(cache.size()).isEqualTo(initialSize - 1);
}

public void testNotUsingSupplierIfAlreadyCached() {
assertThat(testingInstance.getOrCompute(testKey, () -> {
assertThat(cache.getOrCompute("inCache", () -> {
throw new RuntimeException("should not be called");
})).isSameAs(testPsiElement);
})).isSameAs(inCacheElement);
}

public void testComputeIfElementIsAbsent() {
assertThat(testingInstance.getOrCompute(notAddedKey, () -> notAddedPsiElement)).isSameAs(notAddedPsiElement);
assertThat(cache.getOrCompute("notInCache", () -> notInCacheElement)).isSameAs(notInCacheElement);
}

public void testComputedValueIsAddedToCache() {
testingInstance.getOrCompute(notAddedKey, () -> notAddedPsiElement);
assertThat(testingInstance.get(notAddedKey)).isSameAs(notAddedPsiElement);
int initialSize = cache.size();
cache.getOrCompute("notInCache", () -> notInCacheElement);
assertThat(cache.size()).isEqualTo(initialSize + 1);
}
}
}

0 comments on commit 4c1718f

Please sign in to comment.