Skip to content

Commit 1302f78

Browse files
davidgraystondavidgrayston
committed
SDK-766: Don't return MultiValue attribute when a Value is empty
Rename filter methods to allow
1 parent 1bcaea8 commit 1302f78

File tree

4 files changed

+61
-139
lines changed

4 files changed

+61
-139
lines changed

‎src/Yoti/Entity/MultiValue.php‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ private function applyFilters()
6868
// Filter nested items.
6969
foreach ($this->getArrayCopy() as $item) {
7070
if ($item instanceof MultiValue) {
71-
$item->resetFilters();
7271
foreach ($this->filters as $callback) {
7372
$item->filter($callback);
7473
}
@@ -99,7 +98,7 @@ private function filterItem($item)
9998
*
10099
* @return MultiValue
101100
*/
102-
public function filterInstance($type)
101+
public function allowInstance($type)
103102
{
104103
return $this->filter(function ($item) use ($type) {
105104
return $item instanceof $type;
@@ -113,27 +112,13 @@ public function filterInstance($type)
113112
*
114113
* @return MultiValue
115114
*/
116-
public function filterType($type)
115+
public function allowType($type)
117116
{
118117
return $this->filter(function ($item) use ($type) {
119118
return gettype($item) === $type;
120119
});
121120
}
122121

123-
/**
124-
* Resets items to original values.
125-
*
126-
* @return MultiValue
127-
*/
128-
public function resetFilters()
129-
{
130-
$this->assertMutable('Attempting to reset filters on immutable array');
131-
$this->filters = [];
132-
$this->applyFilters();
133-
return $this;
134-
}
135-
136-
137122
/**
138123
* Make this MultiValue Immutable.
139124
*

‎src/Yoti/Util/Profile/AttributeConverter.php‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ private static function convertValueBasedOnAttributeName($value, $attrName)
4040
return null;
4141
}
4242
return $value
43-
->filterInstance(Image::class)
43+
->allowInstance(Image::class)
4444
->immutable();
4545

4646
default:
@@ -101,18 +101,10 @@ private static function convertMultiValue($value)
101101
$protoMultiValue->mergeFromString($value);
102102
$items = [];
103103
foreach ($protoMultiValue->getValues() as $protoValue) {
104-
$item = null;
105-
try {
106-
$item = self::convertValueBasedOnContentType(
107-
$protoValue->getData(),
108-
$protoValue->getContentType()
109-
);
110-
} catch (AttributeException $e) {
111-
error_log($e->getMessage() . " (MultiValue Value ContentType: {$protoValue->getContentType()})", 0);
112-
} catch (\Exception $e) {
113-
error_log($e->getMessage(), 0);
114-
}
115-
$items[] = $item;
104+
$items[] = self::convertValueBasedOnContentType(
105+
$protoValue->getData(),
106+
$protoValue->getContentType()
107+
);
116108
}
117109
return new MultiValue($items);
118110
}

‎tests/Entity/MultiValueTest.php‎

Lines changed: 16 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,12 @@ public function setup()
3434
}
3535

3636
/**
37-
* @covers ::filterInstance
37+
* @covers ::allowInstance
3838
*/
3939
public function testMultiValueFilterArrayAccess()
4040
{
4141
// Allow images
42-
$this->multiValue->filterInstance(Image::class);
42+
$this->multiValue->allowInstance(Image::class);
4343

4444
$this->assertCount(2, $this->multiValue);
4545
$this->assertInstanceOf(Image::class, $this->multiValue[0]);
@@ -48,7 +48,7 @@ public function testMultiValueFilterArrayAccess()
4848
$this->assertEquals('image/png', $this->multiValue[1]->getMimeType());
4949

5050
// Allow MultiValue.
51-
$this->multiValue->filterInstance(MultiValue::class);
51+
$this->multiValue->allowInstance(MultiValue::class);
5252

5353
$this->assertCount(3, $this->multiValue);
5454
$this->assertInstanceOf(MultiValue::class, $this->multiValue[2]);
@@ -59,12 +59,12 @@ public function testMultiValueFilterArrayAccess()
5959
}
6060

6161
/**
62-
* @covers ::filterInstance
62+
* @covers ::allowInstance
6363
*/
6464
public function testMultiValueFilterIterator()
6565
{
6666
// Allow images
67-
$this->multiValue->filterInstance(Image::class);
67+
$this->multiValue->allowInstance(Image::class);
6868

6969
foreach ($this->multiValue as $item) {
7070
$this->assertInstanceOf(Image::class, $item);
@@ -73,14 +73,13 @@ public function testMultiValueFilterIterator()
7373

7474
/**
7575
* @covers ::filterType
76-
* @covers ::filterInstance
76+
* @covers ::allowInstance
7777
*/
7878
public function testMultiValueFilterMultipleTypes()
7979
{
80-
// Allow images and strings;
8180
$this->multiValue
82-
->filterInstance(Image::class)
83-
->filterType('string');
81+
->allowInstance(Image::class)
82+
->allowType('string');
8483

8584
$this->assertCount(5, $this->multiValue);
8685
$this->assertEquals('string 1', $this->multiValue[0]);
@@ -96,12 +95,12 @@ public function testMultiValueFilterMultipleTypes()
9695
*/
9796
public function testMultiValueCustomFilters()
9897
{
99-
// Allow images
98+
// Custom filter to allow images.
10099
$this->multiValue->filter(function ($item) {
101100
return $item instanceof Image;
102101
});
103102

104-
// Allow strings
103+
// Custom filter to allow strings.
105104
$this->multiValue->filter(function ($item) {
106105
return gettype($item) === 'string';
107106
});
@@ -113,62 +112,6 @@ public function testMultiValueCustomFilters()
113112
$this->assertEquals('image/jpeg', $this->multiValue[2]->getMimeType());
114113
$this->assertInstanceOf(Image::class, $this->multiValue[3]);
115114
$this->assertEquals('image/png', $this->multiValue[3]->getMimeType());
116-
117-
// Filter long strings.
118-
$this->multiValue->resetFilters();
119-
$this->multiValue->filter(function ($item) {
120-
return gettype($item) === 'string' && strlen($item) > 8;
121-
});
122-
$this->assertCount(1, $this->multiValue);
123-
$this->assertEquals('longer string 5', $this->multiValue[0]);
124-
}
125-
126-
/**
127-
* @covers ::resetFilters
128-
*/
129-
public function testMultiValueResetFilters()
130-
{
131-
$this->assertCount(8, $this->multiValue);
132-
133-
// Allow images
134-
$this->multiValue->filter(function ($item) {
135-
return $item instanceof Image;
136-
});
137-
138-
// Allow strings
139-
$this->multiValue->filter(function ($item) {
140-
return gettype($item) === 'string';
141-
});
142-
143-
$this->assertCount(5, $this->multiValue);
144-
145-
// Reset filters.
146-
$this->multiValue->resetFilters();
147-
$this->assertCount(8, $this->multiValue);
148-
149-
// Allow MultiValue.
150-
$this->multiValue->filter(function ($item) {
151-
return $item instanceof MultiValue;
152-
});
153-
154-
// Allow images
155-
$this->multiValue->filter(function ($item) {
156-
return $item instanceof Image;
157-
});
158-
159-
$this->assertInstanceOf(MultiValue::class, $this->multiValue[2]);
160-
$this->assertCount(1, $this->multiValue[2]);
161-
162-
// Check nested image.
163-
$this->assertCount(1, $this->multiValue[2]);
164-
$this->assertInstanceOf(Image::class, $this->multiValue[2][0]);
165-
166-
// Reset filters.
167-
$this->multiValue->resetFilters();
168-
169-
// Check nested image.
170-
$this->assertCount(3, $this->multiValue[6]);
171-
$this->assertInstanceOf(Image::class, $this->multiValue[6][2]);
172115
}
173116

174117
/**
@@ -187,51 +130,26 @@ public function testImmutableFilter()
187130

188131
/**
189132
* @covers ::immutable
190-
* @covers ::filterType
133+
* @covers ::allowType
191134
*
192135
* @expectedException \LogicException
193136
* @expectedExceptionMessage Attempting to filter immutable array
194137
*/
195-
public function testImmutableFilterType()
138+
public function testImmutableAllowType()
196139
{
197-
$this->multiValue->immutable()->filterType('string');
140+
$this->multiValue->immutable()->allowType('string');
198141
}
199142

200143
/**
201144
* @covers ::immutable
202-
* @covers ::filterInstance
145+
* @covers ::allowInstance
203146
*
204147
* @expectedException \LogicException
205148
* @expectedExceptionMessage Attempting to filter immutable array
206149
*/
207-
public function testImmutableFilterInstance()
150+
public function testImmutableAllowInstance()
208151
{
209-
$this->multiValue->immutable()->filterInstance(Image::class);
210-
}
211-
212-
/**
213-
* @covers ::immutable
214-
* @covers ::resetFilters
215-
*
216-
* @expectedException \LogicException
217-
* @expectedExceptionMessage Attempting to reset filters on immutable array
218-
*/
219-
public function testImmutableResetFilters()
220-
{
221-
// Apply image filter.
222-
$this->multiValue
223-
->filterInstance(Image::class);
224-
$this->assertCount(2, $this->multiValue);
225-
226-
// Reset filters.
227-
$this->multiValue->resetFilters();
228-
$this->assertCount(8, $this->multiValue);
229-
230-
// Make immutable.
231-
$this->multiValue->immutable();
232-
233-
// Filters cannot be reset.
234-
$this->multiValue->resetFilters();
152+
$this->multiValue->immutable()->allowInstance(Image::class);
235153
}
236154

237155
/**

‎tests/Util/Profile/AttributeConverterTest.php‎

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ public function testConvertToYotiAttributeMultiValue()
185185
$multiValue = $attr->getValue();
186186

187187
// Check top-level items.
188-
$this->assertCount(5, $multiValue);
188+
$this->assertCount(4, $multiValue);
189189
$this->assertInstanceOf(MultiValue::class, $multiValue);
190190

191191
$this->assertInstanceOf(Image::class, $multiValue[0]);
@@ -198,20 +198,48 @@ public function testConvertToYotiAttributeMultiValue()
198198

199199
$this->assertEquals('test string', $multiValue[2]);
200200

201-
$this->assertInstanceOf(MultiValue::class, $multiValue[4]);
201+
$this->assertInstanceOf(MultiValue::class, $multiValue[3]);
202202

203203
// Check nested items.
204-
$this->assertCount(4, $multiValue[4]);
204+
$this->assertCount(3, $multiValue[3]);
205205

206-
$this->assertInstanceOf(Image::class, $multiValue[4][0]);
207-
$this->assertEquals('image/jpeg', $multiValue[4][0]->getMimeType());
208-
$this->assertNotEmpty($multiValue[4][0]->getContent());
206+
$this->assertInstanceOf(Image::class, $multiValue[3][0]);
207+
$this->assertEquals('image/jpeg', $multiValue[3][0]->getMimeType());
208+
$this->assertNotEmpty($multiValue[3][0]->getContent());
209209

210-
$this->assertInstanceOf(Image::class, $multiValue[4][1]);
211-
$this->assertEquals('image/png', $multiValue[4][1]->getMimeType());
212-
$this->assertNotEmpty($multiValue[4][1]->getContent());
210+
$this->assertInstanceOf(Image::class, $multiValue[3][1]);
211+
$this->assertEquals('image/png', $multiValue[3][1]->getMimeType());
212+
$this->assertNotEmpty($multiValue[3][1]->getContent());
213213

214-
$this->assertEquals('test string', $multiValue[4][2]);
214+
$this->assertEquals('test string', $multiValue[3][2]);
215+
}
216+
217+
/**
218+
* Check that empty MultiValue Values result in no attribute being returned.
219+
*
220+
* @covers ::convertToYotiAttribute
221+
*/
222+
public function testEmptyAttributeMultiValueValue()
223+
{
224+
// Get MultiValue values.
225+
$values = $this->createMultiValueValues();
226+
227+
// Add an empty MultiValue.
228+
$values[] = $this->createMultiValueValue('', self::CONTENT_TYPE_STRING);
229+
230+
// Create top-level MultiValue.
231+
$protoMultiValue = new \Attrpubapi\MultiValue();
232+
$protoMultiValue->setValues($values);
233+
234+
// Create mock Attribute that will return MultiValue as the value.
235+
$protobufAttribute = $this->getMockForProtobufAttribute(
236+
'test_attr',
237+
$protoMultiValue->serializeToString(),
238+
self::CONTENT_TYPE_MULTI_VALUE
239+
);
240+
241+
$attr = AttributeConverter::convertToYotiAttribute($protobufAttribute);
242+
$this->assertNull($attr);
215243
}
216244

217245
/**
@@ -238,7 +266,6 @@ private function createMultiValueValues()
238266
['image 1', self::CONTENT_TYPE_JPEG],
239267
['image 2', self::CONTENT_TYPE_PNG],
240268
['test string', self::CONTENT_TYPE_STRING],
241-
['', self::CONTENT_TYPE_STRING],
242269
];
243270

244271
$items = [];

0 commit comments

Comments
 (0)