This repository was archived by the owner on Jan 31, 2022. It is now read-only.
Commit 0d05b04
authored
refactor(Query): Unify Boolean attributes' handling and testing (#552)
## Boolean attributes handling
Although we first thought it would consist in `Move Boolean to boolean in the client`, exploration showed that we need to stick to `Boolean` instead:
- Query's boolean attributes can either be _unset_, _true_ or _false_
- The default for each attribute is _not something we can rely on in the library_ (although sometimes it's mentioned in the docstring)
Consequently, we need our boolean getters to return either of three values: `Boolean.TRUE`, `Boolean.FALSE`, and `null` if unset.
This means **getters should be implemented as `public @nullable Boolean getXXX()`**.
Likewise, the user should be able to unset an attribute's value, putting the `Query` in its default state for that attribute.
This means **setters should be implemented as `public @nonnull Query setXXX(@nullable Boolean xxx);`**.
This is implemented in 530c577, which refactors setters that were not compliant with this reasoning; and 68a955c, which adds appropriate annotations.
## Boolean attributes testing
Testing of our boolean attributes was inconsistent, sometimes testing `TRUE` and `FALSE` cases and sometimes only one of them. Adding a test for the `null` case was an opportunity to refactor those.
I went the following way:
- Extract the Boolean testing logic into [`testBooleanAttribute`](https://github.com/algolia/algoliasearch-client-android/blob/39a4dee0c71eac1db2f8a4a6a53905f1713e9a73/algoliasearch/src/test/java/com/algolia/search/saas/QueryTest.java#L874-L916), a generic method testing all cases of Boolean values for a given attribute
- Refactor Boolean tests to always use `testBooleanAttribute`. This makes the tests much more concise, more consistent, without any significant performance drawback (runnning 5 times `QueryTest` before and after this change results in a mean difference of `0.12`s).
- Reorganize tests into [regions for attribute types](https://github.com/algolia/algoliasearch-client-android/blob/39a4dee0c71eac1db2f8a4a6a53905f1713e9a73/algoliasearch/src/test/java/com/algolia/search/saas/QueryTest.java#L116). If we like the benefits of this new structure, I would apply it to each other attribute type.
- For mixed type attributes that can be Boolean (currently `ignorePlurals`), add assertions for the `null` case and keep it unchanged until further refactoring.1 parent bab7e95 commit 0d05b04
File tree
3 files changed
+166
-176
lines changed- algoliasearch/src
- main/java/com/algolia/search/saas
- places
- test/java/com/algolia/search/saas
3 files changed
+166
-176
lines changedLines changed: 36 additions & 26 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
289 | 289 | | |
290 | 290 | | |
291 | 291 | | |
292 | | - | |
| 292 | + | |
293 | 293 | | |
294 | 294 | | |
295 | 295 | | |
296 | | - | |
| 296 | + | |
| 297 | + | |
297 | 298 | | |
298 | 299 | | |
299 | 300 | | |
| |||
304 | 305 | | |
305 | 306 | | |
306 | 307 | | |
307 | | - | |
| 308 | + | |
308 | 309 | | |
309 | 310 | | |
310 | 311 | | |
311 | | - | |
| 312 | + | |
| 313 | + | |
312 | 314 | | |
313 | 315 | | |
314 | 316 | | |
| |||
318 | 320 | | |
319 | 321 | | |
320 | 322 | | |
321 | | - | |
| 323 | + | |
322 | 324 | | |
323 | 325 | | |
324 | 326 | | |
325 | | - | |
| 327 | + | |
| 328 | + | |
326 | 329 | | |
327 | 330 | | |
328 | 331 | | |
| |||
333 | 336 | | |
334 | 337 | | |
335 | 338 | | |
336 | | - | |
| 339 | + | |
337 | 340 | | |
338 | 341 | | |
339 | 342 | | |
340 | | - | |
| 343 | + | |
| 344 | + | |
341 | 345 | | |
342 | 346 | | |
343 | 347 | | |
| |||
381 | 385 | | |
382 | 386 | | |
383 | 387 | | |
384 | | - | |
| 388 | + | |
385 | 389 | | |
386 | 390 | | |
387 | 391 | | |
388 | | - | |
| 392 | + | |
| 393 | + | |
389 | 394 | | |
390 | 395 | | |
391 | 396 | | |
| |||
611 | 616 | | |
612 | 617 | | |
613 | 618 | | |
614 | | - | |
| 619 | + | |
615 | 620 | | |
616 | 621 | | |
617 | 622 | | |
| |||
648 | 653 | | |
649 | 654 | | |
650 | 655 | | |
651 | | - | |
| 656 | + | |
652 | 657 | | |
653 | 658 | | |
654 | 659 | | |
655 | | - | |
| 660 | + | |
656 | 661 | | |
657 | 662 | | |
658 | 663 | | |
| |||
820 | 825 | | |
821 | 826 | | |
822 | 827 | | |
823 | | - | |
| 828 | + | |
824 | 829 | | |
825 | 830 | | |
826 | 831 | | |
| |||
1277 | 1282 | | |
1278 | 1283 | | |
1279 | 1284 | | |
1280 | | - | |
| 1285 | + | |
1281 | 1286 | | |
1282 | 1287 | | |
1283 | 1288 | | |
1284 | | - | |
| 1289 | + | |
1285 | 1290 | | |
1286 | 1291 | | |
1287 | 1292 | | |
| |||
1372 | 1377 | | |
1373 | 1378 | | |
1374 | 1379 | | |
1375 | | - | |
| 1380 | + | |
1376 | 1381 | | |
1377 | 1382 | | |
1378 | 1383 | | |
1379 | | - | |
| 1384 | + | |
| 1385 | + | |
1380 | 1386 | | |
1381 | 1387 | | |
1382 | 1388 | | |
| |||
1389 | 1395 | | |
1390 | 1396 | | |
1391 | 1397 | | |
1392 | | - | |
| 1398 | + | |
1393 | 1399 | | |
1394 | 1400 | | |
1395 | 1401 | | |
1396 | | - | |
| 1402 | + | |
| 1403 | + | |
1397 | 1404 | | |
1398 | 1405 | | |
1399 | 1406 | | |
| |||
1476 | 1483 | | |
1477 | 1484 | | |
1478 | 1485 | | |
1479 | | - | |
| 1486 | + | |
1480 | 1487 | | |
1481 | 1488 | | |
1482 | 1489 | | |
1483 | | - | |
| 1490 | + | |
| 1491 | + | |
1484 | 1492 | | |
1485 | 1493 | | |
1486 | 1494 | | |
| |||
1491 | 1499 | | |
1492 | 1500 | | |
1493 | 1501 | | |
1494 | | - | |
| 1502 | + | |
1495 | 1503 | | |
1496 | 1504 | | |
1497 | 1505 | | |
1498 | | - | |
| 1506 | + | |
| 1507 | + | |
1499 | 1508 | | |
1500 | 1509 | | |
1501 | 1510 | | |
| |||
1554 | 1563 | | |
1555 | 1564 | | |
1556 | 1565 | | |
1557 | | - | |
| 1566 | + | |
1558 | 1567 | | |
1559 | 1568 | | |
1560 | 1569 | | |
1561 | | - | |
| 1570 | + | |
| 1571 | + | |
1562 | 1572 | | |
1563 | 1573 | | |
1564 | 1574 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
115 | 115 | | |
116 | 116 | | |
117 | 117 | | |
118 | | - | |
| 118 | + | |
119 | 119 | | |
120 | 120 | | |
121 | 121 | | |
| |||
0 commit comments