Adding owner support to ClientDAO #1167
Conversation
| @JsonProperty | ||
| private final String owner; | ||
| private String owner; |
There was a problem hiding this comment.
Same as the last PR: making the owner field writable simplifies the DAO layer code.
| private Optional<Client> getClient(Condition condition) { | ||
| Record record = dslContext | ||
| .select(CLIENTS.fields()) | ||
| .select(CLIENT_OWNERS.NAME) | ||
| .from(CLIENTS) | ||
| .leftJoin(CLIENT_OWNERS) | ||
| .on(CLIENTS.OWNER.eq(CLIENT_OWNERS.ID)) | ||
| .where(condition) |
There was a problem hiding this comment.
I got a little fancier than the GroupDAO PR by parameterizing the where clause.
| @Test public void deleteSetsClientOwnerToNull() { | ||
| String ownerName = randomName(); | ||
| long ownerId = createGroup(ownerName, NO_OWNER); | ||
|
|
||
| String clientName = randomName(); | ||
| long clientId = createClient(clientName, ownerId); | ||
|
|
||
| Client beforeWithOwner = getClientById(clientId); | ||
| assertEquals(ownerName, beforeWithOwner.getOwner()); | ||
|
|
||
| Group owner = getGroupById(ownerId); | ||
| groupDAO.deleteGroup(owner); | ||
|
|
||
| Client afterWithoutOwner = getClientById(clientId); | ||
| assertNull(afterWithoutOwner.getOwner()); | ||
| } |
There was a problem hiding this comment.
It turns out that this test is unnecessary. Because we do a left join on the owning group and then only populate the owner name if the owner is present, a "dangling" owner ID field doesn't cause a problem. Of course, that's bad, because it masks an underlying issue. I think the solution is to add a sanity check in the record conversion code that will barf if the owner ID is present but the owning group is not.
| boolean danglingOwner = clientRecord.getOwner() != null && ownerRecord.getId() == null; | ||
| if (danglingOwner) { | ||
| throw new IllegalStateException( | ||
| String.format( | ||
| "Owner %s for client %s is missing.", | ||
| clientRecord.getOwner(), | ||
| clientRecord.getName())); | ||
| } |
There was a problem hiding this comment.
This logic fixes a bug I'd discussed in another comment: the left join will present a null owner name if the owner is missing regardless of whether the owner ID in the client is populated or not. Because we don't enforce referential integrity in the database, we have to hand-manage it here.
Co-authored-by: Spencer McCreary <spencer_mccreary@compulsivesoftware.com>
Co-authored-by: Spencer McCreary <spencer_mccreary@compulsivesoftware.com>
No description provided.