Use DBObject as type for jsScope in MapReduceCommand #44

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

troger commented Jan 3, 2012

Hi,

I don't know if it's ok for backward compatibility but I find it more convenient to use DBObject for jsScope type as it should be an Object with each field added to the global scope.

Moreover when using String, it just does not work when setting the jsScope with:

...
jsScope = Some("{ testVar: true }")
...

When looking at the mongo logs, it produces the following:

scope: "{ testVar: true }"

instead of

scope: { testVar: true }

I've added a test to the MapReduceSpec.

Maybe it's better to try fixing the jsScope as String instead of using DBObject.

WDYT?

troger commented Jan 4, 2012

Thinking about it and looking at the Java driver, it may be better to use Map[String, Any]:

jsScope: Option[Map[String, Any]]
Contributor

bwmcadams commented Jan 24, 2012

For backwards compatibility, this needs to be a Type Class to take both Strings and DBObjects.

mumoshu commented Jul 24, 2012

Hi,

Thank you for the great Scala driver.
I'm using this for my production service :)

For backwards compatibility, this needs to be a Type Class to take both Strings and DBObjects.

Excuse me, but do you really want backward compatibility here?
Can't we just make it use DBObject as troger originally suggested?

First of all, the current implementation jsScope: Option[String] is not working at all: Basically, it must be an Option[DBObject] or an Option[Map[_, _]] to properly serialized to a JavaScript object, as the original mapReduce comand's scope parameter expects.
In other words, all the users passing Option[String]s would be encountering the bug that the scope variables are totally missing from scope - as troger reported.
Now, shouldn't the users be notified with compile errors for passing jsScope an Option[String]?

mumoshu commented Jul 24, 2012

Also, how do you implement the type class serializes a String like """{ testVar: true }""" to a DBObject or a Map?
Parsing JSON manually?

@rozza rozza added a commit to rozza/casbah that referenced this pull request May 15, 2013

@rozza rozza Fixed support for jsScope (SCALA-43) (#44) d955eb1

@rozza rozza added a commit to rozza/casbah that referenced this pull request May 15, 2013

@rozza rozza Added troger to authors #44 a77fe59
Member

rozza commented May 15, 2013

This has been fixed in 2.6.1 - apologies for the delay

rozza closed this May 15, 2013

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