From 66040bb72fc398c53845df0724ac75a1155f11aa Mon Sep 17 00:00:00 2001 From: Matthias Kreuzriegler Date: Sun, 7 Jan 2018 18:26:23 +0100 Subject: [PATCH 01/14] Introduce UnknownExpenseExcpetion If an expense cannot be found within a nobt the UnknownExpenseException can be thrown --- core/src/main/java/io/nobt/core/UnknownExpenseException.java | 5 +++++ 1 file changed, 5 insertions(+) create mode 100755 core/src/main/java/io/nobt/core/UnknownExpenseException.java diff --git a/core/src/main/java/io/nobt/core/UnknownExpenseException.java b/core/src/main/java/io/nobt/core/UnknownExpenseException.java new file mode 100755 index 00000000..16c1094c --- /dev/null +++ b/core/src/main/java/io/nobt/core/UnknownExpenseException.java @@ -0,0 +1,5 @@ +package io.nobt.core; + +public class UnknownExpenseException extends RuntimeException { + +} From f1e4ea11d040fa13f1d65ec185e701bef71634fa Mon Sep 17 00:00:00 2001 From: Matthias Kreuzriegler Date: Sun, 7 Jan 2018 22:52:24 +0100 Subject: [PATCH 02/14] Make expenses deletable To remove an expense from a nobt a new Enpoint is provided --- .../main/java/io/nobt/core/domain/Nobt.java | 19 ++++++- .../java/io/nobt/core/domain/NobtFactory.java | 2 +- .../java/io/nobt/core/domain/NobtTest.java | 4 +- .../nobt/test/domain/builder/NobtBuilder.java | 15 +++++ .../test/domain/matchers/NobtMatchers.java | 9 +++ .../domain/provider/NobtBuilderProvider.java | 1 + .../db/migration/V2__add_deleted_flag.sql | 3 + .../EntityManagerNobtRepositoryIT.java | 12 +++- .../cashflow/expense/ExpenseEntity.java | 14 +++++ .../cashflow/expense/ExpenseMapper.java | 1 + .../io/nobt/persistence/nobt/NobtEntity.java | 2 +- .../io/nobt/persistence/nobt/NobtMapper.java | 22 +++++++- .../io/nobt/rest/ApiDocumentationTest.java | 56 +++++++++++++++++++ .../main/java/io/nobt/rest/NobtRestApi.java | 42 +++++++++++--- .../io/nobt/rest/json/nobt/NobtMixin.java | 11 ++-- 15 files changed, 189 insertions(+), 24 deletions(-) create mode 100755 db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql diff --git a/core/src/main/java/io/nobt/core/domain/Nobt.java b/core/src/main/java/io/nobt/core/domain/Nobt.java index d0fab7ff..f466162e 100755 --- a/core/src/main/java/io/nobt/core/domain/Nobt.java +++ b/core/src/main/java/io/nobt/core/domain/Nobt.java @@ -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; @@ -18,16 +19,18 @@ public class Nobt { private final String name; private final Set explicitParticipants; private final Set expenses; + private final Set deletedExpenses; private final Set payments; private final ZonedDateTime createdOn; private final Optimizer optimizer; - public Nobt(NobtId id, CurrencyKey currencyKey, String name, Set explicitParticipants, Set expenses, Set payments, ZonedDateTime createdOn, Optimizer optimizer) { + public Nobt(NobtId id, CurrencyKey currencyKey, String name, Set explicitParticipants, Set expenses, Set deletedExpenses, Set 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; @@ -57,6 +60,10 @@ public Set getExpenses() { return Collections.unmodifiableSet(expenses); } + public Set getDeletedExpenses() { + return Collections.unmodifiableSet(deletedExpenses); + } + public Set getPayments() { return payments; } @@ -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) + .findFirst() + .orElseThrow(UnknownExpenseException::new); + + expenses.remove(expenseToDelete); + deletedExpenses.add(expenseToDelete); } + } \ No newline at end of file diff --git a/core/src/main/java/io/nobt/core/domain/NobtFactory.java b/core/src/main/java/io/nobt/core/domain/NobtFactory.java index ae692fd1..4572a381 100755 --- a/core/src/main/java/io/nobt/core/domain/NobtFactory.java +++ b/core/src/main/java/io/nobt/core/domain/NobtFactory.java @@ -24,6 +24,6 @@ public NobtFactory(Supplier optimizerFactory, Clock clock) { } public Nobt create(String name, Set 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()); } } diff --git a/core/src/test/java/io/nobt/core/domain/NobtTest.java b/core/src/test/java/io/nobt/core/domain/NobtTest.java index 9ffc40a0..5aa0c87a 100755 --- a/core/src/test/java/io/nobt/core/domain/NobtTest.java +++ b/core/src/test/java/io/nobt/core/domain/NobtTest.java @@ -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; @@ -163,6 +162,7 @@ public void shouldRemoveExpenseById() throws Exception { assertThat(nobt, hasExpenses(iterableWithSize(0))); + assertThat(nobt, hasDeletedExpenses(iterableWithSize(1))); } @Test diff --git a/core/src/testSupport/java/io/nobt/test/domain/builder/NobtBuilder.java b/core/src/testSupport/java/io/nobt/test/domain/builder/NobtBuilder.java index c5320cef..38cbc787 100644 --- a/core/src/testSupport/java/io/nobt/test/domain/builder/NobtBuilder.java +++ b/core/src/testSupport/java/io/nobt/test/domain/builder/NobtBuilder.java @@ -12,6 +12,7 @@ public class NobtBuilder { private Set expenses; + private Set deletedExpenses; private Set participants; private Set payments; private ZonedDateTime dateTime; @@ -33,6 +34,19 @@ public NobtBuilder withExpenses(ExpenseBuilder... expenseBuilders) { return withExpenses(Arrays.stream(expenseBuilders).map(ExpenseBuilder::build).collect(toSet())); } + public NobtBuilder withDeletedExpenses(Set 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())); } @@ -87,6 +101,7 @@ public Nobt build() { name, participants, expenses, + deletedExpenses, payments, dateTime, optimizer diff --git a/core/src/testSupport/java/io/nobt/test/domain/matchers/NobtMatchers.java b/core/src/testSupport/java/io/nobt/test/domain/matchers/NobtMatchers.java index 41566752..d6ff46b6 100644 --- a/core/src/testSupport/java/io/nobt/test/domain/matchers/NobtMatchers.java +++ b/core/src/testSupport/java/io/nobt/test/domain/matchers/NobtMatchers.java @@ -63,6 +63,15 @@ protected Set featureValueOf(Nobt actual) { }; } + public static Matcher hasDeletedExpenses(final Matcher> subMatcher) { + return new FeatureMatcher>(subMatcher, "deletedExpenses", "deletedExpenses") { + @Override + protected Set featureValueOf(Nobt actual) { + return actual.getDeletedExpenses(); + } + }; + } + public static Matcher hasPayments(final Matcher> subMatcher) { return new FeatureMatcher>(subMatcher, "payments", "payments") { @Override diff --git a/core/src/testSupport/java/io/nobt/test/domain/provider/NobtBuilderProvider.java b/core/src/testSupport/java/io/nobt/test/domain/provider/NobtBuilderProvider.java index 0044a348..7f2331ff 100644 --- a/core/src/testSupport/java/io/nobt/test/domain/provider/NobtBuilderProvider.java +++ b/core/src/testSupport/java/io/nobt/test/domain/provider/NobtBuilderProvider.java @@ -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")) diff --git a/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql b/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql new file mode 100755 index 00000000..b7df217d --- /dev/null +++ b/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql @@ -0,0 +1,3 @@ +ALTER TABLE expenses + ADD deleted BOOLEAN; + diff --git a/persistence/src/integrationTest/java/io/nobt/persistence/repository/EntityManagerNobtRepositoryIT.java b/persistence/src/integrationTest/java/io/nobt/persistence/repository/EntityManagerNobtRepositoryIT.java index c05eb183..708ef4e7 100644 --- a/persistence/src/integrationTest/java/io/nobt/persistence/repository/EntityManagerNobtRepositoryIT.java +++ b/persistence/src/integrationTest/java/io/nobt/persistence/repository/EntityManagerNobtRepositoryIT.java @@ -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); @@ -156,12 +159,14 @@ public void shouldRemoveOrphanExpense() throws Exception { save(retrievedNobt); - final Nobt nobtWithoutExpense = fetch(id); assertThat(nobtWithoutExpense, hasExpenses( iterableWithSize(0) )); + assertThat(nobtWithoutExpense, hasDeletedExpenses( + iterableWithSize(1) + )); } @Test @@ -211,4 +216,5 @@ private NobtId save(Nobt nobtToSave) { private Nobt fetch(NobtId id) { return invoker.invoke(new RetrieveNobtCommand(id)); } + } diff --git a/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseEntity.java b/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseEntity.java index e9666014..194fd6ae 100644 --- a/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseEntity.java +++ b/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseEntity.java @@ -37,6 +37,9 @@ public class ExpenseEntity extends CashFlowEntity { @Column(name = "date", nullable = false) private LocalDate date; + @Column(name = "deleted", nullable = false) + private boolean deleted; + public String getName() { return name; } @@ -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(); + } } \ No newline at end of file diff --git a/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseMapper.java b/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseMapper.java index 8cf635e7..ee4aa5d9 100644 --- a/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseMapper.java +++ b/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseMapper.java @@ -52,4 +52,5 @@ public ExpenseEntity mapToDatabaseModel(Expense domainModel) { return expense; } + } diff --git a/persistence/src/main/java/io/nobt/persistence/nobt/NobtEntity.java b/persistence/src/main/java/io/nobt/persistence/nobt/NobtEntity.java index d7b45718..721ab914 100644 --- a/persistence/src/main/java/io/nobt/persistence/nobt/NobtEntity.java +++ b/persistence/src/main/java/io/nobt/persistence/nobt/NobtEntity.java @@ -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 expenses = new HashSet<>(); @OneToMany(fetch = FetchType.EAGER, mappedBy = "nobt", cascade = CascadeType.ALL, orphanRemoval = true) diff --git a/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java b/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java index 17f81d70..84acefbe 100644 --- a/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java +++ b/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java @@ -16,7 +16,9 @@ public class NobtMapper implements DomainModelMapper { private final DomainModelMapper expenseMapper; private final DomainModelMapper paymentMapper; - public NobtMapper(NobtDatabaseIdResolver nobtDatabaseIdResolver, DomainModelMapper expenseMapper, DomainModelMapper paymentMapper) { + public NobtMapper(NobtDatabaseIdResolver nobtDatabaseIdResolver, + DomainModelMapper expenseMapper, + DomainModelMapper paymentMapper) { this.nobtDatabaseIdResolver = nobtDatabaseIdResolver; this.expenseMapper = expenseMapper; this.paymentMapper = paymentMapper; @@ -26,8 +28,17 @@ public NobtMapper(NobtDatabaseIdResolver nobtDatabaseIdResolver, DomainModelMapp public Nobt mapToDomainModel(NobtEntity databaseModel) { final Set explicitParticipants = databaseModel.getExplicitParticipants(); - final Set expenses = databaseModel.getExpenses().stream().map(expenseMapper::mapToDomainModel).collect(toSet()); - final Set payments = databaseModel.getPayments().stream().map(paymentMapper::mapToDomainModel).collect(toSet()); + final Set expenses = databaseModel.getExpenses().stream() + .filter(ExpenseEntity::isNotDeleted) + .map(expenseMapper::mapToDomainModel) + .collect(toSet()); + final Set deletedExpenses = databaseModel.getExpenses().stream() + .filter(ExpenseEntity::isDeleted) + .map(expenseMapper::mapToDomainModel) + .collect(toSet()); + final Set payments = databaseModel.getPayments().stream() + .map(paymentMapper::mapToDomainModel) + .collect(toSet()); return new Nobt( new NobtId(databaseModel.getExternalId()), @@ -35,6 +46,7 @@ public Nobt mapToDomainModel(NobtEntity databaseModel) { databaseModel.getName(), explicitParticipants, expenses, + deletedExpenses, payments, databaseModel.getCreatedOn(), databaseModel.getOptimizer() @@ -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)) + .forEach(nobtEntity::addExpense); domainModel.getPayments().stream().map(paymentMapper::mapToDatabaseModel).forEach(nobtEntity::addPayment); return nobtEntity; diff --git a/rest-api/src/integrationTest/java/io/nobt/rest/ApiDocumentationTest.java b/rest-api/src/integrationTest/java/io/nobt/rest/ApiDocumentationTest.java index eb55d3c5..a77ea310 100755 --- a/rest-api/src/integrationTest/java/io/nobt/rest/ApiDocumentationTest.java +++ b/rest-api/src/integrationTest/java/io/nobt/rest/ApiDocumentationTest.java @@ -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"); + + + given(this.documentationSpec) + .port(config.port()) + .filter( + document("delete-expense", + 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 { @@ -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."), diff --git a/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java b/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java index 45de5eac..b7cc1b0b 100755 --- a/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java +++ b/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java @@ -5,6 +5,7 @@ import io.nobt.application.BodyParser; import io.nobt.application.NobtApplication; import io.nobt.core.ConversionInformationInconsistentException; +import io.nobt.core.UnknownExpenseException; import io.nobt.core.UnknownNobtException; import io.nobt.core.commands.CreateExpenseCommand; import io.nobt.core.commands.CreatePaymentCommand; @@ -48,6 +49,10 @@ public NobtRestApi(Service service, NobtRepositoryCommandInvoker nobtRepositoryC this.nobtFactory = nobtFactory; } + private static Long extractExpenseId(Request req) { + return Long.parseLong(req.params(":expenseId")); + } + public void run(int port) { http.port(port); @@ -59,18 +64,12 @@ public void run(int port) { registerTestFailRoute(); registerUnknownNobtExceptionHandler(); + registerUnknownExpenseExceptionHandler(); registerConversionInformationInconsistentExceptionHandler(); registerValidationErrorExceptionHandler(); registerUncaughtExceptionHandler(); } - private void registerApplicationRoutes() { - registerCreateNobtRoute(); - registerRetrieveNobtRoute(); - registerCreateExpenseRoute(); - registerCreatePaymentRoute(); - } - private void registerCreateExpenseRoute() { http.post("/nobts/:nobtId/expenses", "application/json", (req, resp) -> { @@ -86,6 +85,14 @@ private void registerCreateExpenseRoute() { }); } + private void registerApplicationRoutes() { + registerCreateNobtRoute(); + registerRetrieveNobtRoute(); + registerCreateExpenseRoute(); + registerDeleteExpenseRoute(); + registerCreatePaymentRoute(); + } + private void registerCreatePaymentRoute() { http.post("/nobts/:nobtId/payments", "application/json", (req, resp) -> { @@ -151,6 +158,15 @@ private static NobtId extractNobtId(Request req) { return new NobtId(externalIdentifier); } + private void registerDeleteExpenseRoute() { + http.delete("/nobts/:nobtId/expenses/:expenseId", "application/json", (req, resp) -> { + + nobtRepositoryCommandInvoker.invoke(new DeleteExpenseCommand(extractNobtId(req), extractExpenseId(req))); + resp.status(204); + return ""; + }); + } + private void setupCORS() { http.before(new CORSHandler()); @@ -180,6 +196,18 @@ private void registerUnknownNobtExceptionHandler() { }); } + private void registerUnknownExpenseExceptionHandler() { + http.exception(UnknownExpenseException.class, (e, request, response) -> { + + final ThrowableProblem problem = Problem.builder() + .withStatus(NOT_FOUND) + .withDetail("The expense you are looking for cannot be found.") + .build(); + + writeProblemAsJsonToResponse(response, problem); + }); + } + private void registerConversionInformationInconsistentExceptionHandler() { http.exception(ConversionInformationInconsistentException.class, (e, request, response) -> { diff --git a/rest-api/src/main/java/io/nobt/rest/json/nobt/NobtMixin.java b/rest-api/src/main/java/io/nobt/rest/json/nobt/NobtMixin.java index 00773808..cb90f02f 100755 --- a/rest-api/src/main/java/io/nobt/rest/json/nobt/NobtMixin.java +++ b/rest-api/src/main/java/io/nobt/rest/json/nobt/NobtMixin.java @@ -13,8 +13,9 @@ public abstract class NobtMixin extends Nobt { - public NobtMixin(NobtId id, CurrencyKey currencyKey, String name, Set explicitParticipants, Set expenses, ZonedDateTime createdOn, Optimizer optimizer) { - super(id, currencyKey, name, explicitParticipants, expenses, Collections.emptySet(), createdOn, optimizer); + public NobtMixin(NobtId id, CurrencyKey currencyKey, String name, Set explicitParticipants, Set expenses, + Set deletedExpenses, ZonedDateTime createdOn, Optimizer optimizer) { + super(id, currencyKey, name, explicitParticipants, expenses, deletedExpenses, Collections.emptySet(), createdOn, optimizer); } @Override @@ -22,10 +23,10 @@ public NobtMixin(NobtId id, CurrencyKey currencyKey, String name, Set ex public abstract List getOptimizedDebts(); @JsonIgnore - @Override - public abstract Optimizer getOptimizer(); + @Override + public abstract Optimizer getOptimizer(); @Override - @JsonProperty("currency") + @JsonProperty("currency") public abstract CurrencyKey getCurrencyKey(); } From c4c5627f199fac1d944b4e076072cd36314cf39a Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 10 Jan 2018 00:38:54 +0100 Subject: [PATCH 03/14] Add VirtualBeanPropertyWriter for Expense-links The VirtualBeanPropertyWriter allows us to add dynamic properties every time Jackson serializes an instance of our Expense. Through the built-in attribute-passing mechanism of Jackson, we pass the PropertyWriter an instance of our LinkFactory so that the logic for actually creating the link lives outside of the PropertyWriter and can be easily tested. We can now also use the "delete"-link in the ApiDocumentationTest to delete the expense. --- .../io/nobt/rest/ApiDocumentationTest.java | 4 +- .../java/io/nobt/rest/ExpenseLinkFactory.java | 23 +++++++++++ .../main/java/io/nobt/rest/NobtRestApi.java | 22 +++++++---- .../expense/ExpenseLinksPropertyWriter.java | 39 +++++++++++++++++++ .../nobt/rest/json/expense/ExpenseMixin.java | 6 +++ .../io/nobt/rest/ExpenseLinkFactoryTest.java | 22 +++++++++++ 6 files changed, 106 insertions(+), 10 deletions(-) create mode 100644 rest-api/src/main/java/io/nobt/rest/ExpenseLinkFactory.java create mode 100644 rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseLinksPropertyWriter.java create mode 100644 rest-api/src/test/java/io/nobt/rest/ExpenseLinkFactoryTest.java diff --git a/rest-api/src/integrationTest/java/io/nobt/rest/ApiDocumentationTest.java b/rest-api/src/integrationTest/java/io/nobt/rest/ApiDocumentationTest.java index a77ea310..b3c68132 100755 --- a/rest-api/src/integrationTest/java/io/nobt/rest/ApiDocumentationTest.java +++ b/rest-api/src/integrationTest/java/io/nobt/rest/ApiDocumentationTest.java @@ -182,7 +182,7 @@ public void shouldDeleteExpense() throws Exception { final String nobtId = client.createGrillfeierNobt(); client.addFleischExpense(nobtId); - final Long idOfFirstExpense = client.getNobt(nobtId).jsonPath().getLong("expenses[0].id"); + final String linkToDelete = client.getNobt(nobtId).jsonPath().getString("expenses[0]._links.delete"); given(this.documentationSpec) @@ -201,7 +201,7 @@ public void shouldDeleteExpense() throws Exception { .when() - .delete("/nobts/{nobtId}/expenses/{expenseId}", nobtId, idOfFirstExpense) + .delete(linkToDelete) .then() diff --git a/rest-api/src/main/java/io/nobt/rest/ExpenseLinkFactory.java b/rest-api/src/main/java/io/nobt/rest/ExpenseLinkFactory.java new file mode 100644 index 00000000..3bab595b --- /dev/null +++ b/rest-api/src/main/java/io/nobt/rest/ExpenseLinkFactory.java @@ -0,0 +1,23 @@ +package io.nobt.rest; + +import io.nobt.core.domain.Expense; +import io.nobt.core.domain.NobtId; + +import java.net.URI; + +public class ExpenseLinkFactory { + + private final String scheme; + private final String host; + private final NobtId nobtId; + + public ExpenseLinkFactory(String scheme, String host, NobtId nobtId) { + this.scheme = scheme; + this.host = host; + this.nobtId = nobtId; + } + + public URI createLinkToExpense(Expense e) { + return URI.create(String.format("%s://%s/nobts/%s/expenses/%d", scheme, host, nobtId.getValue(), e.getId())); + } +} diff --git a/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java b/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java index b7cc1b0b..4d8088da 100755 --- a/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java +++ b/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java @@ -49,10 +49,6 @@ public NobtRestApi(Service service, NobtRepositoryCommandInvoker nobtRepositoryC this.nobtFactory = nobtFactory; } - private static Long extractExpenseId(Request req) { - return Long.parseLong(req.params(":expenseId")); - } - public void run(int port) { http.port(port); @@ -109,6 +105,10 @@ private void registerCreatePaymentRoute() { }); } + private static Long extractExpenseId(Request req) { + return Long.parseLong(req.params(":expenseId")); + } + private void registerRetrieveNobtRoute() { http.get("/nobts/:nobtId", (req, res) -> { @@ -120,8 +120,8 @@ private void registerRetrieveNobtRoute() { res.header("Content-Type", "application/json"); - return nobt; - }, objectMapper::writeValueAsString); + return serialize(nobt, req, nobtId); + }); } private void registerCreateNobtRoute() { @@ -143,8 +143,8 @@ private void registerCreateNobtRoute() { res.header("Location", req.url() + "/" + nobt.getId().getValue()); res.header("Content-Type", "application/json"); - return nobt; - }, objectMapper::writeValueAsString); + return serialize(nobt, req, nobt.getId()); + }); } private void registerTestFailRoute() { @@ -153,6 +153,12 @@ private void registerTestFailRoute() { }); } + private String serialize(Object entity, Request req, NobtId nobtId) throws JsonProcessingException { + return objectMapper.writer() + .withAttribute(ExpenseLinkFactory.class.getName(), new ExpenseLinkFactory(req.scheme(), req.host(), nobtId)) + .writeValueAsString(entity); + } + private static NobtId extractNobtId(Request req) { final String externalIdentifier = req.params(":nobtId"); return new NobtId(externalIdentifier); diff --git a/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseLinksPropertyWriter.java b/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseLinksPropertyWriter.java new file mode 100644 index 00000000..16c42dee --- /dev/null +++ b/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseLinksPropertyWriter.java @@ -0,0 +1,39 @@ +package io.nobt.rest.json.expense; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.cfg.MapperConfig; +import com.fasterxml.jackson.databind.introspect.AnnotatedClass; +import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition; +import com.fasterxml.jackson.databind.ser.VirtualBeanPropertyWriter; +import com.fasterxml.jackson.databind.util.Annotations; +import io.nobt.core.domain.Expense; +import io.nobt.rest.ExpenseLinkFactory; + +import java.util.HashMap; + +public class ExpenseLinksPropertyWriter extends VirtualBeanPropertyWriter { + + public ExpenseLinksPropertyWriter() { + } + + public ExpenseLinksPropertyWriter(BeanPropertyDefinition propDef, Annotations contextAnnotations, JavaType declaredType) { + super(propDef, contextAnnotations, declaredType); + } + + @Override + protected Object value(Object bean, JsonGenerator gen, SerializerProvider prov) throws Exception { + + final ExpenseLinkFactory expenseLinkFactory = (ExpenseLinkFactory) prov.getAttribute(ExpenseLinkFactory.class.getName()); + + return new HashMap() {{ + put("delete", expenseLinkFactory.createLinkToExpense((Expense) bean).toString()); + }}; + } + + @Override + public VirtualBeanPropertyWriter withConfig(MapperConfig config, AnnotatedClass declaringClass, BeanPropertyDefinition propDef, JavaType type) { + return new ExpenseLinksPropertyWriter(propDef, null, type); + } +} diff --git a/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseMixin.java b/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseMixin.java index 6c6182b6..3d1c5666 100755 --- a/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseMixin.java +++ b/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseMixin.java @@ -1,6 +1,7 @@ package io.nobt.rest.json.expense; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.databind.annotation.JsonAppend; import io.nobt.core.domain.ConversionInformation; import io.nobt.core.domain.Expense; import io.nobt.core.domain.Person; @@ -10,6 +11,11 @@ import java.time.ZonedDateTime; import java.util.Set; +import static com.fasterxml.jackson.databind.annotation.JsonAppend.Prop; + +@JsonAppend( + props = @Prop(name = "_links", value = ExpenseLinksPropertyWriter.class) +) public abstract class ExpenseMixin extends Expense { public ExpenseMixin(Long id, String name, String splitStrategy, Person debtee, ConversionInformation conversionInformation, Set shares, LocalDate date, ZonedDateTime createdOn) { diff --git a/rest-api/src/test/java/io/nobt/rest/ExpenseLinkFactoryTest.java b/rest-api/src/test/java/io/nobt/rest/ExpenseLinkFactoryTest.java new file mode 100644 index 00000000..e8ece232 --- /dev/null +++ b/rest-api/src/test/java/io/nobt/rest/ExpenseLinkFactoryTest.java @@ -0,0 +1,22 @@ +package io.nobt.rest; + +import io.nobt.core.domain.NobtId; +import org.junit.Assert; +import org.junit.Test; + +import java.net.URI; + +import static io.nobt.test.domain.provider.ExpenseBuilderProvider.anExpense; + +public class ExpenseLinkFactoryTest { + + @Test + public void shouldCreateUri() { + + final ExpenseLinkFactory expenseLinkFactory = new ExpenseLinkFactory("http", "localhost:1234", new NobtId("foo")); + + final URI linkToExpense = expenseLinkFactory.createLinkToExpense(anExpense().withId(1).build()); + + Assert.assertEquals("http://localhost:1234/nobts/foo/expenses/1", linkToExpense.toString()); + } +} \ No newline at end of file From 68a0a186c163b6817e170f94eb496d2e38e93bcf Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sun, 14 Jan 2018 22:16:20 +0100 Subject: [PATCH 04/14] Add support for a scheme-overriding header The new config value allows to define an HTTP-header that will is used to override the scheme of generated links, if it contains a value. This is needed for our production environment at AWS because our application sits behind a load-balancer that accepts all requests forwards them to our application. The load-balancer accepts HTTPS requests but forwards them as HTTP. Thus, generated links would be invalid if we use the scheme of the current request to generate the link. --- .aws/deploy | 1 + README.md | 1 + .../nobt/application/env/AcceptOverrides.java | 2 + .../java/io/nobt/application/env/Config.java | 35 +++++++----- .../nobt/application/env/ConfigBuilder.java | 18 +++++- .../application/env/ConfigBuilderTest.java | 33 +++++------ .../io/nobt/application/NobtApplication.java | 2 +- .../java/io/nobt/rest/ExpenseLinkFactory.java | 12 ++-- .../main/java/io/nobt/rest/NobtRestApi.java | 22 +++---- .../java/io/nobt/rest/RequestParameters.java | 7 +++ .../io/nobt/rest/SparkRequestParameters.java | 31 ++++++++++ .../io/nobt/rest/ExpenseLinkFactoryTest.java | 8 ++- .../nobt/rest/SparkRequestParametersTest.java | 57 +++++++++++++++++++ 13 files changed, 180 insertions(+), 49 deletions(-) create mode 100644 rest-api/src/main/java/io/nobt/rest/RequestParameters.java create mode 100644 rest-api/src/main/java/io/nobt/rest/SparkRequestParameters.java create mode 100644 rest-api/src/test/java/io/nobt/rest/SparkRequestParametersTest.java diff --git a/.aws/deploy b/.aws/deploy index 25235475..bc3ca8d5 100755 --- a/.aws/deploy +++ b/.aws/deploy @@ -9,6 +9,7 @@ AWS_EB_ENV=$1 eb setenv -e ${AWS_EB_ENV} \ MIGRATE_DATABASE_AT_STARTUP=true \ USE_IN_MEMORY_DATABASE=false \ +SCHEME_OVERRIDE_HEADER=X-Forwarded-Proto \ SENTRY_RELEASE=${CI_BUILD_REF} ~/.local/bin/eb deploy ${AWS_EB_ENV} --staged --verbose \ No newline at end of file diff --git a/README.md b/README.md index 3434bbb1..ddaca4b9 100755 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ Available configuration values: |USE_IN_MEMORY_DATABASE|bool|Subtitutes the actual datastore with a non-persistent hashtable.|false| |MIGRATE_DATABASE_AT_STARTUP|bool|Migrates the defined database to the latest version. Incompatible with `USE_IN_MEMORY_DATABASE`.|false| |DATABASE_CONNECTION_STRING|string|The connection string the application should use to connect to a database.|-| +|SCHEME_OVERRIDE_HEADER|string|The HTTP-Header that the API uses to override the scheme of generated links, if present.|-| ## Documentation diff --git a/application-config/src/main/java/io/nobt/application/env/AcceptOverrides.java b/application-config/src/main/java/io/nobt/application/env/AcceptOverrides.java index 524d7a07..8702f60e 100644 --- a/application-config/src/main/java/io/nobt/application/env/AcceptOverrides.java +++ b/application-config/src/main/java/io/nobt/application/env/AcceptOverrides.java @@ -12,4 +12,6 @@ public interface AcceptOverrides extends AcceptEnvironment, BuildConfig { AcceptOverrides overrideDatabase(DatabaseConfig databaseConfig); + AcceptOverrides overrideSchemeOverrideHeader(String schemeOverrideHeader); + } diff --git a/application-config/src/main/java/io/nobt/application/env/Config.java b/application-config/src/main/java/io/nobt/application/env/Config.java index 95ac2296..6d1fa376 100644 --- a/application-config/src/main/java/io/nobt/application/env/Config.java +++ b/application-config/src/main/java/io/nobt/application/env/Config.java @@ -6,24 +6,22 @@ public final class Config { - public enum Keys { - PORT, - DATABASE_CONNECTION_STRING, - USE_IN_MEMORY_DATABASE, - MIGRATE_DATABASE_AT_STARTUP - } - - public Config(Integer port, Boolean useInMemoryDatabase, Boolean migrateDatabaseAtStartUp, DatabaseConfig databaseConfig) { + private final Integer port; + private final Boolean useInMemoryDatabase; + private final Boolean migrateDatabaseAtStartUp; + private final DatabaseConfig databaseConfig; + private final String schemeOverrideHeader; + public Config(Integer port, Boolean useInMemoryDatabase, Boolean migrateDatabaseAtStartUp, DatabaseConfig databaseConfig, String schemeOverrideHeader) { this.port = port; this.useInMemoryDatabase = useInMemoryDatabase; this.migrateDatabaseAtStartUp = migrateDatabaseAtStartUp; this.databaseConfig = databaseConfig; + this.schemeOverrideHeader = schemeOverrideHeader; } - private Integer port; - private Boolean useInMemoryDatabase; - private Boolean migrateDatabaseAtStartUp; - private DatabaseConfig databaseConfig; + public String schemeOverrideHeader() { + return schemeOverrideHeader; + } public Integer port() { return port; @@ -49,11 +47,20 @@ public boolean equals(Object o) { return Objects.equals(port, config.port) && Objects.equals(useInMemoryDatabase, config.useInMemoryDatabase) && Objects.equals(migrateDatabaseAtStartUp, config.migrateDatabaseAtStartUp) && - Objects.equals(databaseConfig, config.databaseConfig); + Objects.equals(databaseConfig, config.databaseConfig) && + Objects.equals(schemeOverrideHeader, config.schemeOverrideHeader); } @Override public int hashCode() { - return Objects.hash(port, useInMemoryDatabase, migrateDatabaseAtStartUp, databaseConfig); + return Objects.hash(port, useInMemoryDatabase, migrateDatabaseAtStartUp, databaseConfig, schemeOverrideHeader); + } + + public enum Keys { + PORT, + DATABASE_CONNECTION_STRING, + USE_IN_MEMORY_DATABASE, + MIGRATE_DATABASE_AT_STARTUP, + SCHEME_OVERRIDE_HEADER } } diff --git a/application-config/src/main/java/io/nobt/application/env/ConfigBuilder.java b/application-config/src/main/java/io/nobt/application/env/ConfigBuilder.java index 41fbde81..ff168739 100644 --- a/application-config/src/main/java/io/nobt/application/env/ConfigBuilder.java +++ b/application-config/src/main/java/io/nobt/application/env/ConfigBuilder.java @@ -19,6 +19,7 @@ public class ConfigBuilder implements AcceptEnvironment, AcceptOverrides { private Integer port; private Boolean useInMemoryDatabase; private Boolean migrateDatabaseAtStartup; + private String schemeOverrideHeader; public static AcceptEnvironment newInstance() { return new ConfigBuilder(); @@ -54,6 +55,12 @@ public AcceptOverrides overrideDatabase(DatabaseConfig databaseConfig) { return this; } + @Override + public AcceptOverrides overrideSchemeOverrideHeader(String schemeOverrideHeader) { + this.schemeOverrideHeader = schemeOverrideHeader; + return this; + } + @Override public Config build() { @@ -85,7 +92,7 @@ public Config build() { )); } - return new Config(port(), useInMemoryDatabase(), migrateDatabaseAtStartup(), databaseConfig()); + return new Config(port(), useInMemoryDatabase(), migrateDatabaseAtStartup(), databaseConfig(), schemeOverrideHeader()); } private DatabaseConfig databaseConfig() { @@ -124,6 +131,15 @@ private Integer port() { return getEnvVariableValue(environment, PORT, Integer::parseInt).orElse(null); } + private String schemeOverrideHeader() { + + if (schemeOverrideHeader != null) { + return schemeOverrideHeader; + } + + return getEnvVariableValue(environment, SCHEME_OVERRIDE_HEADER, Function.identity()).orElse(null); + } + private static Optional getEnvVariableValue(Environment environment, Config.Keys key, Function mapper) { final String envVariableValue = environment.getValue(key.name()); diff --git a/application-config/src/test/java/io/nobt/application/env/ConfigBuilderTest.java b/application-config/src/test/java/io/nobt/application/env/ConfigBuilderTest.java index d8c7a800..e1ba84d1 100644 --- a/application-config/src/test/java/io/nobt/application/env/ConfigBuilderTest.java +++ b/application-config/src/test/java/io/nobt/application/env/ConfigBuilderTest.java @@ -34,12 +34,12 @@ public void shouldParseValuesFromEnvironment(Environment environment, Config exp private static Object[] parseExamples() { return $( $( - staticEnvironment("8080", "false", "true", VALID_CONNECTION_STRING), - new Config(8080, false, true, ConnectionStringAdapter.parse(VALID_CONNECTION_STRING)) + staticEnvironment("8080", "false", "true", VALID_CONNECTION_STRING, "X-Custom-Header"), + new Config(8080, false, true, ConnectionStringAdapter.parse(VALID_CONNECTION_STRING), "X-Custom-Header") ), $( - staticEnvironment("8080", "true", "false", unset()), - new Config(8080, true, false, null) + staticEnvironment("8080", "true", "false", unset(), unset()), + new Config(8080, true, false, null, null) ) ); } @@ -62,16 +62,16 @@ public void testInvalidConfigurations(Environment environment) throws Exception private static Object[] invalidEnvironments() { return $( - staticEnvironment(aPort(), unset(), unset(), unset()), + staticEnvironment(aPort(), unset(), unset(), unset(), "X-Custom-Header"), - staticEnvironment(aPort(), "false", "true", unset()), - staticEnvironment(aPort(), unset(), "true", unset()), - staticEnvironment(aPort(), "false", unset(), unset()), + staticEnvironment(aPort(), "false", "true", unset(), "X-Custom-Header"), + staticEnvironment(aPort(), unset(), "true", unset(), "X-Custom-Header"), + staticEnvironment(aPort(), "false", unset(), unset(), "X-Custom-Header"), - staticEnvironment(aPort(), "true", "true", unset()), - staticEnvironment(aPort(), "true", unset(), unset()), + staticEnvironment(aPort(), "true", "true", unset(), "X-Custom-Header"), + staticEnvironment(aPort(), "true", unset(), unset(), "X-Custom-Header"), - staticEnvironment(aPort(), "false", "false", unset()) + staticEnvironment(aPort(), "false", "false", unset(), "X-Custom-Header") ); } @@ -83,10 +83,10 @@ public void testValidConfigurations(Environment environment) throws Exception { private static Object[] validEnvironments() { return $( - staticEnvironment(aPort(), "true", "false", unset()), - staticEnvironment(aPort(), "false", unset(), VALID_CONNECTION_STRING), - staticEnvironment(aPort(), unset(), "true", VALID_CONNECTION_STRING), - staticEnvironment(aPort(), unset(), unset(), VALID_CONNECTION_STRING) + staticEnvironment(aPort(), "true", "false", unset(), "X-Custom-Header"), + staticEnvironment(aPort(), "false", unset(), VALID_CONNECTION_STRING, "X-Custom-Header"), + staticEnvironment(aPort(), unset(), "true", VALID_CONNECTION_STRING, "X-Custom-Header"), + staticEnvironment(aPort(), unset(), unset(), VALID_CONNECTION_STRING, "X-Custom-Header") ); } @@ -98,12 +98,13 @@ private static String aPort() { return "8080"; } - private static StaticEnvironment staticEnvironment(final String port, final String inMemory, final String migrate, final String dbConnection) { + private static StaticEnvironment staticEnvironment(final String port, final String inMemory, final String migrate, final String dbConnection, String schemeOverrideHeader) { return new StaticEnvironment(new HashMap() {{ put(PORT.name(), port); put(MIGRATE_DATABASE_AT_STARTUP.name(), migrate); put(USE_IN_MEMORY_DATABASE.name(), inMemory); put(DATABASE_CONNECTION_STRING.name(), dbConnection); + put(SCHEME_OVERRIDE_HEADER.name(), schemeOverrideHeader); }}); } } \ No newline at end of file diff --git a/rest-api/src/main/java/io/nobt/application/NobtApplication.java b/rest-api/src/main/java/io/nobt/application/NobtApplication.java index a9023c8f..449d0ea9 100644 --- a/rest-api/src/main/java/io/nobt/application/NobtApplication.java +++ b/rest-api/src/main/java/io/nobt/application/NobtApplication.java @@ -76,7 +76,7 @@ public void start() { final int httpPort = config.port(); - api.run(httpPort); + api.run(httpPort, config.schemeOverrideHeader()); } private void migrateDatabase() { diff --git a/rest-api/src/main/java/io/nobt/rest/ExpenseLinkFactory.java b/rest-api/src/main/java/io/nobt/rest/ExpenseLinkFactory.java index 3bab595b..de10d304 100644 --- a/rest-api/src/main/java/io/nobt/rest/ExpenseLinkFactory.java +++ b/rest-api/src/main/java/io/nobt/rest/ExpenseLinkFactory.java @@ -7,17 +7,19 @@ public class ExpenseLinkFactory { - private final String scheme; - private final String host; + private final RequestParameters requestParameters; private final NobtId nobtId; - public ExpenseLinkFactory(String scheme, String host, NobtId nobtId) { - this.scheme = scheme; - this.host = host; + public ExpenseLinkFactory(RequestParameters requestParameters, NobtId nobtId) { + this.requestParameters = requestParameters; this.nobtId = nobtId; } public URI createLinkToExpense(Expense e) { + + final String scheme = requestParameters.getScheme(); + final String host = requestParameters.getHost(); + return URI.create(String.format("%s://%s/nobts/%s/expenses/%d", scheme, host, nobtId.getValue(), e.getId())); } } diff --git a/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java b/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java index 4d8088da..5dd5be2a 100755 --- a/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java +++ b/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java @@ -49,14 +49,14 @@ public NobtRestApi(Service service, NobtRepositoryCommandInvoker nobtRepositoryC this.nobtFactory = nobtFactory; } - public void run(int port) { + public void run(int port, String schemeOverrideHeader) { http.port(port); http.staticFiles.externalLocation("../docs"); setupCORS(); - registerApplicationRoutes(); + registerApplicationRoutes(schemeOverrideHeader); registerTestFailRoute(); registerUnknownNobtExceptionHandler(); @@ -81,9 +81,9 @@ private void registerCreateExpenseRoute() { }); } - private void registerApplicationRoutes() { - registerCreateNobtRoute(); - registerRetrieveNobtRoute(); + private void registerApplicationRoutes(String schemeOverrideHeader) { + registerCreateNobtRoute(schemeOverrideHeader); + registerRetrieveNobtRoute(schemeOverrideHeader); registerCreateExpenseRoute(); registerDeleteExpenseRoute(); registerCreatePaymentRoute(); @@ -109,7 +109,7 @@ private static Long extractExpenseId(Request req) { return Long.parseLong(req.params(":expenseId")); } - private void registerRetrieveNobtRoute() { + private void registerRetrieveNobtRoute(String schemeOverrideHeader) { http.get("/nobts/:nobtId", (req, res) -> { final NobtId nobtId = extractNobtId(req); @@ -120,11 +120,11 @@ private void registerRetrieveNobtRoute() { res.header("Content-Type", "application/json"); - return serialize(nobt, req, nobtId); + return serialize(new SparkRequestParameters(req, schemeOverrideHeader), nobtId, nobt); }); } - private void registerCreateNobtRoute() { + private void registerCreateNobtRoute(String schemeOverrideHeader) { http.post("/nobts", "application/json", (req, res) -> { final CreateNobtInput input = bodyParser.parseBodyAs(req, CreateNobtInput.class); @@ -143,7 +143,7 @@ private void registerCreateNobtRoute() { res.header("Location", req.url() + "/" + nobt.getId().getValue()); res.header("Content-Type", "application/json"); - return serialize(nobt, req, nobt.getId()); + return serialize(new SparkRequestParameters(req, schemeOverrideHeader), nobt.getId(), nobt); }); } @@ -153,9 +153,9 @@ private void registerTestFailRoute() { }); } - private String serialize(Object entity, Request req, NobtId nobtId) throws JsonProcessingException { + private String serialize(RequestParameters requestParameters, NobtId nobtId, Object entity) throws JsonProcessingException { return objectMapper.writer() - .withAttribute(ExpenseLinkFactory.class.getName(), new ExpenseLinkFactory(req.scheme(), req.host(), nobtId)) + .withAttribute(ExpenseLinkFactory.class.getName(), new ExpenseLinkFactory(requestParameters, nobtId)) .writeValueAsString(entity); } diff --git a/rest-api/src/main/java/io/nobt/rest/RequestParameters.java b/rest-api/src/main/java/io/nobt/rest/RequestParameters.java new file mode 100644 index 00000000..c2f308c0 --- /dev/null +++ b/rest-api/src/main/java/io/nobt/rest/RequestParameters.java @@ -0,0 +1,7 @@ +package io.nobt.rest; + +public interface RequestParameters { + String getScheme(); + + String getHost(); +} diff --git a/rest-api/src/main/java/io/nobt/rest/SparkRequestParameters.java b/rest-api/src/main/java/io/nobt/rest/SparkRequestParameters.java new file mode 100644 index 00000000..60757159 --- /dev/null +++ b/rest-api/src/main/java/io/nobt/rest/SparkRequestParameters.java @@ -0,0 +1,31 @@ +package io.nobt.rest; + +import spark.Request; + +public class SparkRequestParameters implements RequestParameters { + + private final Request currentRequest; + private String schemeOverrideHeader; + + public SparkRequestParameters(Request currentRequest, String schemeOverrideHeader) { + this.currentRequest = currentRequest; + this.schemeOverrideHeader = schemeOverrideHeader; + } + + @Override + public String getScheme() { + + final String forwardedProtocol = currentRequest.headers(schemeOverrideHeader); + + if (forwardedProtocol != null && !forwardedProtocol.isEmpty()) { + return forwardedProtocol; + } + + return currentRequest.scheme(); + } + + @Override + public String getHost() { + return currentRequest.host(); + } +} diff --git a/rest-api/src/test/java/io/nobt/rest/ExpenseLinkFactoryTest.java b/rest-api/src/test/java/io/nobt/rest/ExpenseLinkFactoryTest.java index e8ece232..408c147a 100644 --- a/rest-api/src/test/java/io/nobt/rest/ExpenseLinkFactoryTest.java +++ b/rest-api/src/test/java/io/nobt/rest/ExpenseLinkFactoryTest.java @@ -7,13 +7,19 @@ import java.net.URI; import static io.nobt.test.domain.provider.ExpenseBuilderProvider.anExpense; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class ExpenseLinkFactoryTest { @Test public void shouldCreateUri() { - final ExpenseLinkFactory expenseLinkFactory = new ExpenseLinkFactory("http", "localhost:1234", new NobtId("foo")); + final RequestParameters requestParameters = mock(RequestParameters.class); + when(requestParameters.getHost()).thenReturn("localhost:1234"); + when(requestParameters.getScheme()).thenReturn("http"); + + final ExpenseLinkFactory expenseLinkFactory = new ExpenseLinkFactory(requestParameters, new NobtId("foo")); final URI linkToExpense = expenseLinkFactory.createLinkToExpense(anExpense().withId(1).build()); diff --git a/rest-api/src/test/java/io/nobt/rest/SparkRequestParametersTest.java b/rest-api/src/test/java/io/nobt/rest/SparkRequestParametersTest.java new file mode 100644 index 00000000..2b2e9f31 --- /dev/null +++ b/rest-api/src/test/java/io/nobt/rest/SparkRequestParametersTest.java @@ -0,0 +1,57 @@ +package io.nobt.rest; + +import org.hamcrest.Matchers; +import org.junit.Before; +import org.junit.Test; +import spark.Request; + +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class SparkRequestParametersTest { + + private Request requestMock; + + @Before + public void setUp() throws Exception { + requestMock = mock(Request.class); + } + + @Test + public void shouldReadHostFromRequest() { + + when(requestMock.host()).thenReturn("localhost:8080"); + final SparkRequestParameters sut = new SparkRequestParameters(requestMock, null); + + assertThat(sut.getHost(), Matchers.is("localhost:8080")); + } + + @Test + public void givenCustomSchemeFromCustomHeader_shouldOverrideRequestScheme() { + + when(requestMock.headers("X-Custom-Header")).thenReturn("https"); + when(requestMock.scheme()).thenReturn("http"); + final SparkRequestParameters sut = new SparkRequestParameters(requestMock, "X-Custom-Header"); + + assertThat(sut.getScheme(), Matchers.is("https")); + } + + @Test + public void givenNoCustomSchemeFromCustomHeader_shouldUseRequestScheme() { + + when(requestMock.scheme()).thenReturn("http"); + final SparkRequestParameters sut = new SparkRequestParameters(requestMock, "X-Custom-Header"); + + assertThat(sut.getScheme(), Matchers.is("http")); + } + + @Test + public void givenNoCustomHeader_shouldUseRequestScheme() { + + when(requestMock.scheme()).thenReturn("http"); + final SparkRequestParameters sut = new SparkRequestParameters(requestMock, null); + + assertThat(sut.getScheme(), Matchers.is("http")); + } +} \ No newline at end of file From cc5860b51b04bd3675b38b38ae2382b7ad816d11 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 18 Jan 2018 00:35:56 +0100 Subject: [PATCH 05/14] Create proper link for Nobt upon creation --- .../java/io/nobt/rest/ExpenseLinkFactory.java | 25 ----------- .../main/java/io/nobt/rest/NobtRestApi.java | 20 ++++++--- .../java/io/nobt/rest/RequestParameters.java | 7 ---- .../io/nobt/rest/SparkRequestParameters.java | 31 -------------- .../expense/ExpenseLinksPropertyWriter.java | 2 +- .../java/io/nobt/rest/links/BasePath.java | 42 +++++++++++++++++++ .../nobt/rest/links/ExpenseLinkFactory.java | 25 +++++++++++ .../io/nobt/rest/links/NobtLinkFactory.java | 18 ++++++++ .../BasePathTest.java} | 29 ++++++------- .../{ => links}/ExpenseLinkFactoryTest.java | 13 +++--- .../nobt/rest/links/NobtLinkFactoryTest.java | 24 +++++++++++ 11 files changed, 143 insertions(+), 93 deletions(-) delete mode 100644 rest-api/src/main/java/io/nobt/rest/ExpenseLinkFactory.java delete mode 100644 rest-api/src/main/java/io/nobt/rest/RequestParameters.java delete mode 100644 rest-api/src/main/java/io/nobt/rest/SparkRequestParameters.java create mode 100644 rest-api/src/main/java/io/nobt/rest/links/BasePath.java create mode 100644 rest-api/src/main/java/io/nobt/rest/links/ExpenseLinkFactory.java create mode 100644 rest-api/src/main/java/io/nobt/rest/links/NobtLinkFactory.java rename rest-api/src/test/java/io/nobt/rest/{SparkRequestParametersTest.java => links/BasePathTest.java} (54%) rename rest-api/src/test/java/io/nobt/rest/{ => links}/ExpenseLinkFactoryTest.java (59%) create mode 100644 rest-api/src/test/java/io/nobt/rest/links/NobtLinkFactoryTest.java diff --git a/rest-api/src/main/java/io/nobt/rest/ExpenseLinkFactory.java b/rest-api/src/main/java/io/nobt/rest/ExpenseLinkFactory.java deleted file mode 100644 index de10d304..00000000 --- a/rest-api/src/main/java/io/nobt/rest/ExpenseLinkFactory.java +++ /dev/null @@ -1,25 +0,0 @@ -package io.nobt.rest; - -import io.nobt.core.domain.Expense; -import io.nobt.core.domain.NobtId; - -import java.net.URI; - -public class ExpenseLinkFactory { - - private final RequestParameters requestParameters; - private final NobtId nobtId; - - public ExpenseLinkFactory(RequestParameters requestParameters, NobtId nobtId) { - this.requestParameters = requestParameters; - this.nobtId = nobtId; - } - - public URI createLinkToExpense(Expense e) { - - final String scheme = requestParameters.getScheme(); - final String host = requestParameters.getHost(); - - return URI.create(String.format("%s://%s/nobts/%s/expenses/%d", scheme, host, nobtId.getValue(), e.getId())); - } -} diff --git a/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java b/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java index 5dd5be2a..7a2c09d5 100755 --- a/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java +++ b/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java @@ -13,6 +13,9 @@ import io.nobt.core.commands.RetrieveNobtCommand; import io.nobt.core.domain.*; import io.nobt.persistence.NobtRepositoryCommandInvoker; +import io.nobt.rest.links.BasePath; +import io.nobt.rest.links.ExpenseLinkFactory; +import io.nobt.rest.links.NobtLinkFactory; import io.nobt.rest.payloads.CreateNobtInput; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -120,7 +123,10 @@ private void registerRetrieveNobtRoute(String schemeOverrideHeader) { res.header("Content-Type", "application/json"); - return serialize(new SparkRequestParameters(req, schemeOverrideHeader), nobtId, nobt); + final BasePath basePath = BasePath.parse(req, schemeOverrideHeader); + final ExpenseLinkFactory expenseLinkFactory = new ExpenseLinkFactory(basePath, nobt); + + return serialize(nobt, expenseLinkFactory); }); } @@ -138,12 +144,16 @@ private void registerCreateNobtRoute(String schemeOverrideHeader) { return repository.getById(id); }); + final BasePath basePath = BasePath.parse(req, schemeOverrideHeader); + + final NobtLinkFactory nobtLinkFactory = new NobtLinkFactory(basePath); + final ExpenseLinkFactory expenseLinkFactory = new ExpenseLinkFactory(basePath, nobt); res.status(201); - res.header("Location", req.url() + "/" + nobt.getId().getValue()); + res.header("Location", nobtLinkFactory.createLinkToNobt(nobt).toString()); res.header("Content-Type", "application/json"); - return serialize(new SparkRequestParameters(req, schemeOverrideHeader), nobt.getId(), nobt); + return serialize(nobt, expenseLinkFactory); }); } @@ -153,9 +163,9 @@ private void registerTestFailRoute() { }); } - private String serialize(RequestParameters requestParameters, NobtId nobtId, Object entity) throws JsonProcessingException { + private String serialize(Object entity, ExpenseLinkFactory expenseLinkFactory) throws JsonProcessingException { return objectMapper.writer() - .withAttribute(ExpenseLinkFactory.class.getName(), new ExpenseLinkFactory(requestParameters, nobtId)) + .withAttribute(ExpenseLinkFactory.class.getName(), expenseLinkFactory) .writeValueAsString(entity); } diff --git a/rest-api/src/main/java/io/nobt/rest/RequestParameters.java b/rest-api/src/main/java/io/nobt/rest/RequestParameters.java deleted file mode 100644 index c2f308c0..00000000 --- a/rest-api/src/main/java/io/nobt/rest/RequestParameters.java +++ /dev/null @@ -1,7 +0,0 @@ -package io.nobt.rest; - -public interface RequestParameters { - String getScheme(); - - String getHost(); -} diff --git a/rest-api/src/main/java/io/nobt/rest/SparkRequestParameters.java b/rest-api/src/main/java/io/nobt/rest/SparkRequestParameters.java deleted file mode 100644 index 60757159..00000000 --- a/rest-api/src/main/java/io/nobt/rest/SparkRequestParameters.java +++ /dev/null @@ -1,31 +0,0 @@ -package io.nobt.rest; - -import spark.Request; - -public class SparkRequestParameters implements RequestParameters { - - private final Request currentRequest; - private String schemeOverrideHeader; - - public SparkRequestParameters(Request currentRequest, String schemeOverrideHeader) { - this.currentRequest = currentRequest; - this.schemeOverrideHeader = schemeOverrideHeader; - } - - @Override - public String getScheme() { - - final String forwardedProtocol = currentRequest.headers(schemeOverrideHeader); - - if (forwardedProtocol != null && !forwardedProtocol.isEmpty()) { - return forwardedProtocol; - } - - return currentRequest.scheme(); - } - - @Override - public String getHost() { - return currentRequest.host(); - } -} diff --git a/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseLinksPropertyWriter.java b/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseLinksPropertyWriter.java index 16c42dee..20817e26 100644 --- a/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseLinksPropertyWriter.java +++ b/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseLinksPropertyWriter.java @@ -9,7 +9,7 @@ import com.fasterxml.jackson.databind.ser.VirtualBeanPropertyWriter; import com.fasterxml.jackson.databind.util.Annotations; import io.nobt.core.domain.Expense; -import io.nobt.rest.ExpenseLinkFactory; +import io.nobt.rest.links.ExpenseLinkFactory; import java.util.HashMap; diff --git a/rest-api/src/main/java/io/nobt/rest/links/BasePath.java b/rest-api/src/main/java/io/nobt/rest/links/BasePath.java new file mode 100644 index 00000000..0dced24b --- /dev/null +++ b/rest-api/src/main/java/io/nobt/rest/links/BasePath.java @@ -0,0 +1,42 @@ +package io.nobt.rest.links; + +import spark.Request; + +public class BasePath { + + private final String protocol; + private final String host; + + public BasePath(String protocol, String host) { + this.protocol = protocol; + this.host = host; + } + + public static BasePath parse(Request request, String schemeOverrideHeader) { + + final String protocol = determineProtocol(request, schemeOverrideHeader); + final String host = request.host(); + + return new BasePath(protocol, host); + + } + + private static String determineProtocol(Request request, String schemeOverrideHeader) { + final String forwardedProtocol = request.headers(schemeOverrideHeader); + + if (forwardedProtocol != null && !forwardedProtocol.isEmpty()) { + return forwardedProtocol; + } + + return request.scheme(); + } + + public String asString() { + return String.format("%s://%s", protocol, host); + } + + @Override + public String toString() { + return String.format("BasePath{%s}", asString()); + } +} diff --git a/rest-api/src/main/java/io/nobt/rest/links/ExpenseLinkFactory.java b/rest-api/src/main/java/io/nobt/rest/links/ExpenseLinkFactory.java new file mode 100644 index 00000000..f1ca9c63 --- /dev/null +++ b/rest-api/src/main/java/io/nobt/rest/links/ExpenseLinkFactory.java @@ -0,0 +1,25 @@ +package io.nobt.rest.links; + +import io.nobt.core.domain.Expense; +import io.nobt.core.domain.Nobt; + +import java.net.URI; + +public class ExpenseLinkFactory { + + private final BasePath basePath; + private final Nobt nobt; + + public ExpenseLinkFactory(BasePath basePath, Nobt nobt) { + this.basePath = basePath; + this.nobt = nobt; + } + + public URI createLinkToExpense(Expense e) { + + final String nobtId = nobt.getId().getValue(); + final long expenseId = e.getId(); + + return URI.create(String.format("%s/nobts/%s/expenses/%d", basePath.asString(), nobtId, expenseId)); + } +} diff --git a/rest-api/src/main/java/io/nobt/rest/links/NobtLinkFactory.java b/rest-api/src/main/java/io/nobt/rest/links/NobtLinkFactory.java new file mode 100644 index 00000000..63430b81 --- /dev/null +++ b/rest-api/src/main/java/io/nobt/rest/links/NobtLinkFactory.java @@ -0,0 +1,18 @@ +package io.nobt.rest.links; + +import io.nobt.core.domain.Nobt; + +import java.net.URI; + +public class NobtLinkFactory { + + private BasePath basePath; + + public NobtLinkFactory(BasePath basePath) { + this.basePath = basePath; + } + + public URI createLinkToNobt(Nobt nobt) { + return URI.create(String.format("%s/nobts/%s", basePath.asString(), nobt.getId().getValue())); + } +} diff --git a/rest-api/src/test/java/io/nobt/rest/SparkRequestParametersTest.java b/rest-api/src/test/java/io/nobt/rest/links/BasePathTest.java similarity index 54% rename from rest-api/src/test/java/io/nobt/rest/SparkRequestParametersTest.java rename to rest-api/src/test/java/io/nobt/rest/links/BasePathTest.java index 2b2e9f31..7aaa44fc 100644 --- a/rest-api/src/test/java/io/nobt/rest/SparkRequestParametersTest.java +++ b/rest-api/src/test/java/io/nobt/rest/links/BasePathTest.java @@ -1,4 +1,4 @@ -package io.nobt.rest; +package io.nobt.rest.links; import org.hamcrest.Matchers; import org.junit.Before; @@ -9,7 +9,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class SparkRequestParametersTest { +public class BasePathTest { private Request requestMock; @@ -22,36 +22,33 @@ public void setUp() throws Exception { public void shouldReadHostFromRequest() { when(requestMock.host()).thenReturn("localhost:8080"); - final SparkRequestParameters sut = new SparkRequestParameters(requestMock, null); + when(requestMock.scheme()).thenReturn("http"); + + final BasePath basePath = BasePath.parse(requestMock, null); - assertThat(sut.getHost(), Matchers.is("localhost:8080")); + assertThat(basePath.asString(), Matchers.is("http://localhost:8080")); } @Test public void givenCustomSchemeFromCustomHeader_shouldOverrideRequestScheme() { + when(requestMock.host()).thenReturn("localhost:8080"); when(requestMock.headers("X-Custom-Header")).thenReturn("https"); when(requestMock.scheme()).thenReturn("http"); - final SparkRequestParameters sut = new SparkRequestParameters(requestMock, "X-Custom-Header"); - assertThat(sut.getScheme(), Matchers.is("https")); + final BasePath basePath = BasePath.parse(requestMock, "X-Custom-Header"); + + assertThat(basePath.asString(), Matchers.is("https://localhost:8080")); } @Test public void givenNoCustomSchemeFromCustomHeader_shouldUseRequestScheme() { + when(requestMock.host()).thenReturn("localhost:8080"); when(requestMock.scheme()).thenReturn("http"); - final SparkRequestParameters sut = new SparkRequestParameters(requestMock, "X-Custom-Header"); - assertThat(sut.getScheme(), Matchers.is("http")); - } - - @Test - public void givenNoCustomHeader_shouldUseRequestScheme() { - - when(requestMock.scheme()).thenReturn("http"); - final SparkRequestParameters sut = new SparkRequestParameters(requestMock, null); + final BasePath basePath = BasePath.parse(requestMock, "X-Custom-Header"); - assertThat(sut.getScheme(), Matchers.is("http")); + assertThat(basePath.asString(), Matchers.is("http://localhost:8080")); } } \ No newline at end of file diff --git a/rest-api/src/test/java/io/nobt/rest/ExpenseLinkFactoryTest.java b/rest-api/src/test/java/io/nobt/rest/links/ExpenseLinkFactoryTest.java similarity index 59% rename from rest-api/src/test/java/io/nobt/rest/ExpenseLinkFactoryTest.java rename to rest-api/src/test/java/io/nobt/rest/links/ExpenseLinkFactoryTest.java index 408c147a..ed2db55f 100644 --- a/rest-api/src/test/java/io/nobt/rest/ExpenseLinkFactoryTest.java +++ b/rest-api/src/test/java/io/nobt/rest/links/ExpenseLinkFactoryTest.java @@ -1,5 +1,6 @@ -package io.nobt.rest; +package io.nobt.rest.links; +import io.nobt.core.domain.Nobt; import io.nobt.core.domain.NobtId; import org.junit.Assert; import org.junit.Test; @@ -7,19 +8,15 @@ import java.net.URI; import static io.nobt.test.domain.provider.ExpenseBuilderProvider.anExpense; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static io.nobt.test.domain.provider.NobtBuilderProvider.aNobt; public class ExpenseLinkFactoryTest { @Test public void shouldCreateUri() { - final RequestParameters requestParameters = mock(RequestParameters.class); - when(requestParameters.getHost()).thenReturn("localhost:1234"); - when(requestParameters.getScheme()).thenReturn("http"); - - final ExpenseLinkFactory expenseLinkFactory = new ExpenseLinkFactory(requestParameters, new NobtId("foo")); + final Nobt nobt = aNobt().withId(new NobtId("foo")).build(); + final ExpenseLinkFactory expenseLinkFactory = new ExpenseLinkFactory(new BasePath("http", "localhost:1234"), nobt); final URI linkToExpense = expenseLinkFactory.createLinkToExpense(anExpense().withId(1).build()); diff --git a/rest-api/src/test/java/io/nobt/rest/links/NobtLinkFactoryTest.java b/rest-api/src/test/java/io/nobt/rest/links/NobtLinkFactoryTest.java new file mode 100644 index 00000000..628cf675 --- /dev/null +++ b/rest-api/src/test/java/io/nobt/rest/links/NobtLinkFactoryTest.java @@ -0,0 +1,24 @@ +package io.nobt.rest.links; + +import io.nobt.core.domain.Nobt; +import io.nobt.core.domain.NobtId; +import org.junit.Assert; +import org.junit.Test; + +import java.net.URI; + +import static io.nobt.test.domain.provider.NobtBuilderProvider.aNobt; + +public class NobtLinkFactoryTest { + + @Test + public void shouldCreateUri() { + + final NobtLinkFactory nobtLinkFactory = new NobtLinkFactory(new BasePath("http", "localhost:1234")); + + final Nobt nobt = aNobt().withId(new NobtId("foo")).build(); + final URI linkToNobt = nobtLinkFactory.createLinkToNobt(nobt); + + Assert.assertEquals("http://localhost:1234/nobts/foo", linkToNobt.toString()); + } +} \ No newline at end of file From 1189594e069836bd846373b0230ce779d6b06dee Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sun, 28 Jan 2018 21:43:15 +0100 Subject: [PATCH 06/14] Make deleted column non-nullable and disallow resetting deleted state --- .../main/resources/db/migration/V2__add_deleted_flag.sql | 2 +- .../io/nobt/persistence/cashflow/expense/ExpenseEntity.java | 6 +++--- .../src/main/java/io/nobt/persistence/nobt/NobtMapper.java | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql b/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql index b7df217d..b0b313c4 100755 --- a/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql +++ b/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql @@ -1,3 +1,3 @@ ALTER TABLE expenses - ADD deleted BOOLEAN; + ADD deleted BOOLEAN NOT NULL; diff --git a/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseEntity.java b/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseEntity.java index 194fd6ae..e2473a97 100644 --- a/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseEntity.java +++ b/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseEntity.java @@ -37,7 +37,7 @@ public class ExpenseEntity extends CashFlowEntity { @Column(name = "date", nullable = false) private LocalDate date; - @Column(name = "deleted", nullable = false) + @Column(name = "deleted", nullable = false, insertable = false) private boolean deleted; public String getName() { @@ -100,8 +100,8 @@ public boolean isDeleted() { return deleted; } - public void setDeleted(boolean deleted) { - this.deleted = deleted; + public void markDeleted() { + this.deleted = true; } public boolean isNotDeleted() { diff --git a/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java b/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java index 84acefbe..bb50e48e 100644 --- a/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java +++ b/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java @@ -74,7 +74,7 @@ public NobtEntity mapToDatabaseModel(Nobt domainModel) { domainModel.getExpenses().stream().map(expenseMapper::mapToDatabaseModel).forEach(nobtEntity::addExpense); domainModel.getDeletedExpenses().stream() .map(expenseMapper::mapToDatabaseModel) - .peek(e -> e.setDeleted(true)) + .peek(ExpenseEntity::markDeleted) .forEach(nobtEntity::addExpense); domainModel.getPayments().stream().map(paymentMapper::mapToDatabaseModel).forEach(nobtEntity::addPayment); From ee2f22b247131d3d846b01adcf036699f6fbdc8f Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sun, 28 Jan 2018 21:43:27 +0100 Subject: [PATCH 07/14] Combine matchers into single assert statement --- core/src/test/java/io/nobt/core/domain/NobtTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/io/nobt/core/domain/NobtTest.java b/core/src/test/java/io/nobt/core/domain/NobtTest.java index 5aa0c87a..367281ea 100755 --- a/core/src/test/java/io/nobt/core/domain/NobtTest.java +++ b/core/src/test/java/io/nobt/core/domain/NobtTest.java @@ -161,8 +161,10 @@ public void shouldRemoveExpenseById() throws Exception { nobt.removeExpense(1L); - assertThat(nobt, hasExpenses(iterableWithSize(0))); - assertThat(nobt, hasDeletedExpenses(iterableWithSize(1))); + assertThat(nobt, allOf( + hasExpenses(iterableWithSize(0)), + hasDeletedExpenses(iterableWithSize(1)) + )); } @Test From 5381c7f53b72fecba96347390904be40b33fe4fd Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sun, 28 Jan 2018 21:47:14 +0100 Subject: [PATCH 08/14] Add documentation about for deleting expenses --- rest-api/src/docs/asciidoc/index.adoc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/rest-api/src/docs/asciidoc/index.adoc b/rest-api/src/docs/asciidoc/index.adoc index cc70671e..d5a320c3 100755 --- a/rest-api/src/docs/asciidoc/index.adoc +++ b/rest-api/src/docs/asciidoc/index.adoc @@ -97,4 +97,14 @@ include::{snippets}/get-nobt/http-request.adoc[] This time, the response is way more interesting, as it includes debts. Note that we also already added payment, which is immediately reflected in the resulting debts. include::{snippets}/get-nobt/http-response.adoc[] -include::{snippets}/get-nobt/response-fields.adoc[] \ No newline at end of file +include::{snippets}/get-nobt/response-fields.adoc[] + +== Delete an expense + +Expenses can be deleted from the nobt in order to exclude it from the debt calculation. However, expenses are not actually deleted from the database but rather only marked as deleted. + +If an expense can be deleted, it has a link attached with a link relation `delete`. Sending an `HTTP` `DELETE` request to this link marks an expense as deleted. This operation cannot be undone. + +include::{snippets}/delete-expense/http-request.adoc[] + +include::{snippets}/delete-expense/http-response.adoc[] \ No newline at end of file From ad901152defb8b9a0fcb7a0ed84b881a8eeec5d3 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sun, 28 Jan 2018 21:47:26 +0100 Subject: [PATCH 09/14] Fix typo --- rest-api/src/docs/asciidoc/index.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest-api/src/docs/asciidoc/index.adoc b/rest-api/src/docs/asciidoc/index.adoc index d5a320c3..cdcbe0e3 100755 --- a/rest-api/src/docs/asciidoc/index.adoc +++ b/rest-api/src/docs/asciidoc/index.adoc @@ -94,7 +94,7 @@ Once we add at least one expense, it makes sense to re-retrieve the nobt we crea include::{snippets}/get-nobt/http-request.adoc[] -This time, the response is way more interesting, as it includes debts. Note that we also already added payment, which is immediately reflected in the resulting debts. +This time, the response is way more interesting, as it includes debts. Note that we also already added a payment, which is immediately reflected in the resulting debts. include::{snippets}/get-nobt/http-response.adoc[] include::{snippets}/get-nobt/response-fields.adoc[] From ba9bea41ddce55dca6ac3fbd2b46d259110438b2 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sun, 28 Jan 2018 21:49:36 +0100 Subject: [PATCH 10/14] Fix error in SQL script --- .../src/main/resources/db/migration/V2__add_deleted_flag.sql | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql b/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql index b0b313c4..230786b6 100755 --- a/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql +++ b/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql @@ -1,3 +1,2 @@ ALTER TABLE expenses - ADD deleted BOOLEAN NOT NULL; - + ADD deleted BOOLEAN NOT NULL DEFAULT FALSE; \ No newline at end of file From 4758bb11152d02ef414a1351ce312161cbf50a37 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sun, 28 Jan 2018 22:00:16 +0100 Subject: [PATCH 11/14] Add doc and (currently failing) assertion --- rest-api/src/docs/asciidoc/index.adoc | 6 +++++- .../java/io/nobt/rest/ApiDocumentationTest.java | 17 +++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/rest-api/src/docs/asciidoc/index.adoc b/rest-api/src/docs/asciidoc/index.adoc index cdcbe0e3..4f43f65c 100755 --- a/rest-api/src/docs/asciidoc/index.adoc +++ b/rest-api/src/docs/asciidoc/index.adoc @@ -107,4 +107,8 @@ If an expense can be deleted, it has a link attached with a link relation `delet include::{snippets}/delete-expense/http-request.adoc[] -include::{snippets}/delete-expense/http-response.adoc[] \ No newline at end of file +include::{snippets}/delete-expense/http-response.adoc[] + +After marking the expense as deleted, it is returned in a separate collection `deletedExpenses`: + +include::{snippets}/get-nobt-with-deleted-expense/http-response.adoc[] \ No newline at end of file diff --git a/rest-api/src/integrationTest/java/io/nobt/rest/ApiDocumentationTest.java b/rest-api/src/integrationTest/java/io/nobt/rest/ApiDocumentationTest.java index b3c68132..08d3783a 100755 --- a/rest-api/src/integrationTest/java/io/nobt/rest/ApiDocumentationTest.java +++ b/rest-api/src/integrationTest/java/io/nobt/rest/ApiDocumentationTest.java @@ -208,9 +208,22 @@ public void shouldDeleteExpense() throws Exception { .statusCode(204); - client.getNobt(nobtId) + given(this.documentationSpec) + .filter( + document("get-nobt-with-deleted-expense", + preprocessRequest( + configureHost(), + replaceLocalhost() + ), + preprocessResponse( + replaceLocalhost() + ) + ) + ) + .get("/nobts/{id}", nobtId) .then() - .body("expenses", response -> hasSize(0)); + .body("expenses", response -> hasSize(0)) + .body("deletedExpenses[0]._links", response -> not(hasKey("delete"))); } @Test From c7db496279c3023e69b3174805b28fcd02cb1fc8 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sun, 28 Jan 2018 22:33:57 +0100 Subject: [PATCH 12/14] Wrap expenses in DeletedExpense upon deletion This prevents DeletedExpenses to render a link for deleting them. --- .../io/nobt/core/domain/DeletedExpense.java | 54 +++++++++++++++++++ .../main/java/io/nobt/core/domain/Nobt.java | 8 +-- .../nobt/test/domain/builder/NobtBuilder.java | 4 +- .../test/domain/matchers/NobtMatchers.java | 6 +-- .../io/nobt/persistence/nobt/NobtMapper.java | 4 +- .../main/java/io/nobt/rest/NobtRestApi.java | 11 ++-- .../java/io/nobt/rest/json/CoreModule.java | 2 + .../json/expense/DeletedExpenseMixin.java | 25 +++++++++ .../expense/ExpenseLinksPropertyWriter.java | 5 +- .../io/nobt/rest/json/nobt/NobtMixin.java | 2 +- .../nobt/rest/links/ExpenseLinkFactory.java | 7 +-- .../java/io/nobt/rest/links/LinkFactory.java | 7 +++ .../io/nobt/rest/links/NobtLinkFactory.java | 5 +- .../rest/links/ExpenseLinkFactoryTest.java | 5 +- .../nobt/rest/links/NobtLinkFactoryTest.java | 2 +- 15 files changed, 121 insertions(+), 26 deletions(-) create mode 100644 core/src/main/java/io/nobt/core/domain/DeletedExpense.java create mode 100755 rest-api/src/main/java/io/nobt/rest/json/expense/DeletedExpenseMixin.java create mode 100644 rest-api/src/main/java/io/nobt/rest/links/LinkFactory.java diff --git a/core/src/main/java/io/nobt/core/domain/DeletedExpense.java b/core/src/main/java/io/nobt/core/domain/DeletedExpense.java new file mode 100644 index 00000000..7871dc2f --- /dev/null +++ b/core/src/main/java/io/nobt/core/domain/DeletedExpense.java @@ -0,0 +1,54 @@ +package io.nobt.core.domain; + +import java.time.LocalDate; +import java.time.ZonedDateTime; +import java.util.Set; + +public class DeletedExpense { + + private final Expense originalExpense; + + public DeletedExpense(Expense originalExpense) { + this.originalExpense = originalExpense; + } + + public long getId() { + return originalExpense.getId(); + } + + public String getName() { + return originalExpense.getName(); + } + + public String getSplitStrategy() { + return originalExpense.getSplitStrategy(); + } + + public Person getDebtee() { + return originalExpense.getDebtee(); + } + + public Set getShares() { + return originalExpense.getShares(); + } + + public LocalDate getDate() { + return originalExpense.getDate(); + } + + public ZonedDateTime getCreatedOn() { + return originalExpense.getCreatedOn(); + } + + public ConversionInformation getConversionInformation() { + return originalExpense.getConversionInformation(); + } + + public Set getParticipants() { + return originalExpense.getParticipants(); + } + + public Expense getOriginalExpense() { + return originalExpense; + } +} diff --git a/core/src/main/java/io/nobt/core/domain/Nobt.java b/core/src/main/java/io/nobt/core/domain/Nobt.java index f466162e..bbdadb19 100755 --- a/core/src/main/java/io/nobt/core/domain/Nobt.java +++ b/core/src/main/java/io/nobt/core/domain/Nobt.java @@ -19,12 +19,12 @@ public class Nobt { private final String name; private final Set explicitParticipants; private final Set expenses; - private final Set deletedExpenses; + private final Set deletedExpenses; private final Set payments; private final ZonedDateTime createdOn; private final Optimizer optimizer; - public Nobt(NobtId id, CurrencyKey currencyKey, String name, Set explicitParticipants, Set expenses, Set deletedExpenses, Set payments, ZonedDateTime createdOn, Optimizer optimizer) { + public Nobt(NobtId id, CurrencyKey currencyKey, String name, Set explicitParticipants, Set expenses, Set deletedExpenses, Set payments, ZonedDateTime createdOn, Optimizer optimizer) { this.id = id; this.currencyKey = currencyKey; this.name = name; @@ -60,7 +60,7 @@ public Set getExpenses() { return Collections.unmodifiableSet(expenses); } - public Set getDeletedExpenses() { + public Set getDeletedExpenses() { return Collections.unmodifiableSet(deletedExpenses); } @@ -132,7 +132,7 @@ public void removeExpense(long expenseId) { .orElseThrow(UnknownExpenseException::new); expenses.remove(expenseToDelete); - deletedExpenses.add(expenseToDelete); + deletedExpenses.add(new DeletedExpense(expenseToDelete)); } } \ No newline at end of file diff --git a/core/src/testSupport/java/io/nobt/test/domain/builder/NobtBuilder.java b/core/src/testSupport/java/io/nobt/test/domain/builder/NobtBuilder.java index 38cbc787..00a78e3c 100644 --- a/core/src/testSupport/java/io/nobt/test/domain/builder/NobtBuilder.java +++ b/core/src/testSupport/java/io/nobt/test/domain/builder/NobtBuilder.java @@ -12,7 +12,7 @@ public class NobtBuilder { private Set expenses; - private Set deletedExpenses; + private Set deletedExpenses; private Set participants; private Set payments; private ZonedDateTime dateTime; @@ -35,7 +35,7 @@ public NobtBuilder withExpenses(ExpenseBuilder... expenseBuilders) { } public NobtBuilder withDeletedExpenses(Set expenses) { - this.deletedExpenses = expenses; + this.deletedExpenses = expenses.stream().map(DeletedExpense::new).collect(toSet()); return this; } diff --git a/core/src/testSupport/java/io/nobt/test/domain/matchers/NobtMatchers.java b/core/src/testSupport/java/io/nobt/test/domain/matchers/NobtMatchers.java index d6ff46b6..b8e2dd97 100644 --- a/core/src/testSupport/java/io/nobt/test/domain/matchers/NobtMatchers.java +++ b/core/src/testSupport/java/io/nobt/test/domain/matchers/NobtMatchers.java @@ -63,10 +63,10 @@ protected Set featureValueOf(Nobt actual) { }; } - public static Matcher hasDeletedExpenses(final Matcher> subMatcher) { - return new FeatureMatcher>(subMatcher, "deletedExpenses", "deletedExpenses") { + public static Matcher hasDeletedExpenses(final Matcher> subMatcher) { + return new FeatureMatcher>(subMatcher, "deletedExpenses", "deletedExpenses") { @Override - protected Set featureValueOf(Nobt actual) { + protected Set featureValueOf(Nobt actual) { return actual.getDeletedExpenses(); } }; diff --git a/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java b/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java index bb50e48e..ef4a8b30 100644 --- a/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java +++ b/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java @@ -32,9 +32,10 @@ public Nobt mapToDomainModel(NobtEntity databaseModel) { .filter(ExpenseEntity::isNotDeleted) .map(expenseMapper::mapToDomainModel) .collect(toSet()); - final Set deletedExpenses = databaseModel.getExpenses().stream() + final Set deletedExpenses = databaseModel.getExpenses().stream() .filter(ExpenseEntity::isDeleted) .map(expenseMapper::mapToDomainModel) + .map(DeletedExpense::new) .collect(toSet()); final Set payments = databaseModel.getPayments().stream() .map(paymentMapper::mapToDomainModel) @@ -73,6 +74,7 @@ public NobtEntity mapToDatabaseModel(Nobt domainModel) { domainModel.getExpenses().stream().map(expenseMapper::mapToDatabaseModel).forEach(nobtEntity::addExpense); domainModel.getDeletedExpenses().stream() + .map(DeletedExpense::getOriginalExpense) .map(expenseMapper::mapToDatabaseModel) .peek(ExpenseEntity::markDeleted) .forEach(nobtEntity::addExpense); diff --git a/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java b/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java index 7a2c09d5..a41061d6 100755 --- a/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java +++ b/rest-api/src/main/java/io/nobt/rest/NobtRestApi.java @@ -15,6 +15,7 @@ import io.nobt.persistence.NobtRepositoryCommandInvoker; import io.nobt.rest.links.BasePath; import io.nobt.rest.links.ExpenseLinkFactory; +import io.nobt.rest.links.LinkFactory; import io.nobt.rest.links.NobtLinkFactory; import io.nobt.rest.payloads.CreateNobtInput; import org.apache.logging.log4j.LogManager; @@ -124,7 +125,7 @@ private void registerRetrieveNobtRoute(String schemeOverrideHeader) { res.header("Content-Type", "application/json"); final BasePath basePath = BasePath.parse(req, schemeOverrideHeader); - final ExpenseLinkFactory expenseLinkFactory = new ExpenseLinkFactory(basePath, nobt); + final LinkFactory expenseLinkFactory = new ExpenseLinkFactory(basePath, nobt); return serialize(nobt, expenseLinkFactory); }); @@ -146,11 +147,11 @@ private void registerCreateNobtRoute(String schemeOverrideHeader) { final BasePath basePath = BasePath.parse(req, schemeOverrideHeader); - final NobtLinkFactory nobtLinkFactory = new NobtLinkFactory(basePath); - final ExpenseLinkFactory expenseLinkFactory = new ExpenseLinkFactory(basePath, nobt); + final LinkFactory nobtLinkFactory = new NobtLinkFactory(basePath); + final LinkFactory expenseLinkFactory = new ExpenseLinkFactory(basePath, nobt); res.status(201); - res.header("Location", nobtLinkFactory.createLinkToNobt(nobt).toString()); + res.header("Location", nobtLinkFactory.createLinkTo(nobt).toString()); res.header("Content-Type", "application/json"); return serialize(nobt, expenseLinkFactory); @@ -163,7 +164,7 @@ private void registerTestFailRoute() { }); } - private String serialize(Object entity, ExpenseLinkFactory expenseLinkFactory) throws JsonProcessingException { + private String serialize(Object entity, LinkFactory expenseLinkFactory) throws JsonProcessingException { return objectMapper.writer() .withAttribute(ExpenseLinkFactory.class.getName(), expenseLinkFactory) .writeValueAsString(entity); diff --git a/rest-api/src/main/java/io/nobt/rest/json/CoreModule.java b/rest-api/src/main/java/io/nobt/rest/json/CoreModule.java index fda991ba..43bb014a 100755 --- a/rest-api/src/main/java/io/nobt/rest/json/CoreModule.java +++ b/rest-api/src/main/java/io/nobt/rest/json/CoreModule.java @@ -12,6 +12,7 @@ import io.nobt.rest.json.currency.CurrencyKeyDeserializer; import io.nobt.rest.json.currency.CurrencyKeySerializer; import io.nobt.rest.json.expense.ConversionInformationMixin; +import io.nobt.rest.json.expense.DeletedExpenseMixin; import io.nobt.rest.json.expense.ExpenseDraftMixin; import io.nobt.rest.json.expense.ExpenseMixin; import io.nobt.rest.json.nobt.NobtIdSerializer; @@ -46,6 +47,7 @@ public CoreModule() { setMixInAnnotation(Nobt.class, NobtMixin.class); setMixInAnnotation(Expense.class, ExpenseMixin.class); + setMixInAnnotation(DeletedExpense.class, DeletedExpenseMixin.class); setMixInAnnotation(ExpenseDraft.class, ExpenseDraftMixin.class); setMixInAnnotation(Payment.class, PaymentMixin.class); diff --git a/rest-api/src/main/java/io/nobt/rest/json/expense/DeletedExpenseMixin.java b/rest-api/src/main/java/io/nobt/rest/json/expense/DeletedExpenseMixin.java new file mode 100755 index 00000000..19957f8e --- /dev/null +++ b/rest-api/src/main/java/io/nobt/rest/json/expense/DeletedExpenseMixin.java @@ -0,0 +1,25 @@ +package io.nobt.rest.json.expense; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import io.nobt.core.domain.DeletedExpense; +import io.nobt.core.domain.Expense; +import io.nobt.core.domain.Person; + +import java.util.Set; + +public abstract class DeletedExpenseMixin extends DeletedExpense { + + public DeletedExpenseMixin(Expense originalExpense) { + super(originalExpense); + } + + @Override + @JsonIgnore + public abstract Set getParticipants(); + + @Override + @JsonIgnore + public Expense getOriginalExpense() { + return super.getOriginalExpense(); + } +} diff --git a/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseLinksPropertyWriter.java b/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseLinksPropertyWriter.java index 20817e26..c6b08a82 100644 --- a/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseLinksPropertyWriter.java +++ b/rest-api/src/main/java/io/nobt/rest/json/expense/ExpenseLinksPropertyWriter.java @@ -10,6 +10,7 @@ import com.fasterxml.jackson.databind.util.Annotations; import io.nobt.core.domain.Expense; import io.nobt.rest.links.ExpenseLinkFactory; +import io.nobt.rest.links.LinkFactory; import java.util.HashMap; @@ -25,10 +26,10 @@ public ExpenseLinksPropertyWriter(BeanPropertyDefinition propDef, Annotations co @Override protected Object value(Object bean, JsonGenerator gen, SerializerProvider prov) throws Exception { - final ExpenseLinkFactory expenseLinkFactory = (ExpenseLinkFactory) prov.getAttribute(ExpenseLinkFactory.class.getName()); + final LinkFactory expenseLinkFactory = (LinkFactory) prov.getAttribute(ExpenseLinkFactory.class.getName()); return new HashMap() {{ - put("delete", expenseLinkFactory.createLinkToExpense((Expense) bean).toString()); + put("delete", expenseLinkFactory.createLinkTo((Expense) bean).toString()); }}; } diff --git a/rest-api/src/main/java/io/nobt/rest/json/nobt/NobtMixin.java b/rest-api/src/main/java/io/nobt/rest/json/nobt/NobtMixin.java index cb90f02f..bfb56a83 100755 --- a/rest-api/src/main/java/io/nobt/rest/json/nobt/NobtMixin.java +++ b/rest-api/src/main/java/io/nobt/rest/json/nobt/NobtMixin.java @@ -14,7 +14,7 @@ public abstract class NobtMixin extends Nobt { public NobtMixin(NobtId id, CurrencyKey currencyKey, String name, Set explicitParticipants, Set expenses, - Set deletedExpenses, ZonedDateTime createdOn, Optimizer optimizer) { + Set deletedExpenses, ZonedDateTime createdOn, Optimizer optimizer) { super(id, currencyKey, name, explicitParticipants, expenses, deletedExpenses, Collections.emptySet(), createdOn, optimizer); } diff --git a/rest-api/src/main/java/io/nobt/rest/links/ExpenseLinkFactory.java b/rest-api/src/main/java/io/nobt/rest/links/ExpenseLinkFactory.java index f1ca9c63..4a3633c5 100644 --- a/rest-api/src/main/java/io/nobt/rest/links/ExpenseLinkFactory.java +++ b/rest-api/src/main/java/io/nobt/rest/links/ExpenseLinkFactory.java @@ -5,7 +5,7 @@ import java.net.URI; -public class ExpenseLinkFactory { +public class ExpenseLinkFactory implements LinkFactory { private final BasePath basePath; private final Nobt nobt; @@ -15,10 +15,11 @@ public ExpenseLinkFactory(BasePath basePath, Nobt nobt) { this.nobt = nobt; } - public URI createLinkToExpense(Expense e) { + @Override + public URI createLinkTo(Expense expense) { final String nobtId = nobt.getId().getValue(); - final long expenseId = e.getId(); + final long expenseId = expense.getId(); return URI.create(String.format("%s/nobts/%s/expenses/%d", basePath.asString(), nobtId, expenseId)); } diff --git a/rest-api/src/main/java/io/nobt/rest/links/LinkFactory.java b/rest-api/src/main/java/io/nobt/rest/links/LinkFactory.java new file mode 100644 index 00000000..f75a1953 --- /dev/null +++ b/rest-api/src/main/java/io/nobt/rest/links/LinkFactory.java @@ -0,0 +1,7 @@ +package io.nobt.rest.links; + +import java.net.URI; + +public interface LinkFactory { + URI createLinkTo(T t); +} diff --git a/rest-api/src/main/java/io/nobt/rest/links/NobtLinkFactory.java b/rest-api/src/main/java/io/nobt/rest/links/NobtLinkFactory.java index 63430b81..16a3574f 100644 --- a/rest-api/src/main/java/io/nobt/rest/links/NobtLinkFactory.java +++ b/rest-api/src/main/java/io/nobt/rest/links/NobtLinkFactory.java @@ -4,7 +4,7 @@ import java.net.URI; -public class NobtLinkFactory { +public class NobtLinkFactory implements LinkFactory { private BasePath basePath; @@ -12,7 +12,8 @@ public NobtLinkFactory(BasePath basePath) { this.basePath = basePath; } - public URI createLinkToNobt(Nobt nobt) { + @Override + public URI createLinkTo(Nobt nobt) { return URI.create(String.format("%s/nobts/%s", basePath.asString(), nobt.getId().getValue())); } } diff --git a/rest-api/src/test/java/io/nobt/rest/links/ExpenseLinkFactoryTest.java b/rest-api/src/test/java/io/nobt/rest/links/ExpenseLinkFactoryTest.java index ed2db55f..6ed908ca 100644 --- a/rest-api/src/test/java/io/nobt/rest/links/ExpenseLinkFactoryTest.java +++ b/rest-api/src/test/java/io/nobt/rest/links/ExpenseLinkFactoryTest.java @@ -1,5 +1,6 @@ package io.nobt.rest.links; +import io.nobt.core.domain.Expense; import io.nobt.core.domain.Nobt; import io.nobt.core.domain.NobtId; import org.junit.Assert; @@ -16,9 +17,9 @@ public class ExpenseLinkFactoryTest { public void shouldCreateUri() { final Nobt nobt = aNobt().withId(new NobtId("foo")).build(); - final ExpenseLinkFactory expenseLinkFactory = new ExpenseLinkFactory(new BasePath("http", "localhost:1234"), nobt); + final LinkFactory expenseLinkFactory = new ExpenseLinkFactory(new BasePath("http", "localhost:1234"), nobt); - final URI linkToExpense = expenseLinkFactory.createLinkToExpense(anExpense().withId(1).build()); + final URI linkToExpense = expenseLinkFactory.createLinkTo(anExpense().withId(1).build()); Assert.assertEquals("http://localhost:1234/nobts/foo/expenses/1", linkToExpense.toString()); } diff --git a/rest-api/src/test/java/io/nobt/rest/links/NobtLinkFactoryTest.java b/rest-api/src/test/java/io/nobt/rest/links/NobtLinkFactoryTest.java index 628cf675..2653d88f 100644 --- a/rest-api/src/test/java/io/nobt/rest/links/NobtLinkFactoryTest.java +++ b/rest-api/src/test/java/io/nobt/rest/links/NobtLinkFactoryTest.java @@ -17,7 +17,7 @@ public void shouldCreateUri() { final NobtLinkFactory nobtLinkFactory = new NobtLinkFactory(new BasePath("http", "localhost:1234")); final Nobt nobt = aNobt().withId(new NobtId("foo")).build(); - final URI linkToNobt = nobtLinkFactory.createLinkToNobt(nobt); + final URI linkToNobt = nobtLinkFactory.createLinkTo(nobt); Assert.assertEquals("http://localhost:1234/nobts/foo", linkToNobt.toString()); } From fb56bb7a826e90c3af3ca3a40b2090c1f22bf53c Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sun, 28 Jan 2018 22:49:33 +0100 Subject: [PATCH 13/14] Store instant of deletion instead of deleted-flag This way, we also know when an expense was deleted. --- .../io/nobt/core/domain/DeletedExpense.java | 12 ++++++++++++ .../db/migration/V2__add_deleted_flag.sql | 2 -- .../migration/V2__add_deleted_timestamp.sql | 2 ++ .../cashflow/expense/ExpenseEntity.java | 19 ++++++++++++------- .../io/nobt/persistence/nobt/NobtMapper.java | 18 +++++++++++++----- .../json/expense/DeletedExpenseMixin.java | 5 +++-- 6 files changed, 42 insertions(+), 16 deletions(-) delete mode 100755 db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql create mode 100755 db-migrations/src/main/resources/db/migration/V2__add_deleted_timestamp.sql diff --git a/core/src/main/java/io/nobt/core/domain/DeletedExpense.java b/core/src/main/java/io/nobt/core/domain/DeletedExpense.java index 7871dc2f..ed83cbe3 100644 --- a/core/src/main/java/io/nobt/core/domain/DeletedExpense.java +++ b/core/src/main/java/io/nobt/core/domain/DeletedExpense.java @@ -1,5 +1,7 @@ package io.nobt.core.domain; +import java.time.Clock; +import java.time.Instant; import java.time.LocalDate; import java.time.ZonedDateTime; import java.util.Set; @@ -7,9 +9,15 @@ public class DeletedExpense { private final Expense originalExpense; + private final Instant deletedOn; public DeletedExpense(Expense originalExpense) { + this(originalExpense, Clock.systemUTC().instant()); + } + + public DeletedExpense(Expense originalExpense, Instant deletedOn) { this.originalExpense = originalExpense; + this.deletedOn = deletedOn; } public long getId() { @@ -51,4 +59,8 @@ public Set getParticipants() { public Expense getOriginalExpense() { return originalExpense; } + + public Instant getDeletedOn() { + return deletedOn; + } } diff --git a/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql b/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql deleted file mode 100755 index 230786b6..00000000 --- a/db-migrations/src/main/resources/db/migration/V2__add_deleted_flag.sql +++ /dev/null @@ -1,2 +0,0 @@ -ALTER TABLE expenses - ADD deleted BOOLEAN NOT NULL DEFAULT FALSE; \ No newline at end of file diff --git a/db-migrations/src/main/resources/db/migration/V2__add_deleted_timestamp.sql b/db-migrations/src/main/resources/db/migration/V2__add_deleted_timestamp.sql new file mode 100755 index 00000000..332ad6af --- /dev/null +++ b/db-migrations/src/main/resources/db/migration/V2__add_deleted_timestamp.sql @@ -0,0 +1,2 @@ +ALTER TABLE expenses + ADD deletedOn TIMESTAMP WITH TIME ZONE; \ No newline at end of file diff --git a/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseEntity.java b/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseEntity.java index e2473a97..551ed499 100644 --- a/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseEntity.java +++ b/persistence/src/main/java/io/nobt/persistence/cashflow/expense/ExpenseEntity.java @@ -8,6 +8,7 @@ import javax.persistence.Entity; import javax.persistence.Table; import java.math.BigDecimal; +import java.time.Instant; import java.time.LocalDate; import java.util.List; @@ -37,8 +38,8 @@ public class ExpenseEntity extends CashFlowEntity { @Column(name = "date", nullable = false) private LocalDate date; - @Column(name = "deleted", nullable = false, insertable = false) - private boolean deleted; + @Column(name = "deletedOn", insertable = false) + private Instant deletedOn; public String getName() { return name; @@ -97,14 +98,18 @@ public void setDate(LocalDate date) { } public boolean isDeleted() { - return deleted; - } - - public void markDeleted() { - this.deleted = true; + return deletedOn != null; } public boolean isNotDeleted() { return !isDeleted(); } + + public Instant getDeletedOn() { + return deletedOn; + } + + public void setDeletedOn(Instant deletedOn) { + this.deletedOn = deletedOn; + } } \ No newline at end of file diff --git a/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java b/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java index ef4a8b30..3c2b814e 100644 --- a/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java +++ b/persistence/src/main/java/io/nobt/persistence/nobt/NobtMapper.java @@ -34,8 +34,10 @@ public Nobt mapToDomainModel(NobtEntity databaseModel) { .collect(toSet()); final Set deletedExpenses = databaseModel.getExpenses().stream() .filter(ExpenseEntity::isDeleted) - .map(expenseMapper::mapToDomainModel) - .map(DeletedExpense::new) + .map(expenseEntity -> { + final Expense expense = expenseMapper.mapToDomainModel(expenseEntity); + return new DeletedExpense(expense, expenseEntity.getDeletedOn()); + }) .collect(toSet()); final Set payments = databaseModel.getPayments().stream() .map(paymentMapper::mapToDomainModel) @@ -74,9 +76,15 @@ public NobtEntity mapToDatabaseModel(Nobt domainModel) { domainModel.getExpenses().stream().map(expenseMapper::mapToDatabaseModel).forEach(nobtEntity::addExpense); domainModel.getDeletedExpenses().stream() - .map(DeletedExpense::getOriginalExpense) - .map(expenseMapper::mapToDatabaseModel) - .peek(ExpenseEntity::markDeleted) + .map(deletedExpense -> { + + final Expense originalExpense = deletedExpense.getOriginalExpense(); + final ExpenseEntity expenseEntity = expenseMapper.mapToDatabaseModel(originalExpense); + + expenseEntity.setDeletedOn(deletedExpense.getDeletedOn()); + + return expenseEntity; + }) .forEach(nobtEntity::addExpense); domainModel.getPayments().stream().map(paymentMapper::mapToDatabaseModel).forEach(nobtEntity::addPayment); diff --git a/rest-api/src/main/java/io/nobt/rest/json/expense/DeletedExpenseMixin.java b/rest-api/src/main/java/io/nobt/rest/json/expense/DeletedExpenseMixin.java index 19957f8e..744ed56c 100755 --- a/rest-api/src/main/java/io/nobt/rest/json/expense/DeletedExpenseMixin.java +++ b/rest-api/src/main/java/io/nobt/rest/json/expense/DeletedExpenseMixin.java @@ -5,12 +5,13 @@ import io.nobt.core.domain.Expense; import io.nobt.core.domain.Person; +import java.time.Instant; import java.util.Set; public abstract class DeletedExpenseMixin extends DeletedExpense { - public DeletedExpenseMixin(Expense originalExpense) { - super(originalExpense); + public DeletedExpenseMixin(Expense originalExpense, Instant deletedOn) { + super(originalExpense, deletedOn); } @Override From 3be7fff5828d4f4269d1a89f7eef2f0300564f07 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sun, 28 Jan 2018 22:51:19 +0100 Subject: [PATCH 14/14] Use combined matcher with single assert --- .../repository/EntityManagerNobtRepositoryIT.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/persistence/src/integrationTest/java/io/nobt/persistence/repository/EntityManagerNobtRepositoryIT.java b/persistence/src/integrationTest/java/io/nobt/persistence/repository/EntityManagerNobtRepositoryIT.java index 708ef4e7..6426e31b 100644 --- a/persistence/src/integrationTest/java/io/nobt/persistence/repository/EntityManagerNobtRepositoryIT.java +++ b/persistence/src/integrationTest/java/io/nobt/persistence/repository/EntityManagerNobtRepositoryIT.java @@ -161,11 +161,9 @@ public void shouldDeleteExpense() throws Exception { final Nobt nobtWithoutExpense = fetch(id); - assertThat(nobtWithoutExpense, hasExpenses( - iterableWithSize(0) - )); - assertThat(nobtWithoutExpense, hasDeletedExpenses( - iterableWithSize(1) + assertThat(nobtWithoutExpense, allOf( + hasExpenses(iterableWithSize(0)), + hasDeletedExpenses(iterableWithSize(1)) )); }