Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

Open
csokol opened this issue Jan 31, 2019 · 3 comments
Open

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

csokol opened this issue Jan 31, 2019 · 3 comments

Comments

@csokol
Copy link

csokol commented Jan 31, 2019

(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

  • Spring Data DynamoDB Version: com.github.derjust:spring-data-dynamodb:jar:5.0.3
  • Spring Data Version: org.springframework.data:spring-data-commons:jar:2.0.11.RELEASE
  • AWS SDK Version: com.amazonaws:aws-java-sdk-dynamodb:jar:1.11.336
  • Java Version: Java(TM) SE Runtime Environment 18.9 (build 11+28)
  • Platform Details: macOS
@derjust
Copy link
Owner

derjust commented Mar 11, 2019

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
Copy link

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

@csokol
Copy link
Author

csokol commented May 16, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants