-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
6 changed files
with
424 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
# Scenarios of failure | ||
|
||
Below are the known scenarios how evolutions of your code, if not mitigated, result in loss of data or degradation of performance. | ||
|
||
Module [examples](/examples) demonstrates these scenarios. The code contains `TODO [x]` items which, when commented or uncommented, simulate the evolution | ||
leading to a bug. After un-/commenting the line, rerun the unit tests to see which will detect the bug. | ||
|
||
------------------------------------------------------------------------------------------------------ | ||
|
||
## Incomplete copy constructor/builder | ||
|
||
**Risk:** data loss | ||
|
||
**Scenario:** When a new field is added to a class which has a copy constructor/copy builder without having the latter updated, this field will not be copied. | ||
|
||
**Demonstration:** Uncomment line `TODO [3]` to demonstrate this bug. | ||
|
||
**Mitigation:** build the copy and test it against the original field-by-field | ||
```java | ||
ImmutableEntity copy = new ImmutableEntity.Builder(original).build(); | ||
assertThat(copy).as("Copy builder is incomplete").usingRecursiveComparison().isEqualTo(original); | ||
``` | ||
|
||
**Example:** [CopyBuilderTest](/examples/src/test/java/com/github/neboskreb/red/and/blue/example/CopyBuilderTest.java) | ||
|
||
------------------------------------------------------------------------------------------------------ | ||
|
||
## Incomplete mapper | ||
|
||
**Risk:** data loss | ||
|
||
**Scenario:** When a generated mapper is not defined properly, or a manually written mapper is incomplete, some fields will not be copied. | ||
|
||
**Demonstration:** Comment line `TODO [1]` and/or `TODO [2]` to demonstrate this bug. | ||
|
||
**Mitigation:** use reflective mapping to obtain the copy and test it against the original field-by-field | ||
```java | ||
Entity copy = mapper.toEntity(mapper.toDTO(entity)); | ||
assertThat(copy).as("Mapper is incomplete").usingRecursiveComparison().isEqualTo(entity); | ||
``` | ||
|
||
**Example:** [MapperTest](/examples/src/test/java/com/github/neboskreb/red/and/blue/example/MapperTest.java) | ||
|
||
------------------------------------------------------------------------------------------------------ | ||
|
||
## Incomplete DTO / DAO | ||
|
||
**Risk:** data loss | ||
|
||
**Scenario:** When a field is added to the Entity object but the DTO/DAO is not updated accordingly, this field will not be transferred/stored. | ||
|
||
**Demonstration:** Comment line `TODO [6]` to demonstrate this bug. | ||
|
||
**Mitigation:** use reflective mapping to obtain the copy and test it against the original field-by-field | ||
```java | ||
Entity copy = mapper.toEntity(mapper.toDTO(entity)); | ||
assertThat(copy).as("DTO is incomplete").usingRecursiveComparison().isEqualTo(entity); | ||
``` | ||
|
||
**Example:** [MapperTest](/examples/src/test/java/com/github/neboskreb/red/and/blue/example/MapperTest.java) | ||
|
||
------------------------------------------------------------------------------------------------------ | ||
|
||
## Excessive DTO | ||
|
||
**Risk:** performance degradation | ||
|
||
**Scenario:** In a client-server setting, when a field is removed from the Entity object on the client's end but the DTO is not updated accordingly, | ||
the server will still populate this field, effectively wasting the CPU cycles and the bandwidth. | ||
|
||
**Demonstration:** Uncomment line `TODO [5]` to demonstrate this bug. | ||
|
||
**Mitigation:** use reflective mapping to obtain the copy of DTO and test it against the original DTO field-by-field | ||
```java | ||
EntityDTO dtoCopy = mapper.toDTO(mapper.toEntity(dto)); | ||
assertThat(dtoCopy).as("DTO is excessive").usingRecursiveComparison().isEqualTo(dto); | ||
``` | ||
|
||
**Example:** [MapperTest](/examples/src/test/java/com/github/neboskreb/red/and/blue/example/MapperTest.java) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
# Example of code evolution | ||
|
||
Assume you have class A, and a new field `newField` is added. But due to a human mistake, only the normal constructor is updated but the copy constructor is not: | ||
```java | ||
public class A { | ||
private int oldField; | ||
private boolean newField; | ||
|
||
public A(int oldField, boolean newField) { | ||
this.oldField = oldField; | ||
this.newField = newField; | ||
} | ||
|
||
/** Copy constructor. */ | ||
public A(A original) { | ||
this.oldField = original.oldField; | ||
// Note that `newField` is not copied. It's a bug! | ||
} | ||
|
||
@Override | ||
public final boolean equals(Object other) { | ||
if (this == other) return true; | ||
if (!(other instanceof A otherA)) return false; | ||
|
||
return oldField == otherA.oldField && newField == otherA.newField; | ||
} | ||
} | ||
``` | ||
|
||
If your test has only one check, there is a big risk such bug will go undetected: | ||
```java | ||
@Test | ||
void incorrect() { | ||
// GIVEN | ||
A original = new A(1, false); | ||
|
||
// WHEN | ||
A copy = new A(original); | ||
|
||
// THEN | ||
assertEquals(original, copy); | ||
} | ||
``` | ||
|
||
You need two checks to reliably detect this bug: | ||
```java | ||
@Test | ||
void correct() { | ||
// GIVEN | ||
A originalRed = new A(1, false); | ||
A originalBlue = new A(2, true); | ||
|
||
// WHEN | ||
A copyRed = new A(originalRed); | ||
A copyBlue = new A(originalBlue); | ||
|
||
// THEN | ||
assertEquals(originalRed, copyRed); | ||
assertEquals(originalBlue, copyBlue); | ||
} | ||
``` | ||
|
||
So you need two objects having every field different from its counterpart. Such two objects are known as "Red" and "Blue" objects. Each field in Red object is non-equal to this field in Blue object. | ||
|
||
This extension offers an easy way of creating the Red and Blue objects of arbitrary class. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
|
||
# How the data loss occurs | ||
|
||
In the beginning, all works well and no data is lost. But no code is carved in stone - after written, it evolves and mutates as the project develops. | ||
Sometimes a seemingly benign change to a data object, like an added field, can result in data loss or performance degradation. Not every such case is | ||
easily detected by the unit tests. | ||
|
||
One infamous example is the `equals` method: adding a new field to the class without having the `equals` updated is notoriously | ||
difficult to catch. Even if your test manually checks each field it will stay green - thus failing to detect the bug. | ||
This happens because your test simply doesn't know that something was added, hence doesn't check it! | ||
For this reason, libraries like EqualsVerifier were created to automatically find and check fields. | ||
|
||
# Path to the pitfall | ||
|
||
But `equals` is not the only scenario. Other areas of risk include: | ||
* builders | ||
* mappers | ||
* copiers | ||
* mutator methods | ||
* etc. | ||
|
||
All of the above can result in a data loss if the bug is not caught in time. | ||
|
||
**Data loss is not a joke. You want to be protected against it.** | ||
|
||
# Solution | ||
|
||
Cover your mappers/builders/etc with unit tests. | ||
|
||
For this you will need two instances of each class under test. Yes, two - "Red" and "Blue". | ||
|
||
# Why one simple `equals` check is not enough? | ||
|
||
Because `equals` only tests that fields are equal but doesn't assure that the field has been copied. If in your test the original's | ||
field had default value (i.e., 0 for `int` or `false` for `boolean`) the `equals` doesn't see a fault because the copy's field is | ||
initialized to the same default value, so they match. | ||
|
||
To catch this bug you need to check the filed **two times** with different values: this way either the first or second check will | ||
catch the discrepancy (because a type can have only one default value). | ||
|
||
Here is the [example](faulty-example.md) demonstrating how the single check fails but the dual check wins. | ||
|
||
**One check is not enough. You need two.** |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
|
||
# What are "Red" and "Blue" objects? | ||
|
||
Objects known as "Red" and "Blue" are two instances of the same class which have each field different from the same field in their counterpart. | ||
|
||
E.g. if class contains two String fields and one int field, like this, | ||
```java | ||
public class A { | ||
private String first; | ||
private String second; | ||
private int third; | ||
} | ||
``` | ||
then the red and blue instances could be like these: | ||
<pre><code> | ||
| red | blue | | ||
|----------------------|----------------------| | ||
| { | { | | ||
| "first": "one", | "first": "two", | | ||
| "second": "one", | "second": "two", | | ||
| "third": 1000 | "third": 2000 | | ||
| } | } | | ||
</code></pre> | ||
|
||
_NOTE that fields `first` and `second` are not required to differ from each other in the same instance._ | ||
|
||
|
||
|
Oops, something went wrong.