Skip to content
This repository has been archived by the owner on May 9, 2019. It is now read-only.

Transaction Service - Submit delivery details #94

Merged
merged 21 commits into from
Jun 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ lazy val transactionApi = (project in file("transaction-api"))
lombok
)
)
.dependsOn(security)
.dependsOn(security, itemApi)

lazy val transactionImpl = (project in file("transaction-impl"))
.settings(commonSettings: _*)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

// import com.example.auction.item.api.DeliveryOption;

import com.fasterxml.jackson.annotation.JsonCreator;
import lombok.Value;

@Value
public final class DeliveryInfo {

private final String addressLine1;
Expand All @@ -12,6 +16,7 @@ public final class DeliveryInfo {
private final String country;
// private final DeliveryOption selectedDeliveryOption;

@JsonCreator
public DeliveryInfo(String addressLine1, String addressLine2, String city, String state, int postalCode, String country /* DeliveryOption selectedDeliveryOption*/) {
this.addressLine1 = addressLine1;
this.addressLine2 = addressLine2;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,35 @@
package com.example.auction.transaction.api;

//import com.example.auction.item.api.Item;
import org.pcollections.PSequence;
import com.example.auction.item.api.ItemData;
import com.fasterxml.jackson.annotation.JsonCreator;
import lombok.Value;

import java.util.Optional;
import java.util.UUID;

@Value
public final class TransactionInfo {

//private final Item item;
private final PSequence<TransactionMessage> messages;
private final TransactionStatus status;
private final DeliveryInfo deliveryInfo;
//private final PSequence<TransactionMessage> messages;
private final UUID itemId;
private final UUID creator;
private final UUID winner;
private final ItemData itemData;
private final int itemPrice;
private final int deliveryPrice;
private final PaymentInfo paymentInfo;
private final Optional<DeliveryInfo> deliveryInfo;
private final TransactionInfoStatus status;
//private final PaymentInfo paymentInfo;

public TransactionInfo(/*Item item, */PSequence<TransactionMessage> messages, TransactionStatus status, DeliveryInfo deliveryInfo, int deliveryPrice, PaymentInfo paymentInfo) {
// this.item = item;
this.messages = messages;
this.status = status;
this.deliveryInfo = deliveryInfo;
@JsonCreator
public TransactionInfo(UUID itemId, UUID creator, UUID winner, ItemData itemData, int itemPrice, int deliveryPrice, Optional<DeliveryInfo> deliveryInfo, TransactionInfoStatus status) {
this.itemId = itemId;
this.creator = creator;
this.winner = winner;
this.itemData = itemData;
this.itemPrice = itemPrice;
this.deliveryPrice = deliveryPrice;
this.paymentInfo = paymentInfo;
this.deliveryInfo = deliveryInfo;
this.status = status;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.example.auction.transaction.api;

public enum TransactionStatus {
public enum TransactionInfoStatus {
/**
* Negotiating delivery details.
*/
Expand All @@ -11,6 +11,11 @@ public enum TransactionStatus {
*/
PAYMENT_SUBMITTED,

/**
* Payment is rejected
*/
PAYMENT_FAILED,

/**
* Payment is confirmed.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
package com.example.auction.transaction.api;

import static com.lightbend.lagom.javadsl.api.Service.named;
import static com.lightbend.lagom.javadsl.api.Service.pathCall;

import akka.Done;
import akka.NotUsed;
import com.example.auction.security.SecurityHeaderFilter;
import com.lightbend.lagom.javadsl.api.Descriptor;
import com.lightbend.lagom.javadsl.api.Service;
import com.lightbend.lagom.javadsl.api.ServiceCall;
import com.lightbend.lagom.javadsl.api.broker.Topic;
import com.lightbend.lagom.javadsl.api.deser.PathParamSerializers;

import java.util.UUID;

/**
* The transaction services.
Expand All @@ -19,7 +27,7 @@ public interface TransactionService extends Service {

//ServiceCall<TransactionMessage, Done> sendMessage(UUID itemId);

//ServiceCall<DeliveryInfo, Done> submitDeliveryDetails(UUID itemId);
ServiceCall<DeliveryInfo, Done> submitDeliveryDetails(UUID itemId);

//ServiceCall<Integer, Done> setDeliveryPrice(UUID itemId);

Expand All @@ -31,6 +39,8 @@ public interface TransactionService extends Service {

//ServiceCall<NotUsed, Done> initiateRefund(UUID itemId);

ServiceCall<NotUsed, TransactionInfo> getTransaction(UUID itemId);

/**
* The transaction events topic.
*/
Expand All @@ -39,8 +49,11 @@ public interface TransactionService extends Service {
@Override
default Descriptor descriptor() {
return named("transaction").withCalls(
// No pathcalls for now ...
);
pathCall("/api/transaction/:id", this::submitDeliveryDetails),
pathCall("/api/transaction/:id", this::getTransaction)
).withPathParamSerializer(UUID.class, PathParamSerializers.required("UUID", UUID::fromString, UUID::toString))
.withHeaderFilter(SecurityHeaderFilter.INSTANCE);

}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.example.auction.transaction.impl;

import com.fasterxml.jackson.annotation.JsonCreator;
import lombok.Value;

@Value
public class DeliveryData {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the private representation of the transaction-api's DeliveryInfo.

We have a terrible mix of naming convetions in that sometimes the private copy is PXyz, sometimes it's the exact same name with only a package difference, ...
In general I hate using Data and Info for anything other than signifying that data is master and info can be derived from data.

I don't think this issue of naming should be solved in this PR but it' be good if we settled and considered creating a convention.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Raised #98

private final String addressLine1;
private final String addressLine2;
private final String city;
private final String state;
private final int postalCode;
private final String country;

@JsonCreator
public DeliveryData(String addressLine1, String addressLine2, String city, String state, int postalCode, String country) {
this.addressLine1 = addressLine1;
this.addressLine2 = addressLine2;
this.city = city;
this.state = state;
this.postalCode = postalCode;
this.country = country;
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package com.example.auction.transaction.impl;

import com.example.auction.item.api.ItemData;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.lightbend.lagom.serialization.Jsonable;
import lombok.Value;

import java.util.Optional;
import java.util.UUID;

@Value
Expand All @@ -12,15 +14,33 @@ public class Transaction implements Jsonable {
private final UUID itemId;
private final UUID creator;
private final UUID winner;
private final ItemData itemData;
private final int itemPrice;
private final int deliveryPrice;
private final Optional<DeliveryData> deliveryData;

@JsonCreator
public Transaction(UUID itemId, UUID creator, UUID winner, int itemPrice, int deliveryPrice) {
private Transaction(UUID itemId, UUID creator, UUID winner, ItemData itemData, int itemPrice, int deliveryPrice, Optional<DeliveryData> deliveryData) {
this.itemId = itemId;
this.creator = creator;
this.winner = winner;
this.itemData = itemData;
this.itemPrice = itemPrice;
this.deliveryPrice = deliveryPrice;
this.deliveryData = deliveryData;
}

public Transaction(UUID itemId, UUID creator, UUID winner, ItemData itemData, int itemPrice) {
this.itemId = itemId;
this.creator = creator;
this.winner = winner;
this.itemData = itemData;
this.itemPrice = itemPrice;
this.deliveryPrice = 0;
this.deliveryData = Optional.empty();
}

public Transaction withDeliveryData(DeliveryData deliveryData){
return new Transaction(itemId, creator, winner, itemData, itemPrice, deliveryPrice, Optional.of(deliveryData));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@

import akka.Done;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.lightbend.lagom.javadsl.persistence.PersistentEntity;
import com.lightbend.lagom.javadsl.persistence.PersistentEntity.ReplyType;
import com.lightbend.lagom.serialization.Jsonable;
import lombok.Value;

import java.util.UUID;

/**
* A transaction command.
*/
public interface TransactionCommand extends Jsonable {

@Value
final class StartTransaction implements TransactionCommand, PersistentEntity.ReplyType<Done> {
final class StartTransaction implements TransactionCommand, ReplyType<Done> {

private final Transaction transaction;

Expand All @@ -20,4 +23,26 @@ public StartTransaction(Transaction transaction) {
this.transaction = transaction;
}
}

@Value
final class SubmitDeliveryDetails implements TransactionCommand, ReplyType<Done> {
private final UUID userId;
private final DeliveryData deliveryData;

@JsonCreator
public SubmitDeliveryDetails(UUID userId, DeliveryData deliveryData) {
this.userId = userId;
this.deliveryData = deliveryData;
}
}

@Value
final class GetTransaction implements TransactionCommand, ReplyType<TransactionState> {
private final UUID userId;

@JsonCreator
public GetTransaction(UUID userId) {
this.userId = userId;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package com.example.auction.transaction.impl;

import akka.Done;
import com.lightbend.lagom.javadsl.api.transport.Forbidden;
import com.lightbend.lagom.javadsl.api.transport.NotFound;
import com.lightbend.lagom.javadsl.persistence.PersistentEntity;
import com.example.auction.transaction.impl.TransactionCommand.*;
import com.example.auction.transaction.impl.TransactionEvent.*;

import java.util.Optional;
import java.util.UUID;

public class TransactionEntity extends PersistentEntity<TransactionCommand, TransactionEvent, TransactionState> {

@Override
public Behavior initialBehavior(Optional<TransactionState> snapshotState) {
if (!snapshotState.isPresent()) {
Expand All @@ -19,6 +23,8 @@ public Behavior initialBehavior(Optional<TransactionState> snapshotState) {
return notStarted(state);
case NEGOTIATING_DELIVERY:
return negotiatingDelivery(state);
case PAYMENT_SUBMITTED:
return paymentSubmitted(state);
default:
throw new IllegalStateException();
}
Expand All @@ -28,25 +34,69 @@ public Behavior initialBehavior(Optional<TransactionState> snapshotState) {
private Behavior notStarted(TransactionState state) {
BehaviorBuilder builder = newBehaviorBuilder(state);

builder.setCommandHandler(StartTransaction.class, (start, ctx) ->
ctx.thenPersist(new TransactionStarted(entityUUID(), start.getTransaction()), (e) ->
builder.setCommandHandler(StartTransaction.class, (cmd, ctx) ->
ctx.thenPersist(new TransactionStarted(entityUUID(), cmd.getTransaction()), (e) ->
ctx.reply(Done.getInstance())
)
);

builder.setEventHandlerChangingBehavior(TransactionStarted.class, started ->
negotiatingDelivery(TransactionState.start(started.getTransaction()))
builder.setEventHandlerChangingBehavior(TransactionStarted.class, event ->
negotiatingDelivery(TransactionState.start(event.getTransaction()))
);

addGetTransactionHandler(builder);
return builder.build();
}

private Behavior negotiatingDelivery(TransactionState state) {
BehaviorBuilder builder = newBehaviorBuilder(state);

builder.setReadOnlyCommandHandler(StartTransaction.class, (cmd, ctx) ->
ctx.reply(Done.getInstance())
);

builder.setCommandHandler(SubmitDeliveryDetails.class, (cmd, ctx) -> {
if(cmd.getUserId().equals(state().getTransaction().get().getWinner())) {
return ctx.thenPersist(new DeliveryDetailsSubmitted(entityUUID(), cmd.getDeliveryData()), (e) ->
ctx.reply(Done.getInstance())
);
}
else
throw new Forbidden("Only the auction winner can submit delivery details");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you'd better use ctx.failed here. Throwing an exception will kill the underlying actor and cause it to be rebuilt. If you use ctx.failed you are signaling the command process completed successfully with a rejection.

Copy link
Contributor

Choose a reason for hiding this comment

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

Changing this will break your tests.
Also, I've just noticed there's a similar case few lines below this.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if that's the case - I think all commands are processed in a try catch, and when a command handler throws an exception, Lagom catches it and passes it to ctx.failed for you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, can I throw an exception? Using ctx.failed breaks test, is it related with lagom/lagom#325?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While browsing the Lagom source code, found this
As @jroper said, commands are inside a try catch, so I think it's safe throwing an exception
I changed this in the latest commit

Copy link
Contributor

Choose a reason for hiding this comment

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

I think ctx.failed would be a better example of what we consider best practices... I'm not sure why it's failing the test, but we can look at it in a separate pull request. This is OK for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since some services use throw new(...) and others ctx.commandFailed, I raised #108

});

builder.setEventHandler(DeliveryDetailsSubmitted.class, evt ->
state().updateDeliveryData(evt.getDeliveryData())
);

addGetTransactionHandler(builder);

return builder.build();
}

private Behavior paymentSubmitted(TransactionState state) {
BehaviorBuilder builder = newBehaviorBuilder(state);
// WIP ...

addGetTransactionHandler(builder);

return builder.build();
}

private void addGetTransactionHandler(BehaviorBuilder builder) {
builder.setReadOnlyCommandHandler(GetTransaction.class, (cmd, ctx) -> {
if(state().getTransaction().isPresent()) {
if (cmd.getUserId().equals(state().getTransaction().get().getCreator()) ||
cmd.getUserId().equals(state().getTransaction().get().getWinner()))
ctx.reply(state());
else
throw new Forbidden("Only the item owner and the auction winner can see transaction details");
}
else
throw new NotFound("Transaction for item " + entityId() + " not found");
});
}

private UUID entityUUID() {
return UUID.fromString(entityId());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,15 @@ public TransactionStarted(UUID itemId, Transaction transaction) {
}
}

@Value
final class DeliveryDetailsSubmitted implements TransactionEvent {
private final UUID itemId;
private final DeliveryData deliveryData;

@JsonCreator
public DeliveryDetailsSubmitted(UUID itemId, DeliveryData deliveryData) {
this.itemId = itemId;
this.deliveryData = deliveryData;
}
}
}
Loading