Scala Stream Collector: add URL redirect replacement macro #3491
Conversation
|
Thanks for your pull request. Is this your first contribution to a Snowplow open source project? Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://github.com/snowplow/snowplow/wiki/CLA to learn more and sign. Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks. |
| @@ -231,7 +231,9 @@ class CollectorService( | |||
| queryParams: Map[String, String] | |||
| ): (HttpResponse, List[Array[Byte]]) = | |||
| queryParams.get("u") match { | |||
| case Some(target) => (HttpResponse(StatusCodes.Found).withHeaders(`Location`(target)), Nil) | |||
| case Some(target) => | |||
| val replacedTarget = if (event.isSetNetworkUserId) target.replaceAllLiterally("${SP_UUID}", event.networkUserId) else target | |||
There was a problem hiding this comment.
should we make ${SP_UUID} configurable?
| event.networkUserId = "1234" | ||
| val (res, Nil) = service.buildRedirectHttpResponse(event, "k", Map("u" -> "http://localhost/?uid=${SP_UUID}")) | ||
| res shouldEqual HttpResponse(302).withHeaders(`Location`("http://localhost/?uid=1234")) | ||
| } |
There was a problem hiding this comment.
could you move this test to https://github.com/snowplow/snowplow/pull/3491/files#diff-88f19e201a23cbae2294497905a6b8a1R201 , you can make a local copy of the event if there are issues with setting the networkUserId
|
@BenFradet I made the suggested changes, and updated the documentation. |
|
That fit's really nicely to my issue https://github.com/snowplow/snowplow/issues/2491 Thanks! |
|
Hmm, reading @christoph-buente's issue, the default placeholder here really ought to be |
|
@alexanderdean yes, whatever it might be, i care more about the possibility to do it at all. As @rbolkey pointed out, it is needed to implement some sort of cookie sync. |
|
No issue changing the default. I wasn't distinguishing between the different types of identities that snowplow recognizes. The original value was simply what we've been using in production. |
The macro placeholder is hard coded to ${SP_UUID}.
rbolkey
force-pushed
the
onespot-cookie-replacement-macro
branch
from
e9bc569
to
71937bb
Compare
|
I updated the default token value, and rebased off master. |
|
Thanks @rbolkey ! |
BenFradet
changed the title
Updates the Scala Stream Collector to look for a replacement macro in the redirect url in order to support cookie syncing with third parties.
Currently, the macro placeholder is hard coded to ${SP_UUID}. I image this feature could be enabled through the config and the placeholder parameterized, if this is desirable.
@alexanderdean