derjust / spring-data-dynamodb

This module deals with enhanced support for a data access layer built on AWS DynamoDB.
https://derjust.github.io/spring-data-dynamodb/
Apache License 2.0
402 stars 141 forks source link

API calls not adding `Limit` param and `Pageable` object ignored #232

Open csokol opened 5 years ago

csokol commented 5 years ago

(this might be a duplicate of #89 but I'm not completely sure as this is also related to query methods)

Expected Behavior

Repository should consider Pageable object and AWS API calls should have the Limit param.

Actual Behavior

When using Pageable objects in repository methods, spring data seems to be ignoring the page request. When limiting the results via the query method name (such as findTopYByX) the results seems to be limited, but by looking at AWS SDK logs, the actual query sent to DynamoDB actually has no Limit param so I suppose the limit is done in memory.

Steps to Reproduce the Problem

Given a Java project with spring boot, lombok, junit 5 and proper logging configured, the following classes will demonstrate the problem:

@DynamoDBTable(tableName = "Album")
@NoArgsConstructor
@AllArgsConstructor
public class Album {

  @Id
  private AlbumId albumId;

  @DynamoDBHashKey(attributeName = "artistId")
  public String getArtistId() {
    return Optional.ofNullable(albumId).map(AlbumId::getArtistId).orElse(null);
  }

  public void setArtistId(String artistId) {
    if (albumId == null) {
      albumId = new AlbumId();
    }
    albumId.setArtistId(artistId);
  }

  @DynamoDBRangeKey(attributeName = "index")
  public Integer getIndex() {
    return Optional.ofNullable(albumId).map(AlbumId::getIndex).orElse(null);
  }

  public void setIndex(Integer index) {
    if (albumId == null) {
      albumId = new AlbumId();
    }
    albumId.setIndex(index);
  }

  @Data
  @AllArgsConstructor
  @NoArgsConstructor
  public static class AlbumId {
    @DynamoDBHashKey(attributeName = "artistId")
    private String artistId;

    @DynamoDBRangeKey(attributeName = "index")
    private Integer index;
  }
}

public interface AlbumRepository extends CrudRepository<Album, Album.AlbumId> {

  List<AccountBalance> findTop2ByArtistIdOrderByIndex(String artistId);
  List<AccountBalance> findByArtistIdOrderByIndex(String artistId, Pageable pageable);

}

@ExtendWith(SpringExtension.class)
@SpringBootTest(
  webEnvironment = SpringBootTest.WebEnvironment.NONE,
  classes = SomeApp.class
)
class AlbumRepositoryIntegrationTest {

  @Autowired
  private AlbumRepository albumRepository;

  @Test
  void shouldFindMostRecentBalance() {
    albumRepository.save(new Album(new Album.AlbumId("1", 1)));
    albumRepository.save(new Album(new Album.AlbumId("1", 2)));
    albumRepository.save(new Album(new Album.AlbumId("1", 3)));

    var r1 = albumRepository.findTop2ByArtistIdOrderByIndex("1");
    System.out.println("Results: " + r1.size());

    var r2 = albumRepository.findByArtistIdOrderByIndex("1", PageRequest.of(0, 2));
    System.out.println("Results: " + r2.size());
  }

}

The output can be checked here: https://gist.github.com/csokol/f6b88e7f55002f80150ebdf5f8ca7d6a

By looking at lines 94 and 118 of the gist. It's possible to see that the queries are sent without the Limit param.

Specifications

derjust commented 5 years ago

Yes, you are correct. Limit has been added via #239 and will be part of 5.1.1 Pageable has a PR I'm working (#117) so that will be supported in the near future, too.

Khangaldyan commented 5 years ago

@csokol have you found a workaround before the 5.1.1 version release?

csokol commented 5 years ago

No, I didn't. In the end we migrated our code to use the dynamo mapper lib officially supported by aws.

Em dom, 12 de mai de 2019 20:12, Khangaldyan notifications@github.com escreveu:

@csokol https://github.com/csokol have you found a workaround before the 5.1.1 version release?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/derjust/spring-data-dynamodb/issues/232#issuecomment-491616865, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHUSDQMGDNO5ZRDNWPKGALPVBMXHANCNFSM4GTS3VRQ .