Reset StreamCache in getBodyAs to be able to re-read it, see #2724 #894

Merged
merged 1 commit into from Nov 26, 2012

Conversation

Projects
None yet
5 participants
@patriknw
Owner

patriknw commented Nov 25, 2012

Should be backported to release-2.1

@RayRoestenburg

This comment has been minimized.

Show comment Hide comment
@RayRoestenburg

RayRoestenburg Nov 25, 2012

Contributor

Nice find, LGTM

Contributor

RayRoestenburg commented on 99b02c0 Nov 25, 2012

Nice find, LGTM

@akka-ci

This comment has been minimized.

Show comment Hide comment
@akka-ci

akka-ci Nov 25, 2012

Collaborator

Started jenkins job akka-pr-validator at https://jenkins.akka.io/job/akka-pr-validator/131/

Collaborator

akka-ci commented Nov 25, 2012

Started jenkins job akka-pr-validator at https://jenkins.akka.io/job/akka-pr-validator/131/

@akka-ci

This comment has been minimized.

Show comment Hide comment
@akka-ci

akka-ci Nov 25, 2012

Collaborator

jenkins job akka-pr-validator: Success - https://jenkins.akka.io/job/akka-pr-validator/131/

Collaborator

akka-ci commented Nov 25, 2012

jenkins job akka-pr-validator: Success - https://jenkins.akka.io/job/akka-pr-validator/131/

+ def getBodyAs[T](clazz: Class[T], camelContext: CamelContext): T = {
+ val result = camelContext.getTypeConverter.mandatoryConvertTo[T](clazz, body)
+ // to be able to re-read a StreamCache we must "undo" the side effect by resetting the StreamCache
+ resetStreamCache()

This comment has been minimized.

Show comment Hide comment
@viktorklang

viktorklang Nov 26, 2012

Owner

What happens if resetStreamCache is called by one Thread as another thread is reading the body? (i.e. the same CamelMessage is sent to 2 actors)

@viktorklang

viktorklang Nov 26, 2012

Owner

What happens if resetStreamCache is called by one Thread as another thread is reading the body? (i.e. the same CamelMessage is sent to 2 actors)

This comment has been minimized.

Show comment Hide comment
@patriknw

patriknw Nov 26, 2012

Owner

yes, that will not end well, body is Any and can be whatever mutable non-thread safe instance, so it's not much we can do about it.
I would say that the recommendation should be that the user should convert the body to a real immutable instance in the first endpoint (actor) before passing it on to other actors.

This fix is still relevant, because you expect to be able to call getBodyAs several times (in same actor) with the same result (e.g. adding a debug log stmt should not break things).

@patriknw

patriknw Nov 26, 2012

Owner

yes, that will not end well, body is Any and can be whatever mutable non-thread safe instance, so it's not much we can do about it.
I would say that the recommendation should be that the user should convert the body to a real immutable instance in the first endpoint (actor) before passing it on to other actors.

This fix is still relevant, because you expect to be able to call getBodyAs several times (in same actor) with the same result (e.g. adding a debug log stmt should not break things).

This comment has been minimized.

Show comment Hide comment
@RayRoestenburg

RayRoestenburg Nov 26, 2012

Contributor

On Mon, Nov 26, 2012 at 8:09 AM, Patrik Nordwall
notifications@github.comwrote:

In akka-camel/src/main/scala/akka/camel/CamelMessage.scala:

@@ -108,7 +108,21 @@ case class CamelMessage(body: Any, headers: Map[String, Any]) {
* Java API
*
*/

  • def getBodyAs[T](clazz: Class[T], camelContext: CamelContext): T = camelContext.getTypeConverter.mandatoryConvertTo[T](clazz, body)
  • def getBodyAs[T](clazz: Class[T], camelContext: CamelContext): T = {
  • val result = camelContext.getTypeConverter.mandatoryConvertTo[T](clazz, body)
  • // to be able to re-read a StreamCache we must "undo" the side effect by resetting the StreamCache
  • resetStreamCache()

yes, that will not end well, body is Any and can be whatever mutable
non-thread safe instance, so it's not much we can do about it.
I would say that the recommendation should be that the user should convert
the body to a real immutable instance in the first endpoint (actor) before
passing it on to other actors.

That is definitely a best practice (and people start doing that quite
naturally since the CamelMessage is hardly ever seen as part of the domain
and the normal immutable messages are easier to work with).

This fix is still relevant, because you expect to be able to call
getBodyAs several times (in same actor) with the same result (e.g. adding a
debug log stmt should not break things).

+1


Reply to this email directly or view it on GitHubhttps://github.com/akka/akka/pull/894/files#r2218705.

Raymond Roestenburg

code: http://github.com/RayRoestenburg
blog: http://roestenburg.agilesquad.com
twtr: @RayRoestenburg

@RayRoestenburg

RayRoestenburg Nov 26, 2012

Contributor

On Mon, Nov 26, 2012 at 8:09 AM, Patrik Nordwall
notifications@github.comwrote:

In akka-camel/src/main/scala/akka/camel/CamelMessage.scala:

@@ -108,7 +108,21 @@ case class CamelMessage(body: Any, headers: Map[String, Any]) {
* Java API
*
*/

  • def getBodyAs[T](clazz: Class[T], camelContext: CamelContext): T = camelContext.getTypeConverter.mandatoryConvertTo[T](clazz, body)
  • def getBodyAs[T](clazz: Class[T], camelContext: CamelContext): T = {
  • val result = camelContext.getTypeConverter.mandatoryConvertTo[T](clazz, body)
  • // to be able to re-read a StreamCache we must "undo" the side effect by resetting the StreamCache
  • resetStreamCache()

yes, that will not end well, body is Any and can be whatever mutable
non-thread safe instance, so it's not much we can do about it.
I would say that the recommendation should be that the user should convert
the body to a real immutable instance in the first endpoint (actor) before
passing it on to other actors.

That is definitely a best practice (and people start doing that quite
naturally since the CamelMessage is hardly ever seen as part of the domain
and the normal immutable messages are easier to work with).

This fix is still relevant, because you expect to be able to call
getBodyAs several times (in same actor) with the same result (e.g. adding a
debug log stmt should not break things).

+1


Reply to this email directly or view it on GitHubhttps://github.com/akka/akka/pull/894/files#r2218705.

Raymond Roestenburg

code: http://github.com/RayRoestenburg
blog: http://roestenburg.agilesquad.com
twtr: @RayRoestenburg

@bantonsson

This comment has been minimized.

Show comment Hide comment
@bantonsson

bantonsson Nov 26, 2012

Member

LGTM

Member

bantonsson commented Nov 26, 2012

LGTM

patriknw added a commit that referenced this pull request Nov 26, 2012

Merge pull request #894 from akka/wip-2724-streamcache-patriknw
Reset StreamCache in getBodyAs to be able to re-read it, see #2724

@patriknw patriknw merged commit e5fce9e into master Nov 26, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment