From ae628ada5000e9c497c999a942359754089fab81 Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Tue, 25 May 2021 09:33:04 -0400 Subject: [PATCH 1/3] feat: add context-aware cache key --- .../grpcutils/context/ContextualCacheKey.java | 22 ++++++ .../context/DefaultContextualCacheKey.java | 73 +++++++++++++++++++ .../grpcutils/context/RequestContext.java | 4 + .../context/RequestContextConstants.java | 26 +++++-- 4 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualCacheKey.java create mode 100644 grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualCacheKey.java diff --git a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualCacheKey.java b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualCacheKey.java new file mode 100644 index 0000000..7249c72 --- /dev/null +++ b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualCacheKey.java @@ -0,0 +1,22 @@ +package org.hypertrace.core.grpcutils.context; + +import java.util.function.Consumer; +import java.util.function.Function; + +public interface ContextualCacheKey { + RequestContext getContext(); + + T getData(); + + /** + * Calls the provided function with the cached data as an argument in the cached context, + * returning any result + */ + R callInContext(Function function); + + /** + * Calls the provided function with the cached data as an argument in the cached context. This is + * a no result version of the callInContext api. + */ + void runInContext(Consumer consumer); +} diff --git a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualCacheKey.java b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualCacheKey.java new file mode 100644 index 0000000..6aeef55 --- /dev/null +++ b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualCacheKey.java @@ -0,0 +1,73 @@ +package org.hypertrace.core.grpcutils.context; + +import java.util.Map; +import java.util.Map.Entry; +import java.util.Objects; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.stream.Collectors; + +class DefaultContextualCacheKey implements ContextualCacheKey { + private final RequestContext context; + private final T data; + private final Map meaningfulContextHeaders; + + DefaultContextualCacheKey(RequestContext context, T data) { + this.context = context; + this.data = data; + this.meaningfulContextHeaders = this.extractMeaningfulHeaders(context.getRequestHeaders()); + } + + @Override + public RequestContext getContext() { + return this.context; + } + + @Override + public T getData() { + return this.data; + } + + @Override + public R callInContext(Function function) { + return this.context.call(() -> function.apply(this.getData())); + } + + @Override + public void runInContext(Consumer consumer) { + this.context.run(() -> consumer.accept(this.getData())); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + DefaultContextualCacheKey that = (DefaultContextualCacheKey) o; + return Objects.equals(getData(), that.getData()) + && meaningfulContextHeaders.equals(that.meaningfulContextHeaders); + } + + @Override + public int hashCode() { + return Objects.hash(getData(), meaningfulContextHeaders); + } + + @Override + public String toString() { + return "DefaultContextualCacheKey{" + + "data=" + + data + + ", meaningfulContextHeaders=" + + meaningfulContextHeaders + + '}'; + } + + private Map extractMeaningfulHeaders(Map allHeaders) { + return allHeaders.entrySet().stream() + .filter( + entry -> + RequestContextConstants.CACHE_MEANINGFUL_HEADERS.contains( + entry.getKey().toLowerCase())) + .collect(Collectors.toUnmodifiableMap(Entry::getKey, Entry::getValue)); + } +} diff --git a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/RequestContext.java b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/RequestContext.java index 196fc35..4467101 100644 --- a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/RequestContext.java +++ b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/RequestContext.java @@ -79,4 +79,8 @@ public V call(@Nonnull Callable callable) { public void run(@Nonnull Runnable runnable) { Context.current().withValue(RequestContext.CURRENT, this).run(runnable); } + + public ContextualCacheKey buildContextualCacheKey(T data) { + return new DefaultContextualCacheKey<>(this, data); + } } diff --git a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/RequestContextConstants.java b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/RequestContextConstants.java index 4926041..4eb9bd3 100644 --- a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/RequestContextConstants.java +++ b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/RequestContextConstants.java @@ -1,13 +1,13 @@ package org.hypertrace.core.grpcutils.context; +import static io.grpc.Metadata.ASCII_STRING_MARSHALLER; + import io.grpc.Metadata; import java.util.Set; -import static io.grpc.Metadata.ASCII_STRING_MARSHALLER; - /** - * GRPC request context constants used to propagate the tenantId, authorization token, tracing headers etc - * in the platform services. + * GRPC request context constants used to propagate the tenantId, authorization token, tracing + * headers etc in the platform services. */ public class RequestContextConstants { public static final String TENANT_ID_HEADER_KEY = "x-tenant-id"; @@ -17,10 +17,20 @@ public class RequestContextConstants { public static final String AUTHORIZATION_HEADER = "authorization"; + /** The values in this set are looked up with case insensitivity. */ + public static final Set HEADER_PREFIXES_TO_BE_PROPAGATED = + Set.of( + TENANT_ID_HEADER_KEY, + "X-B3-", + "grpc-trace-bin", + "traceparent", + "tracestate", + AUTHORIZATION_HEADER); + /** - * The values in this set are looked up with case insensitivity. + * These headers may affect returned results and should be accounted for in any cached remote + * results */ - public static final Set HEADER_PREFIXES_TO_BE_PROPAGATED = - Set.of(TENANT_ID_HEADER_KEY, "X-B3-", "grpc-trace-bin", - "traceparent", "tracestate", AUTHORIZATION_HEADER); + static final Set CACHE_MEANINGFUL_HEADERS = + Set.of(TENANT_ID_HEADER_KEY, AUTHORIZATION_HEADER); } From 42eaf1c0afc3f3f6338238b31088237c8d2768eb Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Fri, 28 May 2021 09:35:58 -0400 Subject: [PATCH 2/3] test: add tests --- .../grpcutils/context/ContextualCacheKey.java | 22 ------ .../core/grpcutils/context/ContextualKey.java | 22 ++++++ ...acheKey.java => DefaultContextualKey.java} | 8 +- .../grpcutils/context/RequestContext.java | 8 +- .../context/DefaultContextualKeyTest.java | 75 +++++++++++++++++++ 5 files changed, 107 insertions(+), 28 deletions(-) delete mode 100644 grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualCacheKey.java create mode 100644 grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualKey.java rename grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/{DefaultContextualCacheKey.java => DefaultContextualKey.java} (88%) create mode 100644 grpc-context-utils/src/test/java/org/hypertrace/core/grpcutils/context/DefaultContextualKeyTest.java diff --git a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualCacheKey.java b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualCacheKey.java deleted file mode 100644 index 7249c72..0000000 --- a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualCacheKey.java +++ /dev/null @@ -1,22 +0,0 @@ -package org.hypertrace.core.grpcutils.context; - -import java.util.function.Consumer; -import java.util.function.Function; - -public interface ContextualCacheKey { - RequestContext getContext(); - - T getData(); - - /** - * Calls the provided function with the cached data as an argument in the cached context, - * returning any result - */ - R callInContext(Function function); - - /** - * Calls the provided function with the cached data as an argument in the cached context. This is - * a no result version of the callInContext api. - */ - void runInContext(Consumer consumer); -} diff --git a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualKey.java b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualKey.java new file mode 100644 index 0000000..2c4da5a --- /dev/null +++ b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualKey.java @@ -0,0 +1,22 @@ +package org.hypertrace.core.grpcutils.context; + +import java.util.function.Consumer; +import java.util.function.Function; + +public interface ContextualKey { + RequestContext getContext(); + + T getData(); + + /** + * Calls the function in the key's context and providing the key's data as an argument, returning + * any result + */ + R callInContext(Function function); + + /** + * Calls the function in the key's context and providing the key's data as an argument, returning + * no result + */ + void runInContext(Consumer consumer); +} diff --git a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualCacheKey.java b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualKey.java similarity index 88% rename from grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualCacheKey.java rename to grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualKey.java index 6aeef55..1e35dce 100644 --- a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualCacheKey.java +++ b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualKey.java @@ -7,12 +7,12 @@ import java.util.function.Function; import java.util.stream.Collectors; -class DefaultContextualCacheKey implements ContextualCacheKey { +class DefaultContextualKey implements ContextualKey { private final RequestContext context; private final T data; private final Map meaningfulContextHeaders; - DefaultContextualCacheKey(RequestContext context, T data) { + DefaultContextualKey(RequestContext context, T data) { this.context = context; this.data = data; this.meaningfulContextHeaders = this.extractMeaningfulHeaders(context.getRequestHeaders()); @@ -42,7 +42,7 @@ public void runInContext(Consumer consumer) { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - DefaultContextualCacheKey that = (DefaultContextualCacheKey) o; + DefaultContextualKey that = (DefaultContextualKey) o; return Objects.equals(getData(), that.getData()) && meaningfulContextHeaders.equals(that.meaningfulContextHeaders); } @@ -54,7 +54,7 @@ public int hashCode() { @Override public String toString() { - return "DefaultContextualCacheKey{" + return "DefaultContextualKey{" + "data=" + data + ", meaningfulContextHeaders=" diff --git a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/RequestContext.java b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/RequestContext.java index 4467101..6116397 100644 --- a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/RequestContext.java +++ b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/RequestContext.java @@ -80,7 +80,11 @@ public void run(@Nonnull Runnable runnable) { Context.current().withValue(RequestContext.CURRENT, this).run(runnable); } - public ContextualCacheKey buildContextualCacheKey(T data) { - return new DefaultContextualCacheKey<>(this, data); + public ContextualKey buildContextualKey(T data) { + return new DefaultContextualKey<>(this, data); + } + + public ContextualKey buildContextualKey() { + return new DefaultContextualKey<>(this, null); } } diff --git a/grpc-context-utils/src/test/java/org/hypertrace/core/grpcutils/context/DefaultContextualKeyTest.java b/grpc-context-utils/src/test/java/org/hypertrace/core/grpcutils/context/DefaultContextualKeyTest.java new file mode 100644 index 0000000..3c7980c --- /dev/null +++ b/grpc-context-utils/src/test/java/org/hypertrace/core/grpcutils/context/DefaultContextualKeyTest.java @@ -0,0 +1,75 @@ +package org.hypertrace.core.grpcutils.context; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import java.util.function.Consumer; +import java.util.function.Function; +import org.junit.jupiter.api.Test; + +class DefaultContextualKeyTest { + + @Test + void callsProvidedMethodsInContext() { + RequestContext testContext = RequestContext.forTenantId("test-tenant"); + ContextualKey key = new DefaultContextualKey<>(testContext, "input"); + + Function testFunction = + value -> + "returned: " + + value + + " for " + + RequestContext.CURRENT.get().getTenantId().orElseThrow(); + + assertEquals("returned: input for test-tenant", key.callInContext(testFunction)); + } + + @Test + void runsProvidedMethodInContext() { + RequestContext testContext = RequestContext.forTenantId("test-tenant"); + ContextualKey key = new DefaultContextualKey<>(testContext, "input"); + + Consumer testConsumer = mock(Consumer.class); + + doAnswer( + invocation -> { + assertSame(testContext, RequestContext.CURRENT.get()); + return null; + }) + .when(testConsumer) + .accept(any()); + key.runInContext(testConsumer); + verify(testConsumer, times(1)).accept(eq("input")); + } + + @Test + void matchesEquivalentKeysOnly() { + RequestContext tenant1Context = RequestContext.forTenantId("first"); + RequestContext alternateTenant1Context = RequestContext.forTenantId("first"); + alternateTenant1Context.add("other", "value"); + RequestContext tenant2Context = RequestContext.forTenantId("second"); + + assertEquals( + new DefaultContextualKey<>(tenant1Context, "input"), + new DefaultContextualKey<>(tenant1Context, "input")); + + assertEquals( + new DefaultContextualKey<>(tenant1Context, "input"), + new DefaultContextualKey<>(alternateTenant1Context, "input")); + + assertNotEquals( + new DefaultContextualKey<>(tenant1Context, "input"), + new DefaultContextualKey<>(tenant2Context, "input")); + + assertNotEquals( + new DefaultContextualKey<>(tenant1Context, "input"), + new DefaultContextualKey<>(tenant1Context, "other input")); + } +} From a1f55020fbd418c9e3cbc40c3b493a32ffec7bd9 Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Thu, 3 Jun 2021 08:08:44 -0400 Subject: [PATCH 3/3] refactor: add no data run methods to key --- .../core/grpcutils/context/ContextualKey.java | 5 +++++ .../core/grpcutils/context/DefaultContextualKey.java | 11 +++++++++++ .../grpcutils/context/DefaultContextualKeyTest.java | 10 ++++++++++ 3 files changed, 26 insertions(+) diff --git a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualKey.java b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualKey.java index 2c4da5a..37eb3f7 100644 --- a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualKey.java +++ b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualKey.java @@ -2,6 +2,7 @@ import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.Supplier; public interface ContextualKey { RequestContext getContext(); @@ -14,9 +15,13 @@ public interface ContextualKey { */ R callInContext(Function function); + R callInContext(Supplier supplier); + /** * Calls the function in the key's context and providing the key's data as an argument, returning * no result */ void runInContext(Consumer consumer); + + void runInContext(Runnable runnable); } diff --git a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualKey.java b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualKey.java index 1e35dce..5088718 100644 --- a/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualKey.java +++ b/grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/DefaultContextualKey.java @@ -5,6 +5,7 @@ import java.util.Objects; import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.Supplier; import java.util.stream.Collectors; class DefaultContextualKey implements ContextualKey { @@ -33,11 +34,21 @@ public R callInContext(Function function) { return this.context.call(() -> function.apply(this.getData())); } + @Override + public R callInContext(Supplier supplier) { + return this.context.call(supplier::get); + } + @Override public void runInContext(Consumer consumer) { this.context.run(() -> consumer.accept(this.getData())); } + @Override + public void runInContext(Runnable runnable) { + this.context.run(runnable); + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/grpc-context-utils/src/test/java/org/hypertrace/core/grpcutils/context/DefaultContextualKeyTest.java b/grpc-context-utils/src/test/java/org/hypertrace/core/grpcutils/context/DefaultContextualKeyTest.java index 3c7980c..2820ead 100644 --- a/grpc-context-utils/src/test/java/org/hypertrace/core/grpcutils/context/DefaultContextualKeyTest.java +++ b/grpc-context-utils/src/test/java/org/hypertrace/core/grpcutils/context/DefaultContextualKeyTest.java @@ -12,6 +12,7 @@ import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.Supplier; import org.junit.jupiter.api.Test; class DefaultContextualKeyTest { @@ -29,6 +30,11 @@ void callsProvidedMethodsInContext() { + RequestContext.CURRENT.get().getTenantId().orElseThrow(); assertEquals("returned: input for test-tenant", key.callInContext(testFunction)); + + Supplier testSupplier = + () -> "returned for " + RequestContext.CURRENT.get().getTenantId().orElseThrow(); + + assertEquals("returned for test-tenant", key.callInContext(testSupplier)); } @Test @@ -47,6 +53,10 @@ void runsProvidedMethodInContext() { .accept(any()); key.runInContext(testConsumer); verify(testConsumer, times(1)).accept(eq("input")); + + Runnable testRunnable = mock(Runnable.class); + key.runInContext(testRunnable); + verify(testRunnable, times(1)).run(); } @Test