Some minor improvements (#65)

- NPE fix in Filters - Better error message in Serializer & Repository
p3t · Dec 3, 2024 · fbf1cfd · fbf1cfd
1 parent 54d65b4
commit fbf1cfd
Show file tree

Hide file tree

Showing 9 changed files with 195 additions and 39 deletions.
diff --git a/cursorpaging-jpa-api/src/main/java/io/vigier/cursorpaging/jpa/serializer/FromDtoMapper.java b/cursorpaging-jpa-api/src/main/java/io/vigier/cursorpaging/jpa/serializer/FromDtoMapper.java
@@ -44,7 +44,7 @@ class FromDtoMapper<E> {
     private final ConversionService conversionService;
     private final Map<String, RuleFactory> ruleFactories;
 
-    public  static <T> FromDtoMapper<T> create( final Consumer<FromDtoMapperBuilder<T>> c ) {
+    public static <T> FromDtoMapper<T> create( final Consumer<FromDtoMapperBuilder<T>> c ) {
         final var builder = FromDtoMapper.<T>builder();
         c.accept( builder );
         return builder.build();
@@ -87,7 +87,7 @@ private FilterList filters() {
 
     private FilterList fromFilterListDto( final Cursor.FilterList dto ) {
         List<QueryElement> filters = new LinkedList<>();
-        
+
         filters.addAll( dto.getFiltersList().stream().map( this::fromFilterDto ).toList() );
         filters.addAll( dto.getFilterListsList().stream().map( this::fromFilterListDto ).toList() );
         return switch ( dto.getType() ) {
@@ -121,18 +121,25 @@ private <T extends Comparable<? super T>> T valueOf( final Attribute attribute,
         if ( value.getValue().isEmpty() ) {
             return null;
         }
-        return conversionService.<T>convert( value.getValue(), attribute.type() );
+        final var convert = conversionService.<T>convert( value.getValue(), attribute.type() );
+        if ( convert == null ) {
+            throw new SerializerException(
+                    "Cannot convert value: " + value.getValue() + " to type: " + attribute.type() );
+        }
+        return convert;
     }
 
     private Position positionOf( final Cursor.Position position ) {
         final var attribute = attributeOf( position.getAttribute() );
 
-        return Position.create( b -> b.attribute( attribute ).value( valueOf( attribute, position.getValue() ) )
+        return Position.create( b -> b.attribute( attribute )
+                .value( valueOf( attribute, position.getValue() ) )
                 .order( switch ( position.getOrder() ) {
                     case ASC -> Order.ASC;
                     case DESC -> Order.DESC;
                     case UNRECOGNIZED -> throw new IllegalArgumentException( "Unrecognized order" );
-                } ).reversed( position.getReversed() ) );
+                } )
+                .reversed( position.getReversed() ) );
     }
 
     private Attribute attributeOf( final Cursor.Attribute attribute ) {

diff --git a/...paging-jpa-api/src/main/java/io/vigier/cursorpaging/jpa/serializer/RequestSerializer.java b/...paging-jpa-api/src/main/java/io/vigier/cursorpaging/jpa/serializer/RequestSerializer.java
@@ -6,12 +6,14 @@
 import io.vigier.cursorpaging.jpa.serializer.dto.Cursor;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Consumer;
 import lombok.Builder;
 import lombok.SneakyThrows;
 import org.springframework.core.convert.ConversionService;
 import org.springframework.core.convert.TypeDescriptor;
+import org.springframework.lang.Nullable;
 
 @Builder
 public class RequestSerializer<E> {
@@ -29,12 +31,9 @@ public boolean canConvert( final Class<?> sourceType, final Class<?> targetType
             if ( sourceType == null || targetType == null ) {
                 return false;
             }
-            return sourceType.isAssignableFrom( String.class ) && (
-                    targetType.isAssignableFrom( Long.class )
-                            || targetType.isAssignableFrom( Integer.class )
-                            || targetType.isAssignableFrom( Boolean.class )
-                            || targetType.isAssignableFrom( String.class )
-                            || targetType.isAssignableFrom( Object.class ) );
+            return sourceType.isAssignableFrom( String.class ) && (targetType.isAssignableFrom( Long.class )
+                    || targetType.isAssignableFrom( Integer.class ) || targetType.isAssignableFrom( Boolean.class )
+                    || targetType.isAssignableFrom( String.class ) || targetType.isAssignableFrom( Object.class ));
         }
 
         @Override
@@ -47,18 +46,19 @@ public boolean canConvert( final TypeDescriptor sourceType, final TypeDescriptor
 
         @Override
         public <T> T convert( final Object source, final Class<T> targetType ) {
-            if (targetType == String.class)
+            if ( targetType == String.class ) {
                 return (T) source.toString();
-            if (targetType == Integer.class) {
+            }
+            if ( targetType == Integer.class ) {
                 return (T) Integer.valueOf( source.toString() );
             }
-            if (targetType == Long.class) {
+            if ( targetType == Long.class ) {
                 return (T) Long.valueOf( source.toString() );
             }
-            if(targetType == Boolean.class) {
+            if ( targetType == Boolean.class ) {
                 return (T) Boolean.valueOf( source.toString().equals( "true" ) );
             }
-            if(targetType==Object.class) {
+            if ( targetType == Object.class ) {
                 return (T) source;
             }
             return null;
@@ -68,7 +68,7 @@ public <T> T convert( final Object source, final Class<T> targetType ) {
         public Object convert( final Object source, final TypeDescriptor sourceType, final TypeDescriptor targetType ) {
             return convert( source, targetType.getObjectType() );
         }
-    } ;
+    };
 
     @Builder.Default
     private final Map<String, RuleFactory> filterRuleFactories = new HashMap<>();
@@ -133,4 +133,16 @@ public PageRequest<E> toPageRequest( final byte[] data ) {
     public PageRequest<E> toPageRequest( final Base64String base64 ) {
         return toPageRequest( base64.decoded() );
     }
+
+    /**
+     * Converts a base64 encoded String into a page-request
+     *
+     * @param cursorStr The encoded string (can be {@code null})
+     * @return a present optional if the input value was present.
+     */
+    public Optional<PageRequest<E>> stringToPageRequest( @Nullable final String cursorStr ) {
+        return Optional.ofNullable( cursorStr != null ? (!cursorStr.isBlank() ? cursorStr : null) : null )
+                .map( Base64String::new )
+                .map( this::toPageRequest );
+    }
 }
diff --git a/...ging-jpa-api/src/main/java/io/vigier/cursorpaging/jpa/serializer/SerializerException.java b/...ging-jpa-api/src/main/java/io/vigier/cursorpaging/jpa/serializer/SerializerException.java
@@ -0,0 +1,7 @@
+package io.vigier.cursorpaging.jpa.serializer;
+
+public class SerializerException extends RuntimeException {
+    public SerializerException( String message ) {
+        super( message );
+    }
+}
diff --git a/cursorpaging-jpa-api/src/test/java/io/vigier/cursorpaging/jpa/serializer/SerializerTest.java b/cursorpaging-jpa-api/src/test/java/io/vigier/cursorpaging/jpa/serializer/SerializerTest.java
@@ -14,6 +14,7 @@
 import java.util.List;
 import java.util.Map;
 import lombok.Data;
+import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -33,18 +34,35 @@
 class SerializerTest {
 
     @Data
-    private static class TestEntity {
+    private static class ValueClass implements Comparable<ValueClass> {
+        private final String theValue;
+
+        @Override
+        public int compareTo( final ValueClass o ) {
+            return theValue.compareTo( o.theValue );
+        }
+    }
 
+    @Data
+    private static class TestEntity {
         private Long id;
         private String name;
+        private ValueClass value;
     }
 
     private static class TestEntity_ {
-
         @SuppressWarnings( "unchecked" )
         public static volatile SingularAttribute<TestEntity, Long> id = Mockito.mock( SingularAttribute.class );
         @SuppressWarnings( "unchecked" )
         public static volatile SingularAttribute<TestEntity, String> name = Mockito.mock( SingularAttribute.class );
+        @SuppressWarnings( "unchecked" )
+        public static volatile SingularAttribute<TestEntity, ValueClass> value = Mockito.mock(
+                SingularAttribute.class );
+    }
+
+    private static class ValueClass_ {
+        @SuppressWarnings( "unchecked" )
+        public static volatile SingularAttribute<ValueClass, String> theValue = Mockito.mock( SingularAttribute.class );
     }
 
     @Mock
@@ -54,20 +72,88 @@ private static class TestEntity_ {
     static void setup() {
         when( TestEntity_.name.getJavaType() ).thenReturn( String.class );
         when( TestEntity_.name.getName() ).thenReturn( "name" );
+        lenient().when( TestEntity_.value.getJavaType() ).thenReturn( ValueClass.class );
+        lenient().when( TestEntity_.value.getName() ).thenReturn( "value" );
         lenient().when( TestEntity_.id.getJavaType() ).thenReturn( Long.class );
         lenient().when( TestEntity_.id.getName() ).thenReturn( "id" );
+        lenient().when( ValueClass_.theValue.getJavaType() ).thenReturn( String.class );
+        lenient().when( ValueClass_.theValue.getName() ).thenReturn( "theValue" );
     }
 
     @Test
     void shouldSerializePageRequests() {
         final PageRequest<TestEntity> pageRequest = PageRequest.create( b -> b.desc( TestEntity_.name ) );
 
-        final RequestSerializer<TestEntity> serializer = RequestSerializer.create(
-                b -> b.use( Attribute.of( TestEntity_.name ) ) );
-        final var serializedRequest = serializer.toBytes( pageRequest );
-        final var deserializeRequest = serializer.toPageRequest( serializedRequest );
+        final var deserializeRequest = serializeAndDeserialize( pageRequest );
+
+        assertThat( deserializeRequest ).isEqualTo( pageRequest );
+    }
+
+    @Test
+    void shouldSerializePageRequestsWithPosition() {
+        final PageRequest<TestEntity> pageRequest = PageRequest.create( b -> b.position( Position.create(
+                p -> p.order( Order.ASC ).attribute( Attribute.of( TestEntity_.id ) ).value( 4711L ) ) ) );
+
+        final var deserializeRequest = serializeAndDeserialize( pageRequest );
 
         assertThat( deserializeRequest ).isEqualTo( pageRequest );
+        assertThat( deserializeRequest.isFirstPage() ).isFalse();
+        assertThat( deserializeRequest.positions() ).first()
+                .satisfies( p -> assertThat( p.value() ).isEqualTo( 4711L ) );
+    }
+
+    @Test
+    void shouldThrowExceptionWhenNotDeserializeable() {
+        // The 'value' in the position of type ValueClass is can be serialized (via toString)
+        // but not converted back (no converter configured)
+        final PageRequest<TestEntity> pageRequest = PageRequest.create( b -> b.position( Position.create(
+                p -> p.order( Order.ASC )
+                        .attribute( Attribute.of( TestEntity_.value ) )
+                        .value( new ValueClass( "123" ) ) ) ) );
+
+        Assertions.assertThatThrownBy( () -> serializeAndDeserialize( pageRequest ) )
+                .isInstanceOf( SerializerException.class )
+                .hasMessageContaining( "Cannot convert value" );
+    }
+
+    @Test
+    void shouldDeserializePositionsWithPathAttributes() {
+        final PageRequest<TestEntity> request = PageRequest.create( r -> r.position( Position.create(
+                pos -> pos.attribute( Attribute.of( TestEntity_.value, ValueClass_.theValue ) )
+                        .value( "123" )
+                        .order( Order.ASC ) ) ) );
+
+        final var deserializeRequest = serializeAndDeserialize( request );
+
+        assertThat( deserializeRequest.positions() ).hasSize( 1 );
+        final var pos = deserializeRequest.positions().getFirst();
+        assertThat( pos.attribute().attributes() ).hasSize( 2 );
+    }
+
+    @Test
+    void shouldAcceptNullAsCursorString() {
+        assertThat( getRequestSerializer().stringToPageRequest( null ) ).isEmpty();
+    }
+
+    @Test
+    void shouldDeserializeFromCursorString() {
+        final PageRequest<TestEntity> request = PageRequest.create( r -> r.asc( TestEntity_.id ) );
+        final var requestSerializer = getRequestSerializer();
+        final String cursor = requestSerializer.toBase64( request ).toString();
+        assertThat( requestSerializer.stringToPageRequest( cursor ) ).isPresent().get().isEqualTo( request );
+    }
+
+    private static PageRequest<TestEntity> serializeAndDeserialize( final PageRequest<TestEntity> pageRequest ) {
+        final var serializer = getRequestSerializer();
+        final var serializedRequest = serializer.toBase64( pageRequest );
+        return serializer.toPageRequest( serializedRequest );
+    }
+
+    private static RequestSerializer<TestEntity> getRequestSerializer() {
+        return RequestSerializer.create( b -> b.use( Attribute.of( TestEntity_.id ) )
+                .use( Attribute.of( TestEntity_.name ) )
+                .use( Attribute.of( TestEntity_.value ) )
+                .use( Attribute.of( ValueClass_.theValue ) ) );
     }
 
     @Test

diff --git a/cursorpaging-jpa/src/main/java/io/vigier/cursorpaging/jpa/Attribute.java b/cursorpaging-jpa/src/main/java/io/vigier/cursorpaging/jpa/Attribute.java
@@ -117,7 +117,7 @@ Comparable<?> valueOf( final Object entity ) {
         return (Comparable<?>) result;
     }
 
-    List<SingleAttribute> attributes() {
+    public List<SingleAttribute> attributes() {
         return attributes;
     }
 
@@ -195,9 +195,11 @@ public Attribute withIgnoreCase() {
     public boolean ignoreCase() {
         return ignoreCase;
     }
-
+    
     public Comparable<?> verify( final Comparable<?> value ) {
-        if ( type().isAssignableFrom( value.getClass() ) ) {
+        if ( value == null ) {
+            return null;
+        } else if ( type().isAssignableFrom( value.getClass() ) ) {
             return type().cast( value );
         } else if ( value instanceof Integer && typeIsInteger() ) {
             return value;
@@ -246,10 +248,10 @@ private boolean typeIsLong() {
     }
 
     public List<? extends Comparable<?>> verify( final List<? extends Comparable<?>> values ) {
-        return values.stream().map( this::verify ).toList();
+        return values != null ? values.stream().map( this::verify ).toList() : List.of();
     }
 
     public Comparable<?>[] verify( final Comparable<?>... values ) {
-        return Arrays.stream( values ).map( this::verify ).toArray( Comparable<?>[]::new );
+        return values != null ? Arrays.stream( values ).map( this::verify ).toArray( Comparable<?>[]::new ) : null;
     }
 }
diff --git a/cursorpaging-jpa/src/main/java/io/vigier/cursorpaging/jpa/Page.java b/cursorpaging-jpa/src/main/java/io/vigier/cursorpaging/jpa/Page.java
@@ -25,7 +25,7 @@ public class Page<E> implements Iterable<E> {
      * The content of this page.
      */
     private final List<E> content;
-    
+
     /**
      * The request used to fetching this page.
      */
@@ -49,6 +49,16 @@ public static <E> Page<E> create( final Consumer<PageBuilder<E>> creator ) {
         return builder.build();
     }
 
+    public static <E> Page<E> empty() {
+        return Page.<E>builder().content( Collections.emptyList() )
+                .build();
+    }
+
+    public static <E> Page<E> empty( final PageRequest<E> self ) {
+        return Page.<E>builder().content( Collections.emptyList() ).self( self )
+                .build();
+    }
+
     /**
      * Get an iterator over the content of this page.
      *

diff --git a/cursorpaging-jpa/src/main/java/io/vigier/cursorpaging/jpa/PageRequest.java b/cursorpaging-jpa/src/main/java/io/vigier/cursorpaging/jpa/PageRequest.java
@@ -21,10 +21,10 @@
 /**
  * A request, which can be used to query a database for a page of entities.
  * <p>
- * The request uses a list of positions (one for each attribute which should be used to address the start of the page).
- * The combination of all positions must uniquely address a certain record in the table. This can i.e. achieved by
- * adding the primary id of the entity as secondary position, if the first position is not unique, but should be the
- * primary order (e.g. like a created-date).
+ * The request uses a list of `positions` (one for each attribute which should be used to address the start of the
+ * page). The combination of all positions must uniquely address a certain record in the table. This can be achieved by
+ * adding the primary id of the entity as secondary position, if the first position is not unique (like a `name`, or
+ * date), but should be the primary order.
  *
  * @param <E> the entity type
  */
@@ -162,7 +162,7 @@ public PageRequestBuilder<E> desc( final String name, final Class<? extends Comp
          * @return the builder
          */
         public PageRequestBuilder<E> filter( final QueryElement filter ) {
-            List<QueryElement> filters = new LinkedList<>();
+            final List<QueryElement> filters = new LinkedList<>();
             if ( this.filters$value != null ) {
                 filters.addAll( this.filters$value.filters() );
             }
@@ -217,8 +217,8 @@ public static <E> PageRequest<E> create( final Consumer<PageRequestBuilder<E>> c
      * @param c customizer for the copy
      * @return A new page-request with existing and customized attributes
      */
-    public PageRequest<E> copy( Consumer<PageRequestBuilder<E>> c ) {
-        PageRequestBuilder<E> builder = PageRequest.<E>builder()
+    public PageRequest<E> copy( final Consumer<PageRequestBuilder<E>> c ) {
+        final PageRequestBuilder<E> builder = PageRequest.<E>builder()
                 .totalCount( totalCount )
                 .enableTotalCount( enableTotalCount )
                 .pageSize( pageSize );
@@ -241,7 +241,7 @@ public PageRequest<E> copy( Consumer<PageRequestBuilder<E>> c ) {
      *
      * @return A copy of the page-request where the total-count is removed and the enable flag is set accordingly
      */
-    public PageRequest<E> withEnableTotalCount( boolean enable ) {
+    public PageRequest<E> withEnableTotalCount( final boolean enable ) {
         return copy( b -> b.enableTotalCount( enable ).totalCount( null ) );
     }
 
@@ -275,10 +275,10 @@ public PageRequest<E> toReversed() {
     }
 
     public boolean isFirstPage() {
-        return positions.get( 0 ).isFirst();
+        return positions.getFirst().isFirst();
     }
 
     public boolean isReversed() {
-        return positions.get( 0 ).reversed();
+        return positions.getFirst().reversed();
     }
 }