You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "ruchiD (via GitHub)" <gi...@apache.org> on 2023/04/07 15:39:46 UTC

[GitHub] [fineract] ruchiD opened a new pull request, #3108: FINERACT-1724-Regular-loan-cob-job-having-catch-up-true

ruchiD opened a new pull request, #3108:
URL: https://github.com/apache/fineract/pull/3108

   ## Description
   Modified saving is_catch_up job parameter to custom job parameters as json.
   Describe the changes made and why they were made.
   
   Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
   
   
   ## 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.
   


-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] ruchiD commented on a diff in pull request #3108: FINERACT-1724-Regular-loan-cob-job-having-catch-up-true

Posted by "ruchiD (via GitHub)" <gi...@apache.org>.
ruchiD commented on code in PR #3108:
URL: https://github.com/apache/fineract/pull/3108#discussion_r1163654737


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/domain/JobCustomParameterRepositoryImpl.java:
##########
@@ -0,0 +1,47 @@
+/**
+ * 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.infrastructure.jobs.domain;
+
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
+import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
+import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
+import org.springframework.jdbc.core.namedparam.SqlParameterSource;
+import org.springframework.stereotype.Component;
+
+@RequiredArgsConstructor
+@Component
+public class JobCustomParameterRepositoryImpl implements JobCustomParameterRepository {
+
+    private final NamedParameterJdbcTemplate namedParameterJdbcTemplate;
+    private final DatabaseSpecificSQLGenerator databaseSpecificSQLGenerator;
+
+    @Override
+    public Long saveCustomJobParameter(String jsonString) {
+        final StringBuilder insertSqlStatementBuilder = new StringBuilder(500);
+        insertSqlStatementBuilder.append("INSERT INTO batch_custom_job_parameters (parameter_json) VALUES (:jsonString"
+                + databaseSpecificSQLGenerator.jsonAppender() + ")");
+        SqlParameterSource parameters = new MapSqlParameterSource("jsonString", jsonString);
+        namedParameterJdbcTemplate.update(insertSqlStatementBuilder.toString(), parameters);
+        final Long customParameterId = namedParameterJdbcTemplate.getJdbcTemplate()
+                .queryForObject("SELECT " + databaseSpecificSQLGenerator.lastInsertId(), Long.class);

Review Comment:
   Hi Arnold, I was not sure about this change..and i think its not fixed the way you expected in refactored code. Please elaborate a bit. 



-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] ruchiD commented on a diff in pull request #3108: FINERACT-1724-Regular-loan-cob-job-having-catch-up-true

Posted by "ruchiD (via GitHub)" <gi...@apache.org>.
ruchiD commented on code in PR #3108:
URL: https://github.com/apache/fineract/pull/3108#discussion_r1164048594


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/domain/JobCustomParameterRepositoryImpl.java:
##########
@@ -0,0 +1,47 @@
+/**
+ * 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.infrastructure.jobs.domain;
+
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
+import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
+import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
+import org.springframework.jdbc.core.namedparam.SqlParameterSource;
+import org.springframework.stereotype.Component;
+
+@RequiredArgsConstructor
+@Component
+public class JobCustomParameterRepositoryImpl implements JobCustomParameterRepository {
+
+    private final NamedParameterJdbcTemplate namedParameterJdbcTemplate;
+    private final DatabaseSpecificSQLGenerator databaseSpecificSQLGenerator;
+
+    @Override
+    public Long saveCustomJobParameter(String jsonString) {
+        final StringBuilder insertSqlStatementBuilder = new StringBuilder(500);
+        insertSqlStatementBuilder.append("INSERT INTO batch_custom_job_parameters (parameter_json) VALUES (:jsonString"
+                + databaseSpecificSQLGenerator.jsonAppender() + ")");

Review Comment:
   Fixed



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/domain/JobCustomParameterRepositoryImpl.java:
##########
@@ -0,0 +1,47 @@
+/**
+ * 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.infrastructure.jobs.domain;
+
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
+import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
+import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
+import org.springframework.jdbc.core.namedparam.SqlParameterSource;
+import org.springframework.stereotype.Component;
+
+@RequiredArgsConstructor
+@Component
+public class JobCustomParameterRepositoryImpl implements JobCustomParameterRepository {
+
+    private final NamedParameterJdbcTemplate namedParameterJdbcTemplate;
+    private final DatabaseSpecificSQLGenerator databaseSpecificSQLGenerator;
+
+    @Override
+    public Long saveCustomJobParameter(String jsonString) {
+        final StringBuilder insertSqlStatementBuilder = new StringBuilder(500);
+        insertSqlStatementBuilder.append("INSERT INTO batch_custom_job_parameters (parameter_json) VALUES (:jsonString"

Review Comment:
   Fixed



-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] ruchiD commented on a diff in pull request #3108: FINERACT-1724-Regular-loan-cob-job-having-catch-up-true

Posted by "ruchiD (via GitHub)" <gi...@apache.org>.
ruchiD commented on code in PR #3108:
URL: https://github.com/apache/fineract/pull/3108#discussion_r1164048949


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/domain/JobExecutionRepository.java:
##########
@@ -180,4 +193,32 @@ bje.JOB_INSTANCE_ID NOT IN (
                 """, Map.of("statuses", List.of(STARTED.name(), STARTING.name()), "jobName", jobName, "completedStatus", COMPLETED.name(),
                 "parameterKeyName", parameterKeyName, "parameterValue", parameterValue), Long.class);
     }
+
+    public List<Long> getRunningJobsIdsByExecutionParameter(String jobName, String jobCustomParamKeyName, String parameterKeyName,
+            String parameterValue) {
+        final StringBuilder sqlStatementBuilder = new StringBuilder();
+        String jsonString = gson.toJson(new JobParameterDTO(parameterKeyName, parameterValue));
+        sqlStatementBuilder.append(
+                "SELECT bje.JOB_EXECUTION_ID FROM BATCH_JOB_INSTANCE bji INNER JOIN BATCH_JOB_EXECUTION bje ON bji.JOB_INSTANCE_ID = bje.JOB_INSTANCE_ID INNER JOIN BATCH_JOB_EXECUTION_PARAMS bjep ON bje.JOB_EXECUTION_ID = bjep.JOB_EXECUTION_ID"
+                        + " WHERE bje.STATUS IN (:statuses) AND bji.JOB_NAME = :jobName AND bjep.KEY_NAME = :jobCustomParamKeyName AND bjep.LONG_VAL IN ("
+                        + getSubQueryForCustomJobParameters()
+                        + ") AND bje.JOB_INSTANCE_ID NOT IN (SELECT bje.JOB_INSTANCE_ID FROM BATCH_JOB_INSTANCE bji INNER JOIN BATCH_JOB_EXECUTION bje ON bji.JOB_INSTANCE_ID = bje.JOB_INSTANCE_ID"
+                        + " WHERE bje.STATUS = :completedStatus AND bji.JOB_NAME = :jobName)");
+        return namedParameterJdbcTemplate.queryForList(
+                sqlStatementBuilder.toString(), Map.of("statuses", List.of(STARTED.name(), STARTING.name()), "jobName", jobName,
+                        "completedStatus", COMPLETED.name(), "jobCustomParamKeyName", jobCustomParamKeyName, "jsonString", jsonString),
+                Long.class);
+    }
+
+    private String getSubQueryForCustomJobParameters() {
+        if (databaseTypeResolver.isMySQL()) {
+            return "SELECT cjp.id FROM batch_custom_job_parameters cjp WHERE JSON_CONTAINS(cjp.parameter_json,:jsonString)";
+        } else if (databaseTypeResolver.isPostgreSQL()) {
+            return "SELECT cjp.id FROM (SELECT id,json_array_elements(parameter_json) AS json_data FROM batch_custom_job_parameters) AS cjp WHERE (cjp.json_data ::jsonb @> :jsonString ::jsonb)";
+        } else {
+            throw new IllegalStateException("Database type is not supported for current schema " + databaseTypeResolver.databaseType());

Review Comment:
   Fixed



-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] ruchiD commented on a diff in pull request #3108: FINERACT-1724-Regular-loan-cob-job-having-catch-up-true

Posted by "ruchiD (via GitHub)" <gi...@apache.org>.
ruchiD commented on code in PR #3108:
URL: https://github.com/apache/fineract/pull/3108#discussion_r1164048159


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/domain/CustomJobParameterRepository.java:
##########
@@ -20,4 +20,4 @@
 
 import org.springframework.data.jpa.repository.JpaRepository;
 
-public interface CustomJobParameterRepository extends JpaRepository<CustomJobParameter, Long> {}
+public interface CustomJobParameterRepository extends JpaRepository<CustomJobParameter, Long>, JobCustomParameterRepository {}

Review Comment:
   Fixed



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/domain/JobCustomParameterRepository.java:
##########
@@ -0,0 +1,24 @@
+/**
+ * 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.infrastructure.jobs.domain;
+
+public interface JobCustomParameterRepository {
+
+    Long saveCustomJobParameter(String jsonString);

Review Comment:
   Fixed



-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] adamsaghy merged pull request #3108: FINERACT-1724-Regular-loan-cob-job-having-catch-up-true

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #3108:
URL: https://github.com/apache/fineract/pull/3108


-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] galovics commented on a diff in pull request #3108: FINERACT-1724-Regular-loan-cob-job-having-catch-up-true

Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #3108:
URL: https://github.com/apache/fineract/pull/3108#discussion_r1162376333


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/domain/CustomJobParameterRepository.java:
##########
@@ -20,4 +20,4 @@
 
 import org.springframework.data.jpa.repository.JpaRepository;
 
-public interface CustomJobParameterRepository extends JpaRepository<CustomJobParameter, Long> {}
+public interface CustomJobParameterRepository extends JpaRepository<CustomJobParameter, Long>, JobCustomParameterRepository {}

Review Comment:
   I think we should get rid of the JpaRepository interface here completely cause it's not working properly (the save method for example, since that's why you have the custom repository implementation).



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/database/DatabaseSpecificSQLGenerator.java:
##########
@@ -168,4 +168,14 @@ public String currentSchema() {
             throw new IllegalStateException("Database type is not supported for current schema " + databaseTypeResolver.databaseType());
         }
     }
+
+    public String jsonAppender() {
+        if (databaseTypeResolver.isMySQL()) {
+            return "";
+        } else if (databaseTypeResolver.isPostgreSQL()) {
+            return " ::json";
+        } else {
+            throw new IllegalStateException("Database type is not supported for current schema " + databaseTypeResolver.databaseType());

Review Comment:
   Copy paste typo on the error msg.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/domain/JobCustomParameterRepository.java:
##########
@@ -0,0 +1,24 @@
+/**
+ * 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.infrastructure.jobs.domain;
+
+public interface JobCustomParameterRepository {
+
+    Long saveCustomJobParameter(String jsonString);

Review Comment:
   Should be called save simply (after removing the JpaRepository)



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/domain/JobCustomParameterRepositoryImpl.java:
##########
@@ -0,0 +1,47 @@
+/**
+ * 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.infrastructure.jobs.domain;
+
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
+import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
+import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
+import org.springframework.jdbc.core.namedparam.SqlParameterSource;
+import org.springframework.stereotype.Component;
+
+@RequiredArgsConstructor
+@Component
+public class JobCustomParameterRepositoryImpl implements JobCustomParameterRepository {
+
+    private final NamedParameterJdbcTemplate namedParameterJdbcTemplate;
+    private final DatabaseSpecificSQLGenerator databaseSpecificSQLGenerator;
+
+    @Override
+    public Long saveCustomJobParameter(String jsonString) {
+        final StringBuilder insertSqlStatementBuilder = new StringBuilder(500);
+        insertSqlStatementBuilder.append("INSERT INTO batch_custom_job_parameters (parameter_json) VALUES (:jsonString"
+                + databaseSpecificSQLGenerator.jsonAppender() + ")");

Review Comment:
   This would look much better with the approach I mentioned in the generator class (like castChar).



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/database/DatabaseSpecificSQLGenerator.java:
##########
@@ -168,4 +168,14 @@ public String currentSchema() {
             throw new IllegalStateException("Database type is not supported for current schema " + databaseTypeResolver.databaseType());
         }
     }
+
+    public String jsonAppender() {

Review Comment:
   I think based on the castChar method, this should be castJson.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/domain/JobCustomParameterRepositoryImpl.java:
##########
@@ -0,0 +1,47 @@
+/**
+ * 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.infrastructure.jobs.domain;
+
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
+import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
+import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
+import org.springframework.jdbc.core.namedparam.SqlParameterSource;
+import org.springframework.stereotype.Component;
+
+@RequiredArgsConstructor
+@Component
+public class JobCustomParameterRepositoryImpl implements JobCustomParameterRepository {
+
+    private final NamedParameterJdbcTemplate namedParameterJdbcTemplate;
+    private final DatabaseSpecificSQLGenerator databaseSpecificSQLGenerator;
+
+    @Override
+    public Long saveCustomJobParameter(String jsonString) {
+        final StringBuilder insertSqlStatementBuilder = new StringBuilder(500);
+        insertSqlStatementBuilder.append("INSERT INTO batch_custom_job_parameters (parameter_json) VALUES (:jsonString"
+                + databaseSpecificSQLGenerator.jsonAppender() + ")");
+        SqlParameterSource parameters = new MapSqlParameterSource("jsonString", jsonString);
+        namedParameterJdbcTemplate.update(insertSqlStatementBuilder.toString(), parameters);
+        final Long customParameterId = namedParameterJdbcTemplate.getJdbcTemplate()
+                .queryForObject("SELECT " + databaseSpecificSQLGenerator.lastInsertId(), Long.class);

Review Comment:
   Formatting here pls. Bonus point if you put this into a constant so this is cached. (note: not necessarily a constant into this class but somewhere else where anybody can reuse it)



##########
fineract-provider/src/main/resources/db/changelog/tenant/parts/0103_modify_parameter_json_column_custom_job_parameters.xml:
##########
@@ -0,0 +1,30 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    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.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+                   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+    <changeSet id="1" author="fineract">
+        <modifyDataType columnName="parameter_json"

Review Comment:
   Does this modification work if you already had string values in the table?



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/domain/JobExecutionRepository.java:
##########
@@ -180,4 +193,32 @@ bje.JOB_INSTANCE_ID NOT IN (
                 """, Map.of("statuses", List.of(STARTED.name(), STARTING.name()), "jobName", jobName, "completedStatus", COMPLETED.name(),
                 "parameterKeyName", parameterKeyName, "parameterValue", parameterValue), Long.class);
     }
+
+    public List<Long> getRunningJobsIdsByExecutionParameter(String jobName, String jobCustomParamKeyName, String parameterKeyName,
+            String parameterValue) {
+        final StringBuilder sqlStatementBuilder = new StringBuilder();
+        String jsonString = gson.toJson(new JobParameterDTO(parameterKeyName, parameterValue));
+        sqlStatementBuilder.append(
+                "SELECT bje.JOB_EXECUTION_ID FROM BATCH_JOB_INSTANCE bji INNER JOIN BATCH_JOB_EXECUTION bje ON bji.JOB_INSTANCE_ID = bje.JOB_INSTANCE_ID INNER JOIN BATCH_JOB_EXECUTION_PARAMS bjep ON bje.JOB_EXECUTION_ID = bjep.JOB_EXECUTION_ID"
+                        + " WHERE bje.STATUS IN (:statuses) AND bji.JOB_NAME = :jobName AND bjep.KEY_NAME = :jobCustomParamKeyName AND bjep.LONG_VAL IN ("
+                        + getSubQueryForCustomJobParameters()
+                        + ") AND bje.JOB_INSTANCE_ID NOT IN (SELECT bje.JOB_INSTANCE_ID FROM BATCH_JOB_INSTANCE bji INNER JOIN BATCH_JOB_EXECUTION bje ON bji.JOB_INSTANCE_ID = bje.JOB_INSTANCE_ID"
+                        + " WHERE bje.STATUS = :completedStatus AND bji.JOB_NAME = :jobName)");
+        return namedParameterJdbcTemplate.queryForList(
+                sqlStatementBuilder.toString(), Map.of("statuses", List.of(STARTED.name(), STARTING.name()), "jobName", jobName,
+                        "completedStatus", COMPLETED.name(), "jobCustomParamKeyName", jobCustomParamKeyName, "jsonString", jsonString),
+                Long.class);
+    }
+
+    private String getSubQueryForCustomJobParameters() {
+        if (databaseTypeResolver.isMySQL()) {
+            return "SELECT cjp.id FROM batch_custom_job_parameters cjp WHERE JSON_CONTAINS(cjp.parameter_json,:jsonString)";
+        } else if (databaseTypeResolver.isPostgreSQL()) {
+            return "SELECT cjp.id FROM (SELECT id,json_array_elements(parameter_json) AS json_data FROM batch_custom_job_parameters) AS cjp WHERE (cjp.json_data ::jsonb @> :jsonString ::jsonb)";
+        } else {
+            throw new IllegalStateException("Database type is not supported for current schema " + databaseTypeResolver.databaseType());

Review Comment:
   Error msg.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/domain/JobCustomParameterRepositoryImpl.java:
##########
@@ -0,0 +1,47 @@
+/**
+ * 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.infrastructure.jobs.domain;
+
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
+import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
+import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
+import org.springframework.jdbc.core.namedparam.SqlParameterSource;
+import org.springframework.stereotype.Component;
+
+@RequiredArgsConstructor
+@Component
+public class JobCustomParameterRepositoryImpl implements JobCustomParameterRepository {
+
+    private final NamedParameterJdbcTemplate namedParameterJdbcTemplate;
+    private final DatabaseSpecificSQLGenerator databaseSpecificSQLGenerator;
+
+    @Override
+    public Long saveCustomJobParameter(String jsonString) {
+        final StringBuilder insertSqlStatementBuilder = new StringBuilder(500);
+        insertSqlStatementBuilder.append("INSERT INTO batch_custom_job_parameters (parameter_json) VALUES (:jsonString"

Review Comment:
   Let's try to use formatting, makes it much easier to read.
   ```
   "INSERT INTO ... VALUES (%s)".formatted(generator.castJson(":jsonString"))
   ```



-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] ruchiD commented on a diff in pull request #3108: FINERACT-1724-Regular-loan-cob-job-having-catch-up-true

Posted by "ruchiD (via GitHub)" <gi...@apache.org>.
ruchiD commented on code in PR #3108:
URL: https://github.com/apache/fineract/pull/3108#discussion_r1164047953


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/database/DatabaseSpecificSQLGenerator.java:
##########
@@ -168,4 +168,14 @@ public String currentSchema() {
             throw new IllegalStateException("Database type is not supported for current schema " + databaseTypeResolver.databaseType());
         }
     }
+
+    public String jsonAppender() {

Review Comment:
   Fixed



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/database/DatabaseSpecificSQLGenerator.java:
##########
@@ -168,4 +168,14 @@ public String currentSchema() {
             throw new IllegalStateException("Database type is not supported for current schema " + databaseTypeResolver.databaseType());
         }
     }
+
+    public String jsonAppender() {
+        if (databaseTypeResolver.isMySQL()) {
+            return "";
+        } else if (databaseTypeResolver.isPostgreSQL()) {
+            return " ::json";
+        } else {
+            throw new IllegalStateException("Database type is not supported for current schema " + databaseTypeResolver.databaseType());

Review Comment:
   Fixed



-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [fineract] ruchiD commented on a diff in pull request #3108: FINERACT-1724-Regular-loan-cob-job-having-catch-up-true

Posted by "ruchiD (via GitHub)" <gi...@apache.org>.
ruchiD commented on code in PR #3108:
URL: https://github.com/apache/fineract/pull/3108#discussion_r1163116967


##########
fineract-provider/src/main/resources/db/changelog/tenant/parts/0103_modify_parameter_json_column_custom_job_parameters.xml:
##########
@@ -0,0 +1,30 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    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.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+                   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+    <changeSet id="1" author="fineract">
+        <modifyDataType columnName="parameter_json"

Review Comment:
   Yes



-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org