Add support for Destination Middleware #664

prayansh · 2020-05-28T22:54:42Z

Summary

This PR implements destination specific middleware that function similar to source middleware, but instead run only for the specified integration.
These can be configured via the Analytics.Builder.
Example

builder
    .useDestinationMiddleware("Segment.io",
      new Middleware() {
          @Override
          public void intercept(Chain chain) {
            // ...transform payload
            chain.proceed(chain.payload()); // send new payload to the chain
          }
    )

This PR also deprecates middleware for sourceMiddleware which are middleware that run for all integrations.

Ref: LIBMOBILE-38

Test Cases

single destination middleware runs (DestinationMiddlewareTest.middlewareRuns)
destination middleware only runs for specified destination (DestinationMiddlewareTest.middlewareDoesNotRunForOtherIntegration)
same middleware can be used for multiple integrations (DestinationMiddlewareTest.middlewareWillRunForMultipleIntegrations)
Middleware can drop event for specified integration
(DestinationMiddlewareTest.middlewareCanShortCircuit)
Middleware can chain
(DestinationMiddlewareTest.middlewareCanChain)
Middleware can transform the payload
(DestinationMiddlewareTest.middlewareCanTransform)

analytics/src/main/java/com/segment/analytics/IntegrationOperation.java

prayansh · 2020-05-28T23:22:31Z

analytics/src/main/java/com/segment/analytics/internal/Utils.java

@@ -235,6 +235,7 @@ public static String assertNotNullOrEmpty(String text, @Nullable String name) {
  /** Returns an immutable copy of the provided map. */
  @NonNull
  public static <K, V> Map<K, V> immutableCopyOf(@NonNull Map<K, V> map) {
+    // TODO (major version change) change this out to allow @Nullable input and guard against it


ideally this would become

@NonNull public static <K, V> Map<K, V> immutableCopyOf(@Nullable Map<K, V> map) { if (isNullOrEmpty(map)) { return Collections.emptyMap(); } return Collections.unmodifiableMap(new LinkedHashMap<>(map)); }

but dont wanna do a breaking change in this version.

Might be good to leave that in the TODO comment in the code itself as well.

prayansh · 2020-05-28T23:24:11Z

analytics/src/test/java/com/segment/analytics/IntegrationOperationTest.java

@@ -361,4 +366,44 @@ public void eventPlanOverridesSchemaDefault() throws IOException {
                + "}"));
    verify(integration, never()).track(payload);
  }
+
+  @Test


These tests have nothing to do with destination middleware, but felt good to have as unit tests

prayansh · 2020-05-28T23:24:59Z

analytics/src/main/java/com/segment/analytics/Analytics.java

+     * Add a {@link Middleware} custom source middleware. This will be run before sending to all
+     * integrations
+     */
+    public Builder useSourceMiddleware(Middleware middleware) {


drop-in replacement for middleware

bsneed · 2020-05-29T18:31:54Z

analytics/src/main/java/com/segment/analytics/Analytics.java

+     * Add a {@link Middleware} custom source middleware. This will be run before sending to all
+     * integrations
+     */
+    public Builder useSourceMiddleware(Middleware middleware) {


bsneed · 2020-05-29T18:35:03Z

analytics/src/main/java/com/segment/analytics/IntegrationOperation.java

+        List<Middleware> applicableMiddleware = getMiddlewareList(destinationMiddleware, key);
+        runMiddlewareChain(
+            payload,
+            applicableMiddleware,


the isIntegrationEnabled bit that seems to have been removed here seems like it might be important. If an integration is disabled on segment.com, there's no need to run it's middleware right?

never mind, I see it down below. however, it's repeated. seems like that could be placed here instead and checked once?

bsneed · 2020-05-29T18:36:44Z

analytics/src/main/java/com/segment/analytics/internal/Utils.java

@@ -235,6 +235,7 @@ public static String assertNotNullOrEmpty(String text, @Nullable String name) {
  /** Returns an immutable copy of the provided map. */
  @NonNull
  public static <K, V> Map<K, V> immutableCopyOf(@NonNull Map<K, V> map) {
+    // TODO (major version change) change this out to allow @Nullable input and guard against it


bsneed · 2020-05-29T18:37:04Z

analytics/src/main/java/com/segment/analytics/internal/Utils.java

@@ -235,6 +235,7 @@ public static String assertNotNullOrEmpty(String text, @Nullable String name) {
  /** Returns an immutable copy of the provided map. */
  @NonNull
  public static <K, V> Map<K, V> immutableCopyOf(@NonNull Map<K, V> map) {
+    // TODO (major version change) change this out to allow @Nullable input and guard against it


Might be good to leave that in the TODO comment in the code itself as well.

analytics/src/main/java/com/segment/analytics/IntegrationOperation.java

prayansh added 8 commits May 26, 2020 17:11

deprecate middleware for sourceMiddleware

2eed0ab

update tests

e77acb6

update tests

aacc078

add destinationMiddleware to Builder

134b5af

implement destination middleware runner

b5c0a97

more IntergrationOperation tests

32b0d49

implement destination middleware tests

fe49c32

run formatter

1ee88f8

prayansh requested a review from bsneed May 28, 2020 23:03

prayansh added 2 commits May 28, 2020 16:08

hot swap RealMiddlewareChain for MiddlewareChainRunner

3e532ea

run formatter

956a647

prayansh commented May 28, 2020

View reviewed changes

analytics/src/main/java/com/segment/analytics/IntegrationOperation.java Show resolved Hide resolved

prayansh commented May 28, 2020

View reviewed changes

add sample middleware with tests

f29f878

prayansh changed the title ~~Add support for Destination Middleware [WIP]~~ Add support for Destination Middleware May 29, 2020

bsneed approved these changes May 29, 2020

View reviewed changes

update comment

251d097

prayansh merged commit aa6a69b into master May 29, 2020

prayansh deleted the pray/dest-middleware branch May 29, 2020 21:00

prayansh mentioned this pull request Jun 5, 2020

Update Android/iOS docs to include Destination Middleware segmentio/segment-docs#935

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Destination Middleware #664

Add support for Destination Middleware #664

prayansh commented May 28, 2020 •

edited

Loading

prayansh May 28, 2020

bsneed May 29, 2020

bsneed May 29, 2020

prayansh May 28, 2020

prayansh May 28, 2020

bsneed May 29, 2020

bsneed May 29, 2020

bsneed May 29, 2020

bsneed May 29, 2020

bsneed May 29, 2020

bsneed May 29, 2020

Add support for Destination Middleware #664

Add support for Destination Middleware #664

Conversation

prayansh commented May 28, 2020 • edited Loading

Summary

Test Cases

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prayansh commented May 28, 2020 •

edited

Loading