Mouwrice commented on code in PR #349:
URL: 
https://github.com/apache/pekko-persistence-dynamodb/pull/349#discussion_r3747798274


##########
src/main/scala/org/apache/pekko/persistence/dynamodb/journal/DynamoDBHelper.scala:
##########
@@ -135,103 +123,59 @@ trait DynamoDBHelper {
     f
   }
 
-  trait Describe[T] {
-    def desc(t: T): String
-    protected def formatKey(i: Item): String = {
-      val key = i.get(Key) match {
-        case null => "<none>"
-        case x    => x.getS
-      }
-      val sort = i.get(Sort) match {
-        case null => "<none>"
-        case x    => x.getN
-      }
-      s"[$Key=$key,$Sort=$sort]"
+  protected def formatKey(i: Item): String = {
+    val key = i.get(Key) match {
+      case null => "<none>"
+      case x    => x.s
     }
-  }
-
-  object Describe {
-    implicit object GenericDescribe extends Describe[AmazonWebServiceRequest] {
-      def desc(aws: AmazonWebServiceRequest): String = 
aws.getClass.getSimpleName
+    val sort = i.get(Sort) match {
+      case null => "<none>"
+      case x    => x.n
     }
+    s"[$Key=$key,$Sort=$sort]"
   }
 
-  implicit object DescribeDescribe extends Describe[DescribeTableRequest] {
-    def desc(aws: DescribeTableRequest): String = 
s"DescribeTableRequest(${aws.getTableName})"
-  }
+  def listTables(aws: ListTablesRequest): Future[ListTablesResponse] =
+    send(s"ListTablesRequest", dynamoDB.listTables(aws).asScala)
 
-  implicit object QueryDescribe extends Describe[QueryRequest] {
-    def desc(aws: QueryRequest): String = 
s"QueryRequest(${aws.getTableName},${aws.getExpressionAttributeValues})"
-  }
+  def describeTable(aws: DescribeTableRequest): Future[DescribeTableResponse] =
+    send(s"DescribeTableRequest(${aws.tableName})", 
dynamoDB.describeTable(aws).asScala)
 
-  implicit object PutItemDescribe extends Describe[PutItemRequest] {
-    def desc(aws: PutItemRequest): String = 
s"PutItemRequest(${aws.getTableName},${formatKey(aws.getItem)})"
-  }
+  def createTable(aws: CreateTableRequest): Future[CreateTableResponse] =
+    send(s"CreateTableRequest(${aws.tableName})", 
dynamoDB.createTable(aws).asScala)
 
-  implicit object DeleteDescribe extends Describe[DeleteItemRequest] {
-    def desc(aws: DeleteItemRequest): String = 
s"DeleteItemRequest(${aws.getTableName},${formatKey(aws.getKey)})"
-  }
+  def updateTable(aws: UpdateTableRequest): Future[UpdateTableResponse] =
+    send(s"UpdateTableRequest(${aws.tableName})", 
dynamoDB.updateTable(aws).asScala)
 
-  implicit object BatchGetItemDescribe extends Describe[BatchGetItemRequest] {
-    def desc(aws: BatchGetItemRequest): String = {
-      val entry = aws.getRequestItems.entrySet.iterator.next()
-      val table = entry.getKey
-      val keys = entry.getValue.getKeys.asScala.map(formatKey)
-      s"BatchGetItemRequest($table, ${keys.mkString("(", ",", ")")})"
-    }
-  }
-
-  implicit object BatchWriteItemDescribe extends 
Describe[BatchWriteItemRequest] {
-    def desc(aws: BatchWriteItemRequest): String = {
-      val entry = aws.getRequestItems.entrySet.iterator.next()
-      val table = entry.getKey
-      val keys = entry.getValue.asScala.map { write =>
-        write.getDeleteRequest match {
-          case null => "put" + formatKey(write.getPutRequest.getItem)
-          case del  => "del" + formatKey(del.getKey)
-        }
-      }
-      s"BatchWriteItemRequest($table, ${keys.mkString("(", ",", ")")})"
-    }
-  }
+  def deleteTable(aws: DeleteTableRequest): Future[DeleteTableResponse] =
+    send(s"DeleteTableRequest(${aws.tableName})", 
dynamoDB.deleteTable(aws).asScala)
 
-  def listTables(aws: ListTablesRequest): Future[ListTablesResult] =
-    send[ListTablesRequest, ListTablesResult](aws, 
dynamoDB.listTablesAsync(aws, _))
+  def query(aws: QueryRequest): Future[QueryResponse] =
+    send(s"QueryRequest(${aws.tableName},${aws.expressionAttributeValues})", 
dynamoDB.query(aws).asScala)
 
-  def describeTable(aws: DescribeTableRequest): Future[DescribeTableResult] =
-    send[DescribeTableRequest, DescribeTableResult](aws, 
dynamoDB.describeTableAsync(aws, _))
+  def scan(aws: ScanRequest): Future[ScanResponse] =
+    send(s"ScanRequest(${aws.tableName})", dynamoDB.scan(aws).asScala)
 
-  def createTable(aws: CreateTableRequest): Future[CreateTableResult] =
-    send[CreateTableRequest, CreateTableResult](aws, 
dynamoDB.createTableAsync(aws, _))
+  def putItem(aws: PutItemRequest): Future[PutItemResponse] =
+    send(s"PutItemRequest(${aws.tableName},${formatKey(aws.item)})", 
dynamoDB.putItem(aws).asScala)
 
-  def updateTable(aws: UpdateTableRequest): Future[UpdateTableResult] =
-    send[UpdateTableRequest, UpdateTableResult](aws, 
dynamoDB.updateTableAsync(aws, _))
+  def getItem(aws: GetItemRequest): Future[GetItemResponse] =
+    send(s"GetItemRequest(${aws.tableName})", dynamoDB.getItem(aws).asScala)
 
-  def deleteTable(aws: DeleteTableRequest): Future[DeleteTableResult] =
-    send[DeleteTableRequest, DeleteTableResult](aws, 
dynamoDB.deleteTableAsync(aws, _))
+  def updateItem(aws: UpdateItemRequest): Future[UpdateItemResponse] =
+    send(s"UpdateItemRequest(${aws.tableName})", 
dynamoDB.updateItem(aws).asScala)
 
-  def query(aws: QueryRequest): Future[QueryResult] =
-    send[QueryRequest, QueryResult](aws, dynamoDB.queryAsync(aws, _))
+  def deleteItem(aws: DeleteItemRequest): Future[DeleteItemResponse] =
+    send(s"DeleteItemRequest(${aws.tableName},${formatKey(aws.key)})", 
dynamoDB.deleteItem(aws).asScala)
 
-  def scan(aws: ScanRequest): Future[ScanResult] =
-    send[ScanRequest, ScanResult](aws, dynamoDB.scanAsync(aws, _))
+  def batchWriteItem(aws: BatchWriteItemRequest): 
Future[BatchWriteItemResponse] =
+    send(s"BatchWriteItemRequest(${aws.requestItems.keySet.iterator.next})", 
dynamoDB.batchWriteItem(aws).asScala)

Review Comment:
   Looks good!



##########
src/main/scala/org/apache/pekko/persistence/dynamodb/journal/DynamoDBRecovery.scala:
##########
@@ -270,7 +270,7 @@ trait DynamoDBRecovery extends AsyncReplayMessages {
     def dynamoSummingPager(queryReq: QueryRequest, acc: Seq[Item]): 
Future[Seq[Item]] = {
       dynamo.query(queryReq).flatMap { result =>
         val currentPageItems = result.items.asScala.toSeq
-        if (!result.hasLastEvaluatedKey || result.lastEvaluatedKey.isEmpty)
+        if (!result.hasLastEvaluatedKey)

Review Comment:
   Hey @pjfanning could you just explain this change (similar changes have been 
applied elsewhere)? Why no longer doing the emptyness checks?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to