You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "oleksii-novikov-onix (via GitHub)" <gi...@apache.org> on 2025/02/21 06:42:34 UTC
[PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
oleksii-novikov-onix opened a new pull request, #4364:
URL: https://github.com/apache/fineract/pull/4364
## Description
1. Add a customData field to the loan and loan transaction events.
2. Create a new interface, CustomDataSerializer, to be implemented as part of Fineract's custom module handling.
## 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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "oleksii-novikov-onix (via GitHub)" <gi...@apache.org>.
oleksii-novikov-onix commented on code in PR #4364:
URL: https://github.com/apache/fineract/pull/4364#discussion_r1968221053
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/serializer/CustomDataSerializer.java:
##########
@@ -0,0 +1,30 @@
+/**
+ * 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.event.external.service.serialization.serializer;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+
+public interface CustomDataSerializer {
Review Comment:
@galovics What would be the most suitable name? ExternalEventCustomDataSerializer, maybe?
--
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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "oleksii-novikov-onix (via GitHub)" <gi...@apache.org>.
oleksii-novikov-onix commented on code in PR #4364:
URL: https://github.com/apache/fineract/pull/4364#discussion_r1969540534
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/serializer/loan/LoanBusinessEventSerializer.java:
##########
@@ -56,6 +59,7 @@ public class LoanBusinessEventSerializer implements BusinessEventSerializer {
private final LoanSummaryBalancesRepository loanSummaryBalancesRepository;
@Lazy
private final LoanSummaryProviderDelegate loanSummaryProviderDelegate;
+ private final CustomDataSerializer customDataSerializer;
Review Comment:
Fixed. Now we are injecting a list of serializers: `List<ExternalEventCustomDataSerializer<LoanBusinessEvent>>`
--
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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #4364:
URL: https://github.com/apache/fineract/pull/4364
--
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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "mariiaKraievska (via GitHub)" <gi...@apache.org>.
mariiaKraievska commented on PR #4364:
URL: https://github.com/apache/fineract/pull/4364#issuecomment-2677780685
LGTM
--
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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #4364:
URL: https://github.com/apache/fineract/pull/4364#discussion_r1967949188
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/serializer/CustomDataSerializer.java:
##########
@@ -0,0 +1,30 @@
+/**
+ * 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.event.external.service.serialization.serializer;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+
+public interface CustomDataSerializer {
Review Comment:
Also, I'd rename this to correspond more to the External Event framework.
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/serializer/CustomDataSerializer.java:
##########
@@ -0,0 +1,30 @@
+/**
+ * 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.event.external.service.serialization.serializer;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+
+public interface CustomDataSerializer {
+
+ Map<String, ByteBuffer> serializeLoanCustomData(Long loanId);
Review Comment:
Is it necessary to have these methods here specifically for Loans? The classname suggests a much more generic way of handling things.
Based on the Avro schema, a serializer like this should have two methods with this signature:
```
ByteBuffer serialize(BusinessEvent ...);
String key();
```
Where the first one serializes some data into a byte buffer and the second one just returns the key used for placing the byte buffer into the map.
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/serializer/DefaultCustomDataSerializer.java:
##########
@@ -0,0 +1,36 @@
+/**
+ * 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.event.external.service.serialization.serializer;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+
+public class DefaultCustomDataSerializer implements CustomDataSerializer {
Review Comment:
I really don't like this approach. This implementation doesn't do anything so why is it here even?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/serializer/loan/LoanBusinessEventSerializer.java:
##########
@@ -56,6 +59,7 @@ public class LoanBusinessEventSerializer implements BusinessEventSerializer {
private final LoanSummaryBalancesRepository loanSummaryBalancesRepository;
@Lazy
private final LoanSummaryProviderDelegate loanSummaryProviderDelegate;
+ private final CustomDataSerializer customDataSerializer;
Review Comment:
No need for the "no implementations" case. Just refer to them as a List<CustomDataSerializer> and if there are no implementations, the list will just be empty.
--
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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #4364:
URL: https://github.com/apache/fineract/pull/4364#issuecomment-2681603719
@oleksii-novikov-onix Please squash your commits
--
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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #4364:
URL: https://github.com/apache/fineract/pull/4364#discussion_r1968428958
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/serializer/CustomDataSerializer.java:
##########
@@ -0,0 +1,30 @@
+/**
+ * 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.event.external.service.serialization.serializer;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+
+public interface CustomDataSerializer {
Review Comment:
yep, sounds good @oleksii-novikov-onix
--
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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4364:
URL: https://github.com/apache/fineract/pull/4364#discussion_r1967623192
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/serializer/loan/LoanBusinessEventSerializer.java:
##########
@@ -56,6 +59,7 @@ public class LoanBusinessEventSerializer implements BusinessEventSerializer {
private final LoanSummaryBalancesRepository loanSummaryBalancesRepository;
@Lazy
private final LoanSummaryProviderDelegate loanSummaryProviderDelegate;
+ private final CustomDataSerializer customDataSerializer;
Review Comment:
I would have it as a list, and if there is any implementation we are calling each serializer one by one and adding their piece of code into the `customData` field. What do you think?
--
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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "oleksii-novikov-onix (via GitHub)" <gi...@apache.org>.
oleksii-novikov-onix commented on code in PR #4364:
URL: https://github.com/apache/fineract/pull/4364#discussion_r1967821213
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/serializer/loan/LoanBusinessEventSerializer.java:
##########
@@ -56,6 +59,7 @@ public class LoanBusinessEventSerializer implements BusinessEventSerializer {
private final LoanSummaryBalancesRepository loanSummaryBalancesRepository;
@Lazy
private final LoanSummaryProviderDelegate loanSummaryProviderDelegate;
+ private final CustomDataSerializer customDataSerializer;
Review Comment:
Great idea! This will allow me to remove DefaultCustomDataSerializer because if there are no implementations, the list will be empty
--
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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "oleksii-novikov-onix (via GitHub)" <gi...@apache.org>.
oleksii-novikov-onix commented on code in PR #4364:
URL: https://github.com/apache/fineract/pull/4364#discussion_r1969543895
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/serializer/CustomDataSerializer.java:
##########
@@ -0,0 +1,30 @@
+/**
+ * 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.event.external.service.serialization.serializer;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+
+public interface CustomDataSerializer {
+
+ Map<String, ByteBuffer> serializeLoanCustomData(Long loanId);
Review Comment:
@galovics Thank you, that's a very valid point! Now we have a generalized interface: `ExternalEventCustomDataSerializer<T extends BusinessEvent<?>>` with these two methods.
--
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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "oleksii-novikov-onix (via GitHub)" <gi...@apache.org>.
oleksii-novikov-onix commented on code in PR #4364:
URL: https://github.com/apache/fineract/pull/4364#discussion_r1969548696
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/serializer/DefaultCustomDataSerializer.java:
##########
@@ -0,0 +1,36 @@
+/**
+ * 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.event.external.service.serialization.serializer;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+
+public class DefaultCustomDataSerializer implements CustomDataSerializer {
Review Comment:
Removed `DefaultCustomDataSerializer`
--
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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "oleksii-novikov-onix (via GitHub)" <gi...@apache.org>.
oleksii-novikov-onix commented on code in PR #4364:
URL: https://github.com/apache/fineract/pull/4364#discussion_r1969544414
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/event/external/service/serialization/serializer/CustomDataSerializer.java:
##########
@@ -0,0 +1,30 @@
+/**
+ * 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.event.external.service.serialization.serializer;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+
+public interface CustomDataSerializer {
Review Comment:
Renamed
--
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
Re: [PR] FINERACT-2181: Introduce customData field for Loan and LoanTransaction business events [fineract]
Posted by "oleksii-novikov-onix (via GitHub)" <gi...@apache.org>.
oleksii-novikov-onix commented on PR #4364:
URL: https://github.com/apache/fineract/pull/4364#issuecomment-2681659120
@adamsaghy Done, I have squashed the commits.
--
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