Assoc with null values #457
Conversation
|
👍 |
| $prepared = $this->processAssoc($value); | ||
| $node->setProperty($mapping['assoc'], $prepared['keys'], PropertyType::STRING); | ||
| $node->setProperty($mapping['assocNulls'], $prepared['nulls'], PropertyType::STRING); | ||
| $value = $prepared['values']; } |
There was a problem hiding this comment.
cs issue with the closing parenthesis.
|
interesting, i think this will work. i guess it is also BC in the sense that previously stored hashmaps without any null values still work. did we not implement an exception for when one value is null? |
|
The exception came from phpcr when trying to convert the null value to a string - which is another issue in Jackalope: it should not store null values in multivalued properties but not throw an exception either. |
| @@ -509,7 +509,7 @@ public function testAssocProperty() | |||
| { | |||
| $user = new User(); | |||
| $user->username = "test"; | |||
| $assocArray = array('foo' => 'bar', 'ding' => 'dong'); | |||
| $assocArray = array('foo' => 'bar', 'ding' => 'dong', 'dong' => null, 'dang' => null); | |||
There was a problem hiding this comment.
my question was what happens when we have 'dang' => 'buzz' - but it will work too so its ok like this.
| protected function processAssoc(NodeInterface $node, array $mapping, array $assoc) | ||
| { | ||
| $keys = array_keys($assoc); | ||
| $values = array_values(array_map(function ($item) { return null === $item ? '<null>' : $item; }, $assoc)); |
There was a problem hiding this comment.
think some whitespace in the two closures would help readability
There was a problem hiding this comment.
I will break the lines ...
|
hmm why do we even need to store |
|
@lsmith77 the point is that the order of the fields needs to be |
|
i dont think that this is necessary. if have 3 arrays: 1) ordered list of keys 2) ordered list of values without nulls 3) ordered list of keys with a null value. i can then iterate over the list of keys, create a new array where i check if the key is in the list of null values, if so i use null, otherwise i pop off first value of the list of values. |
|
true. i think that sounds slightly more elegant. i think querying on those things is a PITA anyways - and when you look for null values you should check if $property . Null field is non-empty, not in the values. |
|
Right, I thought about Lukas proposal first but decided to start with a simpler version. Anyway I will give it a try ... |
|
Done ... |
| reset($values); | ||
| $result = array(); | ||
| foreach ($keys as $key) { | ||
| if (in_array($key, $nulls)) { |
There was a problem hiding this comment.
you could flip the $nulls array and then do an isset check instead of in_array. if somebody has a larger array with many nulls, that will be faster.
|
thanks uwe. i added one comment, but its not critical to me. can you squash the commets please, then i will merge. |
|
jackalope/jackalope#210 would tend to remove null keys rather than keep them. but i think its more natural to do it this way for hashmaps. people just need to be aware that when using non-assoc multivalue, null will be removed. (or will they?) |
|
squashed ... |
|
thanks uwe! |
|
added a little fix to deal with older data structures 22d03a8 |
See my corresponding message on symfony-cmf-devs.
Feedback welcome.