galovics commented on code in PR #2777: URL: https://github.com/apache/fineract/pull/2777#discussion_r1065847510
########## fineract-provider/src/main/java/org/apache/fineract/cob/api/LoanCOBCatchUpApiResource.java: ########## @@ -0,0 +1,73 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.cob.api; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.responses.ApiResponses; +import io.swagger.v3.oas.annotations.tags.Tag; +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import lombok.RequiredArgsConstructor; +import org.apache.fineract.cob.data.OldestCOBProcessedLoanDTO; +import org.apache.fineract.cob.service.LoanCOBCatchUpService; +import org.apache.fineract.infrastructure.core.serialization.DefaultToApiJsonSerializer; +import org.springframework.stereotype.Component; + +@Path("/loans") +@Component +@Tag(name = "Loan COB Catch Up", description = "") +@RequiredArgsConstructor +public class LoanCOBCatchUpApiResource { + + private final DefaultToApiJsonSerializer<OldestCOBProcessedLoanDTO> oldestCOBProcessedLoanSerializeService; + private final LoanCOBCatchUpService loanCOBCatchUpService; + + @GET + @Path("oldest_cob_processed_loan") Review Comment: 1. Let's use dashes instead of underscores. 2. Naming could be a bit better. `/loans` is already in the API path so let's drop the loan postfix. `processed` is not the best way to name it cause the terminology is `closed`. So it could be named like `/loans/oldest-closed` ########## fineract-provider/src/main/java/org/apache/fineract/cob/service/AsyncLoanCOBExecutorServiceImpl.java: ########## @@ -0,0 +1,93 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.cob.service; + +import java.time.LocalDate; +import java.time.format.DateTimeFormatter; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import lombok.RequiredArgsConstructor; +import org.apache.fineract.cob.loan.LoanCOBConstant; +import org.apache.fineract.infrastructure.businessdate.domain.BusinessDateType; +import org.apache.fineract.infrastructure.core.domain.FineractContext; +import org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil; +import org.apache.fineract.infrastructure.jobs.data.JobParameterDTO; +import org.apache.fineract.infrastructure.jobs.domain.ScheduledJobDetail; +import org.apache.fineract.infrastructure.jobs.domain.ScheduledJobDetailRepository; +import org.apache.fineract.infrastructure.jobs.exception.JobNotFoundException; +import org.apache.fineract.infrastructure.jobs.service.JobStarter; +import org.apache.fineract.portfolio.loanaccount.domain.Loan; +import org.apache.fineract.portfolio.loanaccount.domain.LoanRepository; +import org.springframework.batch.core.Job; +import org.springframework.batch.core.JobParametersInvalidException; +import org.springframework.batch.core.configuration.JobLocator; +import org.springframework.batch.core.launch.NoSuchJobException; +import org.springframework.batch.core.repository.JobExecutionAlreadyRunningException; +import org.springframework.batch.core.repository.JobInstanceAlreadyCompleteException; +import org.springframework.batch.core.repository.JobRestartException; +import org.springframework.scheduling.annotation.Async; +import org.springframework.stereotype.Service; + +@Service +@RequiredArgsConstructor +public class AsyncLoanCOBExecutorServiceImpl implements AsyncLoanCOBExecutorService { + + private final LoanRepository loanRepository; + private final JobLocator jobLocator; + private final ScheduledJobDetailRepository scheduledJobDetailRepository; + private final JobStarter jobStarter; + + @Override + @Async("threadPoolTaskExecutor") Review Comment: This is way too generic. Let's use a specific threadpool for this. ########## fineract-provider/src/main/java/org/apache/fineract/cob/api/LoanCOBCatchUpApiResource.java: ########## @@ -0,0 +1,73 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.cob.api; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.responses.ApiResponses; +import io.swagger.v3.oas.annotations.tags.Tag; +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import lombok.RequiredArgsConstructor; +import org.apache.fineract.cob.data.OldestCOBProcessedLoanDTO; +import org.apache.fineract.cob.service.LoanCOBCatchUpService; +import org.apache.fineract.infrastructure.core.serialization.DefaultToApiJsonSerializer; +import org.springframework.stereotype.Component; + +@Path("/loans") +@Component +@Tag(name = "Loan COB Catch Up", description = "") +@RequiredArgsConstructor +public class LoanCOBCatchUpApiResource { + + private final DefaultToApiJsonSerializer<OldestCOBProcessedLoanDTO> oldestCOBProcessedLoanSerializeService; + private final LoanCOBCatchUpService loanCOBCatchUpService; + + @GET + @Path("oldest_cob_processed_loan") + @Consumes({ MediaType.APPLICATION_JSON }) + @Produces({ MediaType.APPLICATION_JSON }) + @Operation(summary = "Retrieves the oldest COB processed loan", description = "Retrieves the COB business date and the oldest COB processed loan") + @ApiResponses({ + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = LoanCOBCatchUpApiResourceSwagger.GetOldestCOBProcessedLoanResponse.class))) }) + public String getOldestCOBProcessedLoan() { + OldestCOBProcessedLoanDTO response = loanCOBCatchUpService.getOldestCOBProcessedLoan(); + + return oldestCOBProcessedLoanSerializeService.serialize(response); + } + + @POST + @Path("loan_cob_catch_up") + @Consumes({ MediaType.APPLICATION_JSON }) + @Produces({ MediaType.APPLICATION_JSON }) + @Operation(summary = "Executes Loan COB Catch Up", description = "Executes the Loan COB job on every day from the oldest Loan to the current COB business date") + @ApiResponses({ @ApiResponse(responseCode = "200", description = "OK") }) + public Response executeLoanCOBCatchUp() { + loanCOBCatchUpService.executeLoanCOBCatchUp(); + + return Response.status(202).build(); Review Comment: Didn't we discuss returning an ID that identifies the catch-up so that it can be easily polled from the UI? ########## fineract-provider/src/main/java/org/apache/fineract/cob/api/LoanCOBCatchUpApiResource.java: ########## @@ -0,0 +1,73 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.cob.api; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.responses.ApiResponses; +import io.swagger.v3.oas.annotations.tags.Tag; +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import lombok.RequiredArgsConstructor; +import org.apache.fineract.cob.data.OldestCOBProcessedLoanDTO; +import org.apache.fineract.cob.service.LoanCOBCatchUpService; +import org.apache.fineract.infrastructure.core.serialization.DefaultToApiJsonSerializer; +import org.springframework.stereotype.Component; + +@Path("/loans") +@Component +@Tag(name = "Loan COB Catch Up", description = "") +@RequiredArgsConstructor +public class LoanCOBCatchUpApiResource { + + private final DefaultToApiJsonSerializer<OldestCOBProcessedLoanDTO> oldestCOBProcessedLoanSerializeService; + private final LoanCOBCatchUpService loanCOBCatchUpService; + + @GET + @Path("oldest_cob_processed_loan") + @Consumes({ MediaType.APPLICATION_JSON }) + @Produces({ MediaType.APPLICATION_JSON }) + @Operation(summary = "Retrieves the oldest COB processed loan", description = "Retrieves the COB business date and the oldest COB processed loan") + @ApiResponses({ + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = LoanCOBCatchUpApiResourceSwagger.GetOldestCOBProcessedLoanResponse.class))) }) + public String getOldestCOBProcessedLoan() { + OldestCOBProcessedLoanDTO response = loanCOBCatchUpService.getOldestCOBProcessedLoan(); + + return oldestCOBProcessedLoanSerializeService.serialize(response); + } + + @POST + @Path("loan_cob_catch_up") Review Comment: Naming (based on the above suggestion) could be POST `/loans/catch-up` ########## fineract-provider/src/main/java/org/apache/fineract/cob/api/LoanCOBCatchUpApiResource.java: ########## @@ -0,0 +1,73 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.cob.api; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.responses.ApiResponses; +import io.swagger.v3.oas.annotations.tags.Tag; +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import lombok.RequiredArgsConstructor; +import org.apache.fineract.cob.data.OldestCOBProcessedLoanDTO; +import org.apache.fineract.cob.service.LoanCOBCatchUpService; +import org.apache.fineract.infrastructure.core.serialization.DefaultToApiJsonSerializer; +import org.springframework.stereotype.Component; + +@Path("/loans") +@Component +@Tag(name = "Loan COB Catch Up", description = "") +@RequiredArgsConstructor +public class LoanCOBCatchUpApiResource { + + private final DefaultToApiJsonSerializer<OldestCOBProcessedLoanDTO> oldestCOBProcessedLoanSerializeService; + private final LoanCOBCatchUpService loanCOBCatchUpService; + + @GET + @Path("oldest_cob_processed_loan") + @Consumes({ MediaType.APPLICATION_JSON }) + @Produces({ MediaType.APPLICATION_JSON }) + @Operation(summary = "Retrieves the oldest COB processed loan", description = "Retrieves the COB business date and the oldest COB processed loan") + @ApiResponses({ + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = LoanCOBCatchUpApiResourceSwagger.GetOldestCOBProcessedLoanResponse.class))) }) + public String getOldestCOBProcessedLoan() { + OldestCOBProcessedLoanDTO response = loanCOBCatchUpService.getOldestCOBProcessedLoan(); + + return oldestCOBProcessedLoanSerializeService.serialize(response); + } + + @POST + @Path("loan_cob_catch_up") + @Consumes({ MediaType.APPLICATION_JSON }) + @Produces({ MediaType.APPLICATION_JSON }) + @Operation(summary = "Executes Loan COB Catch Up", description = "Executes the Loan COB job on every day from the oldest Loan to the current COB business date") + @ApiResponses({ @ApiResponse(responseCode = "200", description = "OK") }) + public Response executeLoanCOBCatchUp() { + loanCOBCatchUpService.executeLoanCOBCatchUp(); Review Comment: Bonus: This way the system can be easily overloaded by just spamming this API. We should do some checks to prevent multiple executions of the catch-up at the same time. ########## fineract-provider/src/main/java/org/apache/fineract/cob/api/LoanCOBCatchUpApiResource.java: ########## @@ -0,0 +1,73 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.cob.api; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.responses.ApiResponses; +import io.swagger.v3.oas.annotations.tags.Tag; +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import lombok.RequiredArgsConstructor; +import org.apache.fineract.cob.data.OldestCOBProcessedLoanDTO; +import org.apache.fineract.cob.service.LoanCOBCatchUpService; +import org.apache.fineract.infrastructure.core.serialization.DefaultToApiJsonSerializer; +import org.springframework.stereotype.Component; + +@Path("/loans") +@Component +@Tag(name = "Loan COB Catch Up", description = "") +@RequiredArgsConstructor +public class LoanCOBCatchUpApiResource { + + private final DefaultToApiJsonSerializer<OldestCOBProcessedLoanDTO> oldestCOBProcessedLoanSerializeService; + private final LoanCOBCatchUpService loanCOBCatchUpService; + + @GET + @Path("oldest_cob_processed_loan") + @Consumes({ MediaType.APPLICATION_JSON }) + @Produces({ MediaType.APPLICATION_JSON }) + @Operation(summary = "Retrieves the oldest COB processed loan", description = "Retrieves the COB business date and the oldest COB processed loan") + @ApiResponses({ + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = LoanCOBCatchUpApiResourceSwagger.GetOldestCOBProcessedLoanResponse.class))) }) + public String getOldestCOBProcessedLoan() { + OldestCOBProcessedLoanDTO response = loanCOBCatchUpService.getOldestCOBProcessedLoan(); + + return oldestCOBProcessedLoanSerializeService.serialize(response); + } + + @POST + @Path("loan_cob_catch_up") + @Consumes({ MediaType.APPLICATION_JSON }) + @Produces({ MediaType.APPLICATION_JSON }) + @Operation(summary = "Executes Loan COB Catch Up", description = "Executes the Loan COB job on every day from the oldest Loan to the current COB business date") + @ApiResponses({ @ApiResponse(responseCode = "200", description = "OK") }) Review Comment: We're lying here because we're returning HTTP 202 in practice. ########## fineract-provider/src/main/java/org/apache/fineract/cob/api/LoanCOBCatchUpApiResource.java: ########## @@ -0,0 +1,73 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.cob.api; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.responses.ApiResponses; +import io.swagger.v3.oas.annotations.tags.Tag; +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import lombok.RequiredArgsConstructor; +import org.apache.fineract.cob.data.OldestCOBProcessedLoanDTO; +import org.apache.fineract.cob.service.LoanCOBCatchUpService; +import org.apache.fineract.infrastructure.core.serialization.DefaultToApiJsonSerializer; +import org.springframework.stereotype.Component; + +@Path("/loans") +@Component +@Tag(name = "Loan COB Catch Up", description = "") +@RequiredArgsConstructor +public class LoanCOBCatchUpApiResource { + + private final DefaultToApiJsonSerializer<OldestCOBProcessedLoanDTO> oldestCOBProcessedLoanSerializeService; + private final LoanCOBCatchUpService loanCOBCatchUpService; + + @GET + @Path("oldest_cob_processed_loan") + @Consumes({ MediaType.APPLICATION_JSON }) + @Produces({ MediaType.APPLICATION_JSON }) + @Operation(summary = "Retrieves the oldest COB processed loan", description = "Retrieves the COB business date and the oldest COB processed loan") + @ApiResponses({ + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = LoanCOBCatchUpApiResourceSwagger.GetOldestCOBProcessedLoanResponse.class))) }) + public String getOldestCOBProcessedLoan() { + OldestCOBProcessedLoanDTO response = loanCOBCatchUpService.getOldestCOBProcessedLoan(); + + return oldestCOBProcessedLoanSerializeService.serialize(response); + } + + @POST + @Path("loan_cob_catch_up") + @Consumes({ MediaType.APPLICATION_JSON }) + @Produces({ MediaType.APPLICATION_JSON }) + @Operation(summary = "Executes Loan COB Catch Up", description = "Executes the Loan COB job on every day from the oldest Loan to the current COB business date") + @ApiResponses({ @ApiResponse(responseCode = "200", description = "OK") }) + public Response executeLoanCOBCatchUp() { + loanCOBCatchUpService.executeLoanCOBCatchUp(); Review Comment: I think we could be a bit smarter here. The API could return 2 responses: 1. If the catch-up is being executed, HTTP 202 2. If the catch-up doesn't need to be executed since all loans are up-to-date, just return HTTP 200 a.k.a nothing to do -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
