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

FINERACT-2021: migrate fund resource to MVC #3823

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

BackslashWelsh
Copy link

@BackslashWelsh BackslashWelsh commented Mar 25, 2024

Description

Initial PR on FINERACT-2021
Migrated Fund resource.

Type-safe

  • Each endpoint in Fund receives and returns a specific class instead of json.
  • To keep the type-safe request until it reaches the target service, I added classes that mirror JsonCommand/Wrapper functionality but instead of json and JsonElement have the type-safe request.
  • For this reason, I had to duplicate some services so that they could use these new classes.
  • However, some of the classes that I duplicated, such as SynchronousCommandProcessingService have a lot of code that relies on JsonCommand/Wrapper. I didn't want to completely rewrite it and create even more duplicate classes just for the sake of it. That's why the new classes that mirror JsonCommand/Wrapper temporarily have JsonCommand/Wrapper inside. I can rewrite all the other classes completely in future PRs. However, the end classes like FundWritePlatformService only use my new class.

Errors

  • All MVC errors are managed by SpringErrorHandler. To map to the correct response, it uses the existing jakarta ExceptionMapper and findMostSpecificExceptionHandler method. It also handles UnsupportedParameter errors that are thrown by Jackson failOnUnknownProperties(true).
  • I migrated from manual validation to Spring's. To make the validation error response exactly as it was, I created ValidationErrorConverter to correctly map the message and the messageCode.

Features

  • I migrated the partial parameter response (JacksonPartialResponseFilter).
  • I didn't implement the prettyPrint flag. Currently the response is always pretty printed.
  • The partial resource update looks a bit strange as it uses Optional, but I haven't found another way to implement it.

MVC

  • To run the server in MVC mode, you need to add the "mvc" profile.
  • Any bean classes I added or duplicated will only be initialized if the application has the "mvc" profile.
  • To route requests to MVC, I created MvcRoutingFilter. If Spring can handle the request, it routes it to the Spring dispatcher.

Tests

  • The integration tests will work in MVC if you run the cargo with the "mvc" profile.

Note

To work as the original classes, all duplicate classes must be up to date with the original.
Classes that I had to duplicate completely or to some extent in order to be able to use a type-safe API:

ApiRequestParameterHelper
CommandHandlerProvider
CommandProcessingService
SynchronousCommandProcessingService
CommandWrapper (CommandTypeWrapper)
CommandWrapperBuilder (CommandTypeWrapperBuilder)
JsonCommand (TypeCommand)
JacksonConverterConfig
PlatformSecurityContext
SpringSecurityPlatformSecurityContext
NewCommandSourceHandler (NewCommandSourceHandlerType)

FundsApiResource
FundsApiResourceSwagger
CreateFundCommandHandler
UpdateFundCommandHandler
FundWritePlatformService
FundWritePlatformServiceJpaRepositoryImpl
FundConfiguration

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Write the commit message as per https://github.com/apache/fineract/#pull-requests

  • Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.

  • Create/update unit or integration tests for verifying the changes made.

  • Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.

  • Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes

  • Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)

FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.

@BackslashWelsh BackslashWelsh force-pushed the FINERACT-2021/type-safe-rest-api-fund-resource branch 2 times, most recently from 89b611b to 298e65b Compare April 9, 2024 11:56
@BackslashWelsh BackslashWelsh marked this pull request as draft April 9, 2024 12:10
@BackslashWelsh BackslashWelsh force-pushed the FINERACT-2021/type-safe-rest-api-fund-resource branch 6 times, most recently from 97b700f to 89b8c84 Compare April 11, 2024 17:03
@BackslashWelsh BackslashWelsh force-pushed the FINERACT-2021/type-safe-rest-api-fund-resource branch from 89b8c84 to 4296a0c Compare April 24, 2024 11:54
@BackslashWelsh BackslashWelsh force-pushed the FINERACT-2021/type-safe-rest-api-fund-resource branch from 6b85fa6 to a0d6d5f Compare April 25, 2024 11:34
@BackslashWelsh BackslashWelsh marked this pull request as ready for review April 25, 2024 11:40
@vidakovic
Copy link
Contributor

Let's talk first...

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