masumsoft / express-cassandra

Cassandra ORM/ODM/OGM for NodeJS with support for Apache Cassandra, ScyllaDB, Datastax Enterprise, Elassandra & JanusGraph.
http://express-cassandra.readthedocs.io
GNU Lesser General Public License v3.0
228 stars 67 forks source link

Reusing an options object doesn't work when it's used for finds as well as a stream or eachRow call. #246

Closed mmillies closed 2 years ago

mmillies commented 2 years ago

eachRow https://github.com/masumsoft/express-cassandra/blob/8a2f85e6595210b6dc1b9aa701f64c2a5d4aa754/src/orm/base_model.js#L437

stream https://github.com/masumsoft/express-cassandra/blob/8a2f85e6595210b6dc1b9aa701f64c2a5d4aa754/src/orm/base_model.js#L498

Any issue with changing this:

options = _.defaultsDeep(options, defaults);

to

options = _.defaultsDeep({}, options, defaults);

?

I ran into this while reusing a query options object such as:

const DEFAULT_QUERY_OPTIONS = { raw: true };

Worked fine until I happened to execute a stream or eachRow call. Those modify the passed in options object, and subsequent find calls using the default options object "just hang" (because they are not expected to return the query).

passed in options object being modified: https://github.com/masumsoft/express-cassandra/blob/8a2f85e6595210b6dc1b9aa701f64c2a5d4aa754/src/orm/base_model.js#L500

https://github.com/masumsoft/express-cassandra/blob/8a2f85e6595210b6dc1b9aa701f64c2a5d4aa754/src/orm/base_model.js#L439

work around of course is cloning in my layer... so I'm doing that at the moment.

(I'm not opposed to opening a pull request and added a test. Just wanted to test the waters with the issue before I invest time trying to install the elassandra_janusgraph dependencies for running the unit tests.)

let me know. thanks.

masumsoft commented 2 years ago

@mmillies yes that's a good suggestion and the change still passes all the available tests. I'm going to push the changes.

masumsoft commented 2 years ago

Fixed in v2.8.0

mmillies commented 2 years ago

thanks.