Undeclared properties

[klobuczek]

Fixes #
Introduces handling of undeclared properties via optional Neo4j::UndeclaredProperties module.
This pull introduces/changes:

• I have tried to keep existing code without any functional change with one exception. I fixed a problem where _persisted_obj was not refreshed after model update
• All new functionality is in a separate optional module which if included changes the behavior of the basic model manipulation method.

Pings:
@cheerfulstoic
@subvertallchris

[klobuczek]

Looking for suggestion how to fix the 1 rubocop offense, which is a too long module. I just introduced the absolutely necessary hooks. I could split the persistence module into create and update persistence.

[subvertallchris]

I really like the way you implemented this!

Out of curiosity, would it be preferable to require that undeclared properties be set and accessed through dedicated methods instead of mixed with existing mass-assignment? My concern is that this approach could lead to accidental, invisible setting of incorrect keys. If the user knows that they are going to be adding keys dynamically, it seems like they should be able to programmatically assign them separately. What do you think?

Full disclosure, I am not exactly the target audience for this, as I think that undeclared properties are pure evil. I won't stand in the way of a merge if others give their +1. ;-)

[klobuczek]

@subvertallchris I couldn't think of more intuitive way than using the standard update_attributes method. This is the way active_node gem was doing it which was inspired by mongo_mapper. Both being obsolete meanwhile I just looked at mongoid and they are even going a step further. They are even generating the accessor methods for dynamic attributes if the Dynamic module is included. https://docs.mongodb.com/ruby-driver/master/tutorials/6.0.0/mongoid-documents/#dynamic-fields . So multiple places did it intuitively in a similar way.

Furthermore, there is an additional safety net in form of permitted attributes in rails controllers. So in our case, those fields are not totally wild. The superset of allowed undeclared properties is restricted on user/client basis.

[codecov-io]

Current coverage is 97.01% (diff: 100%)

Merging #1294 into master will increase coverage by 0.02%

@@             master      #1294   diff @@
==========================================
  Files           199        201     +2   
  Lines         12051      12148    +97   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          11688      11785    +97   
  Misses          363        363          
  Partials          0          0          

Powered by Codecov. Last update 2deda24...f3c9b63

[subvertallchris]

Got it, thanks for the background info. 👍 as far as I'm concerned, but @cheerfulstoic and/or @ProGM should probably chime in, too.

[cheerfulstoic]

I also really like how this is an optional module. I looked through the code and couldn't find anything bad, so 👍 from me. I'd like a final opinion from @ProGM though

[ProGM]

I like this!
The fact of being a separate model solves 90% of my concerns about accidental attributes assignment, so it looks definitely good to me! 👍

[cheerfulstoic]

Great, merging! Thanks again!