[Improve] add unit test

[yuluo-yx]

No description provided.

[yuluo-yx]

yep. I think so. I am debug in my local. If has new discover, i will push. I will mark its draft later. tomsun28 ***@***.***>于2024年8月5日 周一12:37写道：

…

***@***.**** commented on this pull request. ------------------------------ In alerter/src/main/java/org/apache/hertzbeat/alert/controller/AlertDefinesController.java <#2454 (comment)>: > - return ResponseEntity.ok(Message.success(alertDefinePage)); + + return ResponseEntity.ok(Message.success( + new PageImpl<>( + alertDefinePage.getContent(), + PageRequest.of(pageIndex, pageSize, Sort.by(new Sort.Order(Sort.Direction.fromString(order), sort))), + alertDefinePage.getTotalElements())) + ); Hi, it maybe caused by test cases or other reasons. I feel that we cannot repeat the construction like this. — Reply to this email directly, view it on GitHub <#2454 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ASS2GCIP55OT2HPTIBFACADZP36RDAVCNFSM6AAAAABL6OCNHCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMJXHE4TGNJZGI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[yuluo-yx]

@tomsun28 I find question: pls see: spring-projects/spring-data-commons#2987

[tomsun28]

@tomsun28 I find question: pls see: spring-projects/spring-data-commons#2987

👍 it seem this can fix it. spring-projects/spring-data-commons#2987 (comment) add pageModule in jackson

[yuluo-yx]

@tomsun28 I find question: pls see: spring-projects/spring-data-commons#2987

👍 it seem this can fix it. spring-projects/spring-data-commons#2987 (comment) add pageModule in jackson

This would be due to additional class configuration, not sure if there are other side effects, I think it's better to use new PageImpl to be safe. 🤔

[tomsun28]

This would be due to additional class configuration, not sure if there are other side effects, I think it's better to use new PageImpl to be safe. 🤔

hi, the root cause is the serialization problem of the page. It is recommended that we solve the problem instead of circumventing it. we has the custom jackson config here like timemodule https://github.com/apache/hertzbeat/blob/master/manager/src/main/java/org/apache/hertzbeat/manager/config/JacksonConfig.java

builder.modules(new SpringDataJacksonConfiguration.PageModule());

[yuluo-yx]

hi, the root cause is the serialization problem of the page. It is recommended that we solve the problem instead of circumventing it. we has the custom jackson config here like timemodule https://github.com/apache/hertzbeat/blob/master/manager/src/main/java/org/apache/hertzbeat/manager/config/JacksonConfig.java

builder.modules(new SpringDataJacksonConfiguration.PageModule());

got it, let me try

[yuluo-yx]

@tomsun28 hi, This e2e error is probably caused by #2486, I think we can run the ci test again after the #2486 merge.

cc @crossoverJie

[tomsun28]

hi，this will conflict with current existed JacksonConfig class

[yuluo-yx]

got it, The json serialization piece doesn't look easy to fix. 🤔

[tomsun28]

how about add builder.modules(new SpringDataJacksonConfiguration.PageModule()); in JacksonConfig like this.

[yuluo-yx]

I don't think so, this would still report an error inside the unit test because the unit test is only tested against the current class. From the application level it won't.

Maybe there's another solution, let me research it and see 🤔

[tomsun28]

🤔 🤔 🤔 🤔 🤔

[yuluo-yx]

For this pr, the current state is that it may affect all controllers that use Page as a return value, but it does not affect front-end api calls. It only appears in e2e tests and unit tests, so this pr is in pending status, and will be updated again when there is a better solution for serializing Page objects in spring data jpa.