Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete bills #8

Merged
merged 14 commits into from
Feb 11, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions core/src/main/java/io/nobt/core/UnknownExpenseException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package io.nobt.core;

public class UnknownExpenseException extends RuntimeException {

}
19 changes: 17 additions & 2 deletions core/src/main/java/io/nobt/core/domain/Nobt.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.nobt.core.domain;

import io.nobt.core.UnknownExpenseException;
import io.nobt.core.domain.debt.Debt;
import io.nobt.core.optimizer.Optimizer;

Expand All @@ -18,16 +19,18 @@ public class Nobt {
private final String name;
private final Set<Person> explicitParticipants;
private final Set<Expense> expenses;
private final Set<Expense> deletedExpenses;
private final Set<Payment> payments;
private final ZonedDateTime createdOn;
private final Optimizer optimizer;

public Nobt(NobtId id, CurrencyKey currencyKey, String name, Set<Person> explicitParticipants, Set<Expense> expenses, Set<Payment> payments, ZonedDateTime createdOn, Optimizer optimizer) {
public Nobt(NobtId id, CurrencyKey currencyKey, String name, Set<Person> explicitParticipants, Set<Expense> expenses, Set<Expense> deletedExpenses, Set<Payment> payments, ZonedDateTime createdOn, Optimizer optimizer) {
this.id = id;
this.currencyKey = currencyKey;
this.name = name;
this.explicitParticipants = new HashSet<>(explicitParticipants);
this.expenses = new HashSet<>(expenses);
this.deletedExpenses = new HashSet<>(deletedExpenses);
this.payments = new HashSet<>(payments);
this.createdOn = createdOn;
this.optimizer = optimizer;
Expand Down Expand Up @@ -57,6 +60,10 @@ public Set<Expense> getExpenses() {
return Collections.unmodifiableSet(expenses);
}

public Set<Expense> getDeletedExpenses() {
return Collections.unmodifiableSet(deletedExpenses);
}

public Set<Payment> getPayments() {
return payments;
}
Expand Down Expand Up @@ -118,6 +125,14 @@ private long getNextIdentifier() {
}

public void removeExpense(long expenseId) {
this.expenses.removeIf(e -> e.getId() == expenseId);

Expense expenseToDelete = expenses.stream()
.filter(e -> e.getId() == expenseId)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funktioniert der Referenzvergleich hier?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ja, ist ein primitive type

.findFirst()
.orElseThrow(UnknownExpenseException::new);

expenses.remove(expenseToDelete);
deletedExpenses.add(expenseToDelete);
}

}
2 changes: 1 addition & 1 deletion core/src/main/java/io/nobt/core/domain/NobtFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ public NobtFactory(Supplier<Optimizer> optimizerFactory, Clock clock) {
}

public Nobt create(String name, Set<Person> explicitParticipants, CurrencyKey currencyKey) {
return new Nobt(NobtId.newInstance(), currencyKey, name, explicitParticipants, emptySet(), emptySet(), ZonedDateTime.now(clock), optimizerFactory.get());
return new Nobt(NobtId.newInstance(), currencyKey, name, explicitParticipants, emptySet(), emptySet(), emptySet(), ZonedDateTime.now(clock), optimizerFactory.get());
}
}
4 changes: 2 additions & 2 deletions core/src/test/java/io/nobt/core/domain/NobtTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@
import static io.nobt.test.domain.factories.StaticPersonFactory.david;
import static io.nobt.test.domain.factories.StaticPersonFactory.thomas;
import static io.nobt.test.domain.matchers.ExpenseMatchers.hasId;
import static io.nobt.test.domain.matchers.NobtMatchers.hasExpenses;
import static io.nobt.test.domain.matchers.NobtMatchers.hasPayments;
import static io.nobt.test.domain.matchers.NobtMatchers.*;
import static io.nobt.test.domain.matchers.PaymentMatchers.*;
import static io.nobt.test.domain.provider.ExpenseBuilderProvider.anExpense;
import static io.nobt.test.domain.provider.ExpenseDraftBuilderProvider.anExpenseDraft;
Expand Down Expand Up @@ -163,6 +162,7 @@ public void shouldRemoveExpenseById() throws Exception {


assertThat(nobt, hasExpenses(iterableWithSize(0)));
assertThat(nobt, hasDeletedExpenses(iterableWithSize(1)));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
public class NobtBuilder {

private Set<Expense> expenses;
private Set<Expense> deletedExpenses;
private Set<Person> participants;
private Set<Payment> payments;
private ZonedDateTime dateTime;
Expand All @@ -33,6 +34,19 @@ public NobtBuilder withExpenses(ExpenseBuilder... expenseBuilders) {
return withExpenses(Arrays.stream(expenseBuilders).map(ExpenseBuilder::build).collect(toSet()));
}

public NobtBuilder withDeletedExpenses(Set<Expense> expenses) {
this.deletedExpenses = expenses;
return this;
}

public NobtBuilder withDeletedExpenses(Expense... expenses) {
return withDeletedExpenses(Arrays.stream(expenses).collect(toSet()));
}

public NobtBuilder withDeletedExpenses(ExpenseBuilder... expenseBuilders) {
return withDeletedExpenses(Arrays.stream(expenseBuilders).map(ExpenseBuilder::build).collect(toSet()));
}

public NobtBuilder withPayments(PaymentBuilder... payments) {
return withPayments(Arrays.stream(payments).map(PaymentBuilder::build).collect(toSet()));
}
Expand Down Expand Up @@ -87,6 +101,7 @@ public Nobt build() {
name,
participants,
expenses,
deletedExpenses,
payments,
dateTime,
optimizer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,15 @@ protected Set<Expense> featureValueOf(Nobt actual) {
};
}

public static Matcher<Nobt> hasDeletedExpenses(final Matcher<? super Iterable<Expense>> subMatcher) {
return new FeatureMatcher<Nobt, Set<Expense>>(subMatcher, "deletedExpenses", "deletedExpenses") {
@Override
protected Set<Expense> featureValueOf(Nobt actual) {
return actual.getDeletedExpenses();
}
};
}

public static Matcher<Nobt> hasPayments(final Matcher<? super Iterable<Payment>> subMatcher) {
return new FeatureMatcher<Nobt, Set<Payment>>(subMatcher, "payments", "payments") {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public static NobtBuilder aNobt() {
return new NobtBuilder()
.withId(NobtId.newInstance())
.withExpenses(emptySet())
.withDeletedExpenses(emptySet())
.withParticipants(emptySet())
.withPayments(emptySet())
.withCurrency(new CurrencyKey("EUR"))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE expenses
ADD deleted BOOLEAN;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sollten wir das vielleicht NOT NULL machen und alle anderen explizit auf false setzen?


Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,17 @@ public void savingAndFetchingResultsInDifferentInstance() throws Exception {
}

@Test
public void shouldRemoveOrphanExpense() throws Exception {
public void shouldDeleteExpense() throws Exception {

final Share thomasShare = ShareFactory.randomShare(thomas);
final Share matthiasShare = ShareFactory.randomShare(matthias);
final LocalDate expenseDate = LocalDate.now();

final Nobt nobtToSave = aNobt()
.withExpenses(anExpense().withDebtee(thomas).withShares(thomasShare, matthiasShare).happendOn(expenseDate))
.withExpenses(anExpense()
.withDebtee(thomas)
.withShares(thomasShare, matthiasShare)
.happendOn(expenseDate))
.build();

final NobtId id = save(nobtToSave);
Expand All @@ -156,12 +159,14 @@ public void shouldRemoveOrphanExpense() throws Exception {

save(retrievedNobt);


final Nobt nobtWithoutExpense = fetch(id);

assertThat(nobtWithoutExpense, hasExpenses(
iterableWithSize(0)
));
assertThat(nobtWithoutExpense, hasDeletedExpenses(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die beiden Assertions kannst du zusammenfassen auf ein: allOf(hasExpenses(...), hasDeletedExpenses(...))

iterableWithSize(1)
));
}

@Test
Expand Down Expand Up @@ -211,4 +216,5 @@ private NobtId save(Nobt nobtToSave) {
private Nobt fetch(NobtId id) {
return invoker.invoke(new RetrieveNobtCommand(id));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ public class ExpenseEntity extends CashFlowEntity {
@Column(name = "date", nullable = false)
private LocalDate date;

@Column(name = "deleted", nullable = false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wenns hier nullable=false ist, sollte das auch im Migrationskript so in die DB geschrieben werden. Ich würde vielleicht auch insertable=false setzen. Ein hinzufügen einer gelöschten Rechnung macht keinen Sinn.

private boolean deleted;

public String getName() {
return name;
}
Expand Down Expand Up @@ -93,4 +96,15 @@ public void setDate(LocalDate date) {
this.date = date;
}

public boolean isDeleted() {
return deleted;
}

public void setDeleted(boolean deleted) {
this.deleted = deleted;
}

public boolean isNotDeleted() {
return !isDeleted();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,5 @@ public ExpenseEntity mapToDatabaseModel(Expense domainModel) {

return expense;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class NobtEntity {
@Column(name = "createdOn", nullable = false, updatable = false)
private ZonedDateTime createdOn;

@OneToMany(fetch = FetchType.EAGER, mappedBy = "nobt", cascade = CascadeType.ALL, orphanRemoval = true)
@OneToMany(fetch = FetchType.EAGER, mappedBy = "nobt", cascade = CascadeType.ALL)
private Set<ExpenseEntity> expenses = new HashSet<>();

@OneToMany(fetch = FetchType.EAGER, mappedBy = "nobt", cascade = CascadeType.ALL, orphanRemoval = true)
Expand Down
22 changes: 19 additions & 3 deletions persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ public class NobtMapper implements DomainModelMapper<NobtEntity, Nobt> {
private final DomainModelMapper<ExpenseEntity, Expense> expenseMapper;
private final DomainModelMapper<PaymentEntity, Payment> paymentMapper;

public NobtMapper(NobtDatabaseIdResolver nobtDatabaseIdResolver, DomainModelMapper<ExpenseEntity, Expense> expenseMapper, DomainModelMapper<PaymentEntity, Payment> paymentMapper) {
public NobtMapper(NobtDatabaseIdResolver nobtDatabaseIdResolver,
DomainModelMapper<ExpenseEntity, Expense> expenseMapper,
DomainModelMapper<PaymentEntity, Payment> paymentMapper) {
this.nobtDatabaseIdResolver = nobtDatabaseIdResolver;
this.expenseMapper = expenseMapper;
this.paymentMapper = paymentMapper;
Expand All @@ -26,15 +28,25 @@ public NobtMapper(NobtDatabaseIdResolver nobtDatabaseIdResolver, DomainModelMapp
public Nobt mapToDomainModel(NobtEntity databaseModel) {

final Set<Person> explicitParticipants = databaseModel.getExplicitParticipants();
final Set<Expense> expenses = databaseModel.getExpenses().stream().map(expenseMapper::mapToDomainModel).collect(toSet());
final Set<Payment> payments = databaseModel.getPayments().stream().map(paymentMapper::mapToDomainModel).collect(toSet());
final Set<Expense> expenses = databaseModel.getExpenses().stream()
.filter(ExpenseEntity::isNotDeleted)
.map(expenseMapper::mapToDomainModel)
.collect(toSet());
final Set<Expense> deletedExpenses = databaseModel.getExpenses().stream()
.filter(ExpenseEntity::isDeleted)
.map(expenseMapper::mapToDomainModel)
.collect(toSet());
final Set<Payment> payments = databaseModel.getPayments().stream()
.map(paymentMapper::mapToDomainModel)
.collect(toSet());

return new Nobt(
new NobtId(databaseModel.getExternalId()),
new CurrencyKey(databaseModel.getCurrency()),
databaseModel.getName(),
explicitParticipants,
expenses,
deletedExpenses,
payments,
databaseModel.getCreatedOn(),
databaseModel.getOptimizer()
Expand All @@ -60,6 +72,10 @@ public NobtEntity mapToDatabaseModel(Nobt domainModel) {
nobtEntity.setExplicitParticipant(domainModel.getParticipatingPersons());

domainModel.getExpenses().stream().map(expenseMapper::mapToDatabaseModel).forEach(nobtEntity::addExpense);
domainModel.getDeletedExpenses().stream()
.map(expenseMapper::mapToDatabaseModel)
.peek(e -> e.setDeleted(true))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Da wir eine Expense eigentlich nie mehr auf "nicht deleted" setzen wollen, könnte man in der Expense-Entity eine Methode anbieten die heißt markDeleted() oder so ähnlich und diesen boolean gar nie nach außen preisgibt. Dann kannst du hier auch ne Methodenreferenz verwenden. Kommt halt darauf an, wieviele solche "Convenience"-Methoden man in seiner Entity haben will.

.forEach(nobtEntity::addExpense);
domainModel.getPayments().stream().map(paymentMapper::mapToDatabaseModel).forEach(nobtEntity::addPayment);

return nobtEntity;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,61 @@ public void shouldAddNewExpense() throws Exception {
.statusCode(201);
}

@Test
public void shouldDeleteExpense() throws Exception {

final String nobtId = client.createGrillfeierNobt();
client.addFleischExpense(nobtId);

final Long idOfFirstExpense = client.getNobt(nobtId).jsonPath().getLong("expenses[0].id");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Das hier sollte dann den Link aus der Response holen und nicht die ID, sobald der Link mal zurückgeliefert wird.



given(this.documentationSpec)
.port(config.port())
.filter(
document("delete-expense",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Für dieses Snippet müsstest du im index.adoc File noch eine Sektion hinzufügen, wo du es wirklich inkludierst und die eigentliche Doku schreibst.

preprocessRequest(
configureHost(),
replaceLocalhost()
),
preprocessResponse(
replaceLocalhost()
)
)
)

.when()

.delete("/nobts/{nobtId}/expenses/{expenseId}", nobtId, idOfFirstExpense)

.then()

.statusCode(204);


client.getNobt(nobtId)
.then()
.body("expenses", response -> hasSize(0));
}

@Test
public void deletingExpenseThatDoesNotExistRespondsWith404() throws Exception {

final String nobtId = client.createGrillfeierNobt();
client.addFleischExpense(nobtId);

given(this.documentationSpec)
.port(config.port())

.when()

.delete("/nobts/{nobtId}/expenses/{expenseId}", nobtId, 104)

.then()

.statusCode(404);
}

@Test
public void shouldAddNewPayment() throws Exception {

Expand Down Expand Up @@ -245,6 +300,7 @@ public void shouldGetCompleteNobt() throws Exception {
fieldWithPath("createdOn").type(JsonFieldType.STRING).description("An ISO6801-compliant timestamp when the nobt was created."),

fieldWithPath("expenses").type(JsonFieldType.ARRAY).description("All expenses associated with this nobt."),
fieldWithPath("deletedExpenses").type(JsonFieldType.ARRAY).description("All deleted expenses associated with this nobt."),
fieldWithPath("expenses[].id").type(JsonFieldType.NUMBER).description("The id of the expense."),
fieldWithPath("expenses[].createdOn").type(JsonFieldType.STRING).description("An ISO6801-compliant timestamp when the expense was created."),
fieldWithPath("expenses[].date").type(JsonFieldType.STRING).description("The given date of the expense."),
Expand Down
Loading