[v2] Possible marshal/unmarshaling bug

[someone1]

I've been getting warnings from the logger I placed in the onError function:

nds:loadCache setValue: datastore: cannot load field "rg" into a "models.UserAccount": multiple-valued property requires a slice field type

I think there's an opportunity to update the tests to ensure that any logs made are expected, otherwise to fail. This may help uncover situations in which we think we're pulling from the cache but are really failing at a later step (e.g. setValue) and falling back to the datastore.

[someone1]

I've finally tracked this issue down - I implement the PropertyLoadSaver interface and during migrations I may drop a field from my structs and thus remove it from the PropertyList slice prior to calling datastore.LoadStruct. To do this, I delete any matched element using ps = append(ps[:idx], ps[idx+1:]...) which updates the local slice to the new slice length, but the original slice kept in nds has the original length (both still pointing to the same underlying array), so the last element of the slice gets duplicated when marshaled.

I'm not sure what's the best way to address this. I don't think that updating entities using this interface is bad practice perse, and we can't modify the signature of the function so maybe this can be supported another way?

What do you think of the following:

• When dropping elements from the entity using PropertyLoadSaver, copy the slice locally in the user defined Load function to remove the elements you don't want prior to calling LoadStruct
• For the original sliced passed in, set the value of the slice at the index where you want to delete an item to nds.DeleteProperty (a special value defined in nds) - you can modify slices in-place passed into functions!
• NDS would scan the PropertyList for this value after calling Load internally and remove those items from the slice prior to continuing the caching process, only for the situations where we will be writing to the cache

Alternatively, push this back on the user (me in this case) to explicitly save the new entity if its updated during a Load so nds immediately clears its cached value

[someone1]

Actually, I think the best solution here is to call setValue after marshalling the data (just a simple reordering of events). This way the PropertyList is exactly as it came from datastore and no longer required internally in nds.

No dumb hacks like I described, no user "gotchas" like copying the list before modifying or must save after update when it's not required.