Skip to content

Commit

Permalink
catalog: use the tenantRecordId as the catalog cache key
Browse files Browse the repository at this point in the history 
This makes sure the catalog is correctly cached, regardless
of the content of the context object.

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>
  • Loading branch information
@pierre
pierre committed Jan 21, 2015
1 parent 80bd1e9 commit d1a1474
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 57 deletions.
12 changes: 6 additions & 6 deletions catalog/src/main/java/org/killbill/billing/catalog/caching/EhCacheCatalogCache.java
View file
@@ -1,6 +1,6 @@
/*
* Copyright 2014 Groupon, Inc
* Copyright 2014 The Billing Project, LLC
* Copyright 2014-2015 Groupon, Inc
* Copyright 2014-2015 The Billing Project, LLC
*
* The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
Expand Down Expand Up @@ -63,23 +63,23 @@ public VersionedCatalog getCatalog(final InternalTenantContext tenantContext) th
if (tenantContext.getTenantRecordId() == InternalCallContextFactory.INTERNAL_TENANT_RECORD_ID) {
if (defaultCatalog == null) {
throw new CatalogApiException(ErrorCode.CAT_INVALID_DEFAULT,
": the system property org.killbill.catalog.uri must be specified and point to valid catalog xml");
"the system property org.killbill.catalog.uri must be specified and point to valid catalog xml");
}
return defaultCatalog;
}
// The cache loader might choke on some bad xml -- unlikely since we check its validity prior storing it,
// but to be on the safe side;;
try {
final VersionedCatalog tenantCatalog = (VersionedCatalog) cacheController.get(tenantContext, cacheLoaderArgument);
final VersionedCatalog tenantCatalog = (VersionedCatalog) cacheController.get(tenantContext.getTenantRecordId(), cacheLoaderArgument);
return (tenantCatalog != null) ? tenantCatalog : defaultCatalog;
} catch (IllegalStateException e) {
} catch (final IllegalStateException e) {
throw new CatalogApiException(ErrorCode.CAT_INVALID_FOR_TENANT, tenantContext.getTenantRecordId());
}
}

@Override
public void clearCatalog(final InternalTenantContext tenantContext) {
cacheController.remove(tenantContext);
cacheController.remove(tenantContext.getTenantRecordId());
}

//
Expand Down
111 changes: 66 additions & 45 deletions catalog/src/test/java/org/killbill/billing/catalog/caching/TestEhCacheCatalogCache.java
View file
@@ -1,6 +1,6 @@
/*
* Copyright 2014 Groupon, Inc
* Copyright 2014 The Billing Project, LLC
* Copyright 2014-2015 Groupon, Inc
* Copyright 2014-2015 The Billing Project, LLC
*
* The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
Expand All @@ -22,15 +22,19 @@
import java.io.InputStreamReader;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;

import org.killbill.billing.callcontext.InternalCallContext;
import org.killbill.billing.callcontext.InternalTenantContext;
import org.killbill.billing.catalog.CatalogTestSuiteNoDB;
import org.killbill.billing.catalog.DefaultProduct;
import org.killbill.billing.catalog.VersionedCatalog;
import org.killbill.billing.catalog.api.CatalogApiException;
import org.killbill.billing.tenant.api.TenantInternalApi.CacheInvalidationCallback;
import org.killbill.xmlloader.UriAccessor;
import org.mockito.Mockito;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.testng.Assert;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
Expand All @@ -41,23 +45,28 @@

public class TestEhCacheCatalogCache extends CatalogTestSuiteNoDB {

final CacheInvalidationCallback cacheInvalidationCallback = Mockito.mock(CacheInvalidationCallback.class);
private InternalTenantContext multiTenantContext;
private InternalTenantContext otherMultiTenantContext;

@BeforeMethod(groups = "fast")
protected void beforeMethod() throws Exception {
cacheControllerDispatcher.clearAll();

multiTenantContext = Mockito.mock(InternalTenantContext.class);
Mockito.when(multiTenantContext.getAccountRecordId()).thenReturn(456L);
Mockito.when(multiTenantContext.getTenantRecordId()).thenReturn(99L);

otherMultiTenantContext = Mockito.mock(InternalCallContext.class);
Mockito.when(otherMultiTenantContext.getAccountRecordId()).thenReturn(123L);
Mockito.when(otherMultiTenantContext.getTenantRecordId()).thenReturn(112233L);
}

//
// Verify CatalogCache throws CatalogApiException when used in mono-tenant and catalog system property has not been set
//
@Test(groups = "fast", expectedExceptions = CatalogApiException.class)
public void testMissingDefaultCatalog() throws CatalogApiException {

final InternalTenantContext tenantContext = Mockito.mock(InternalTenantContext.class);
Mockito.when(tenantContext.getTenantRecordId()).thenReturn(0L);
catalogCache.loadDefaultCatalog(null);
Mockito.when(tenantInternalApi.getTenantCatalogs(tenantContext)).thenReturn(ImmutableList.<String>of());
catalogCache.getCatalog(internalCallContext);
}

Expand All @@ -66,34 +75,18 @@ public void testMissingDefaultCatalog() throws CatalogApiException {
//
@Test(groups = "fast")
public void testDefaultCatalog() throws CatalogApiException {

final InternalTenantContext tenantContext = Mockito.mock(InternalTenantContext.class);
Mockito.when(tenantContext.getTenantRecordId()).thenReturn(0L);

catalogCache.loadDefaultCatalog(Resources.getResource("SpyCarBasic.xml").toExternalForm());
Mockito.when(tenantInternalApi.getTenantCatalogs(tenantContext)).thenReturn(ImmutableList.<String>of());
VersionedCatalog result = catalogCache.getCatalog(internalCallContext);
Assert.assertNotNull(result);
final DefaultProduct[] products = result.getProducts(clock.getUTCNow());
Assert.assertEquals(products.length, 3);
}

//
// Verify CatalogCache returns default catalog for the (non 0) tenant when its tenant catalog has not been uploaded
//
@Test(groups = "fast")
public void testMissingTenantCatalog() throws CatalogApiException, URISyntaxException, IOException {

catalogCache.loadDefaultCatalog(Resources.getResource("SpyCarBasic.xml").toExternalForm());

final InternalTenantContext tenantContext = Mockito.mock(InternalTenantContext.class);
Mockito.when(tenantContext.getTenantRecordId()).thenReturn(99L);

Mockito.when(tenantInternalApi.getTenantCatalogs(Mockito.any(InternalTenantContext.class))).thenReturn(ImmutableList.<String>of());
VersionedCatalog result = catalogCache.getCatalog(tenantContext);
final VersionedCatalog result = catalogCache.getCatalog(internalCallContext);
Assert.assertNotNull(result);
final DefaultProduct[] products = result.getProducts(clock.getUTCNow());
Assert.assertEquals(products.length, 3);

// Verify the lookup with other contexts
Assert.assertEquals(catalogCache.getCatalog(multiTenantContext), result);
Assert.assertEquals(catalogCache.getCatalog(otherMultiTenantContext), result);
Assert.assertEquals(catalogCache.getCatalog(Mockito.mock(InternalTenantContext.class)), result);
Assert.assertEquals(catalogCache.getCatalog(Mockito.mock(InternalCallContext.class)), result);
}

//
Expand All @@ -104,26 +97,54 @@ public void testMissingTenantCatalog() throws CatalogApiException, URISyntaxExce
//
@Test(groups = "fast")
public void testExistingTenantCatalog() throws CatalogApiException, URISyntaxException, IOException {

final AtomicBoolean shouldThrow = new AtomicBoolean(false);
final Long multiTenantRecordId = multiTenantContext.getTenantRecordId();
catalogCache.loadDefaultCatalog(Resources.getResource("SpyCarBasic.xml").toExternalForm());

final InputStream inputCatalog = UriAccessor.accessUri(new URI(Resources.getResource("SpyCarAdvanced.xml").toExternalForm()));
final String catalogXML = CharStreams.toString(new InputStreamReader(inputCatalog, "UTF-8"));

final InternalTenantContext tenantContext = Mockito.mock(InternalTenantContext.class);
Mockito.when(tenantContext.getTenantRecordId()).thenReturn(156L);

Mockito.when(tenantInternalApi.getTenantCatalogs(Mockito.any(InternalTenantContext.class))).thenReturn(ImmutableList.<String>of(catalogXML));
VersionedCatalog result = catalogCache.getCatalog(tenantContext);
final InputStream tenantInputCatalog = UriAccessor.accessUri(new URI(Resources.getResource("SpyCarAdvanced.xml").toExternalForm()));
final String tenantCatalogXML = CharStreams.toString(new InputStreamReader(tenantInputCatalog, "UTF-8"));
final InputStream otherTenantInputCatalog = UriAccessor.accessUri(new URI(Resources.getResource("SpyCarBasic.xml").toExternalForm()));
final String otherTenantCatalogXML = CharStreams.toString(new InputStreamReader(otherTenantInputCatalog, "UTF-8"));
Mockito.when(tenantInternalApi.getTenantCatalogs(Mockito.any(InternalTenantContext.class))).thenAnswer(new Answer<List<String>>() {
@Override
public List<String> answer(final InvocationOnMock invocation) throws Throwable {
if (shouldThrow.get()) {
throw new RuntimeException();
}
final InternalTenantContext internalContext = (InternalTenantContext) invocation.getArguments()[0];
if (multiTenantRecordId.equals(internalContext.getTenantRecordId())) {
return ImmutableList.<String>of(tenantCatalogXML);
} else {
return ImmutableList.<String>of(otherTenantCatalogXML);
}
}
});

// Verify the lookup for this tenant
final VersionedCatalog result = catalogCache.getCatalog(multiTenantContext);
Assert.assertNotNull(result);
final DefaultProduct[] products = result.getProducts(clock.getUTCNow());
Assert.assertEquals(products.length, 6);

Mockito.when(tenantInternalApi.getTenantCatalogs(tenantContext)).thenThrow(RuntimeException.class);
// Verify the lookup for another tenant
final VersionedCatalog otherResult = catalogCache.getCatalog(otherMultiTenantContext);
Assert.assertNotNull(otherResult);
final DefaultProduct[] otherProducts = otherResult.getProducts(clock.getUTCNow());
Assert.assertEquals(otherProducts.length, 3);

shouldThrow.set(true);

// Verify the lookup for this tenant
final VersionedCatalog result2 = catalogCache.getCatalog(multiTenantContext);
Assert.assertEquals(result2, result);

// Verify the lookup with another context for the same tenant
final InternalCallContext sameMultiTenantContext = Mockito.mock(InternalCallContext.class);
Mockito.when(sameMultiTenantContext.getAccountRecordId()).thenReturn(9102L);
Mockito.when(sameMultiTenantContext.getTenantRecordId()).thenReturn(multiTenantRecordId);
Assert.assertEquals(catalogCache.getCatalog(sameMultiTenantContext), result);

VersionedCatalog result2 = catalogCache.getCatalog(tenantContext);
Assert.assertNotNull(result2);
final DefaultProduct[] products2 = result.getProducts(clock.getUTCNow());
Assert.assertEquals(products2.length, 6);
// Verify the lookup with the other tenant
Assert.assertEquals(catalogCache.getCatalog(otherMultiTenantContext), otherResult);
}
}
13 changes: 7 additions & 6 deletions util/src/main/java/org/killbill/billing/util/cache/TenantCatalogCacheLoader.java
View file
@@ -1,6 +1,6 @@
/*
* Copyright 2014 Groupon, Inc
* Copyright 2014 The Billing Project, LLC
* Copyright 2014-2015 Groupon, Inc
* Copyright 2014-2015 The Billing Project, LLC
*
* The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
Expand All @@ -25,7 +25,6 @@
import org.killbill.billing.callcontext.InternalTenantContext;
import org.killbill.billing.catalog.api.CatalogApiException;
import org.killbill.billing.tenant.api.TenantInternalApi;
import org.killbill.billing.tenant.api.TenantInternalApi.CacheInvalidationCallback;
import org.killbill.billing.util.cache.Cachable.CacheType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -52,14 +51,15 @@ public CacheType getCacheType() {
public Object load(final Object key, final Object argument) {
checkCacheLoaderStatus();

if (!(key instanceof InternalTenantContext)) {
if (!(key instanceof Long)) {
throw new IllegalArgumentException("Unexpected key type of " + key.getClass().getName());
}
if (!(argument instanceof CacheLoaderArgument)) {
throw new IllegalArgumentException("Unexpected argument type of " + argument.getClass().getName());
}

final InternalTenantContext internalTenantContext = (InternalTenantContext) key;
final Long tenantRecordId = (Long) key;
final InternalTenantContext internalTenantContext = new InternalTenantContext(tenantRecordId);
final CacheLoaderArgument cacheLoaderArgument = (CacheLoaderArgument) argument;

if (cacheLoaderArgument.getArgs() == null || !(cacheLoaderArgument.getArgs()[0] instanceof LoaderCallback)) {
Expand All @@ -74,13 +74,14 @@ public Object load(final Object key, final Object argument) {
try {
logger.info("Loading catalog cache for tenant " + internalTenantContext.getTenantRecordId());
return callback.loadCatalog(catalogXMLs);
} catch (CatalogApiException e) {
} catch (final CatalogApiException e) {
throw new IllegalStateException(String.format("Failed to de-serialize catalog for tenant %s : %s",
internalTenantContext.getTenantRecordId(), e.getMessage()), e);
}
}

public interface LoaderCallback {

public Object loadCatalog(final List<String> catalogXMLs) throws CatalogApiException;
}
}

0 comments on commit d1a1474

Please sign in to comment.