[MAGNOLIA-5377] Implement multi-value properties for MockNode Created: 10/Oct/13  Updated: 25/Oct/13  Resolved: 14/Oct/13

Status: Closed
Project: Magnolia
Component/s: None
Affects Version/s: None
Fix Version/s: 5.2

Type: Bug Priority: Neutral
Reporter: Daniel Lipp Assignee: Daniel Lipp
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Sprint: 5.1.1

 Description   

MockNode currently will throw UnsupportedOperationException when trying to use it with multi-value properties.
Implementing that feature would ease testing in quite a few cases. Using dynamic mocks (Mockito) instead can be a much bigger effort.



 Comments   
Comment by Jan Haderka [ 14/Oct/13 ]

the error message w/ square brackets in GetValue() is confusing. I believe there's no reason to not use normal brackets right?

+            throw new ValueFormatException("Cannot call getValue[] on a multi-value property.");

But more importantly, the new implementation would act as multivalue property only if there were more then 1 values set. What about multival props with single values set? or those just created, thus w/o any values?

Also the old impl allowed to just making property appear multivalued w/o need to set vals explicitly if they were not necessary for test.

Comment by Jan Haderka [ 14/Oct/13 ]

Sorry for being ... myself i guess , but I don't see the point in changing the constructor for this.

-    public MockProperty(String name, Value[] values, MockNode parent) {
+    public MockProperty(String name, Value[] values, MockNode parent, boolean multiple) {

since we already have other constructor that takes single value, we know that anyone using the init method w/ array wants to create multivalue property. No need to have to define it in constructor. The only time this becomes a bit cumbersome is when you want to set value to null. The you would have to cast to either Value or Object or Value[], but since you have to do that for the first two already I don't see any issue w/ that. Or did I miss anything?

Comment by Daniel Lipp [ 14/Oct/13 ]

I thought it helps to explicitly create problematic constellations with multiValued properties. But as those potential problems are actually only relevant for the tests in MockPropertyTest that's most likely. Happily removed that additional constructor and like it that way

Generated at Mon Feb 12 04:04:35 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.