You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/05/31 17:13:39 UTC
[GitHub] [fineract] vorburger opened a new pull request #948: replace iText with openPDF [FINERACT-965]
vorburger opened a new pull request #948:
URL: https://github.com/apache/fineract/pull/948
FINERACT-965
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror edited a comment on pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
xurror edited a comment on pull request #948:
URL: https://github.com/apache/fineract/pull/948#issuecomment-636499948
Build failed because of this:
`org.apache.fineract.integrationtests.FixedDepositTest > testFixedDepositAccountClosureTypeWithdrawal_WITH_HOLD_TAX SKIPPED
org.apache.fineract.integrationtests.ClientLoanIntegrationTest > testLoanRefundByCashCashBasedAccounting FAILED
java.lang.AssertionError: 1 expectation failed.
Expected status code <200> but was <403>.
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at `
I've faced a couple of those locally too. Thought it was maybe related to some setting on my PC, but this could really be a problem.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #948:
URL: https://github.com/apache/fineract/pull/948#issuecomment-636496261
@fynmanoj @xurror better now? :smile_cat: Go Ahead and Merge if OK. Thanks again for spotting that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror commented on pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
xurror commented on pull request #948:
URL: https://github.com/apache/fineract/pull/948#issuecomment-636499948
Build failed because of this:
```org.apache.fineract.integrationtests.FixedDepositTest > testFixedDepositAccountClosureTypeWithdrawal_WITH_HOLD_TAX SKIPPED
org.apache.fineract.integrationtests.ClientLoanIntegrationTest > testLoanRefundByCashCashBasedAccounting FAILED
java.lang.AssertionError: 1 expectation failed.
Expected status code <200> but was <403>.
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at ```
I've faced a couple of those locally too. Thought it was maybe related to some setting on my PC, but this could really be a problem.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #948:
URL: https://github.com/apache/fineract/pull/948#issuecomment-636811574
Oh, it's because of `java.util.Base64`'s _default_ VS _URL_ VS _MIME_ variants... I'll fix it. -- BTW: The (failing) `StaffImageApiTest` **really** should cover `ImagesApiResource.retrieveImage()` method as well; I'll try to add that here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #948:
URL: https://github.com/apache/fineract/pull/948#issuecomment-636778244
This is now failing an integration test because of the changes I made here (not flaky), still something with that Base64 change... I haven't been able to entirely understand it - yet. If someone reading along "sees" the problem, please comment, else I'll dig further to figure it out...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror merged pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
xurror merged pull request #948:
URL: https://github.com/apache/fineract/pull/948
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #948:
URL: https://github.com/apache/fineract/pull/948#issuecomment-636706496
/rebase
The flaky test is "just" due to what @ptuomola documented in FINERACT-885 (nice find!)
@andreasrosdal great to hear from you! Thank You very very much for maintaining OpenPDF.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on a change in pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #948:
URL: https://github.com/apache/fineract/pull/948#discussion_r432963841
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/api/ImagesApiResource.java
##########
@@ -140,7 +140,8 @@ public Response retrieveImage(@PathParam("entity") final String entityName, @Pat
imageDataURISuffix = ContentRepositoryUtils.ImageDataURIsuffix.PNG.getValue();
}
- final String clientImageAsBase64Text = imageDataURISuffix + Base64.encodeBytes(imageData.getContentOfSize(maxWidth, maxHeight));
+ byte[] resizedImage = imageData.getContentOfSize(maxWidth, maxHeight);
+ final String clientImageAsBase64Text = imageDataURISuffix + Base64.getEncoder().encode(resizedImage);
Review comment:
@fynmanoj no you're absolutely right of course, it was (very) wrong - Thank You very much for spotting that!
PS: It's totally OK to help code review and tell me "this is wrong!" more strongly than your very polite "Are you sure about" :smile:
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] fynmanoj commented on a change in pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
fynmanoj commented on a change in pull request #948:
URL: https://github.com/apache/fineract/pull/948#discussion_r432147005
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/api/ImagesApiResource.java
##########
@@ -140,7 +140,8 @@ public Response retrieveImage(@PathParam("entity") final String entityName, @Pat
imageDataURISuffix = ContentRepositoryUtils.ImageDataURIsuffix.PNG.getValue();
}
- final String clientImageAsBase64Text = imageDataURISuffix + Base64.encodeBytes(imageData.getContentOfSize(maxWidth, maxHeight));
+ byte[] resizedImage = imageData.getContentOfSize(maxWidth, maxHeight);
+ final String clientImageAsBase64Text = imageDataURISuffix + Base64.getEncoder().encode(resizedImage);
Review comment:
Base64.getEncoder().encode() returns byte[]
Are you sure about this string concatenation?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] fynmanoj removed a comment on pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
fynmanoj removed a comment on pull request #948:
URL: https://github.com/apache/fineract/pull/948#issuecomment-636497502
:)
On Sun, 31 May, 2020, 22:13 Michael Vorburger โ๏ธ, <no...@github.com>
wrote:
> *@vorburger* commented on this pull request.
> ------------------------------
>
> In
> fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/api/ImagesApiResource.java
> <https://github.com/apache/fineract/pull/948#discussion_r432963841>:
>
> > @@ -140,7 +140,8 @@ public Response retrieveImage(@PathParam("entity") final String entityName, @Pat
>
> imageDataURISuffix = ContentRepositoryUtils.ImageDataURIsuffix.PNG.getValue();
>
> }
>
>
>
> - final String clientImageAsBase64Text = imageDataURISuffix + Base64.encodeBytes(imageData.getContentOfSize(maxWidth, maxHeight));
>
> + byte[] resizedImage = imageData.getContentOfSize(maxWidth, maxHeight);
>
> + final String clientImageAsBase64Text = imageDataURISuffix + Base64.getEncoder().encode(resizedImage);
>
>
> @fynmanoj <https://github.com/fynmanoj> no you're absolutely right of
> course, it was (very) wrong - Thank You very much for spotting that!
>
> PS: It's totally OK to help code review and tell me "this is wrong!" more
> strongly than your very polite "Are you sure about" ๐
>
> โ
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/fineract/pull/948#discussion_r432963841>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ANQLL2SUUHFFDYOBGXKQLI3RUKCL7ANCNFSM4NNNXV4Q>
> .
>
--
Disclaimer:
Privileged & confidential information is contained in this
message (including all attachments). If you are not an intended recipient
of this message, please destroy this message immediately and kindly notify
the sender by reply e-mail. Any unauthorised use or dissemination of this
message in any manner whatsoever, in whole or in part, is strictly
prohibited. This e-mail, including all attachments hereto, (i) is for
discussion purposes only and shall not be deemed or construed to be a
professional opinion unless expressly stated otherwise, and (ii) is not
intended, written or sent to be used, and cannot and shall not be used, for
any unlawful purpose. This communication, including any attachments, may
not be free of viruses, interceptions or interference, and may not be
compatible with your systems. You should carry out your own virus checks
before opening any attachment to this e-mail. The sender of this e-mail andย
*Fynarfin Tech Private Limited* shall not be liable for any damage that
you may sustain as a result of viruses, incompleteness of this message, a
delay in receipt of this message or computer problems experienced.ย
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #948:
URL: https://github.com/apache/fineract/pull/948#issuecomment-635609888
@xurror @awasum would you like to review and merge this?
based on and extending @nnatarajan #940
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] andreasrosdal commented on pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
andreasrosdal commented on pull request #948:
URL: https://github.com/apache/fineract/pull/948#issuecomment-636698738
It's awesome that you want to use OpenPDF. Let me know if you need any changes in OpenPDF. I am one of the OpenPDF maintainers.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #948:
URL: https://github.com/apache/fineract/pull/948#issuecomment-637472253
@xurror do you want to re-review this, and if OK, click Rebase and Merge?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror commented on a change in pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
xurror commented on a change in pull request #948:
URL: https://github.com/apache/fineract/pull/948#discussion_r432261788
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/api/ImagesApiResource.java
##########
@@ -140,7 +140,8 @@ public Response retrieveImage(@PathParam("entity") final String entityName, @Pat
imageDataURISuffix = ContentRepositoryUtils.ImageDataURIsuffix.PNG.getValue();
}
- final String clientImageAsBase64Text = imageDataURISuffix + Base64.encodeBytes(imageData.getContentOfSize(maxWidth, maxHeight));
+ byte[] resizedImage = imageData.getContentOfSize(maxWidth, maxHeight);
+ final String clientImageAsBase64Text = imageDataURISuffix + Base64.getEncoder().encode(resizedImage);
Review comment:
```suggestion
final String clientImageAsBase64Text = imageDataURISuffix + Base64.getEncoder().encodeToString(resizedImage);
```
I suggest you use this instead. Check this for reference: https://www.baeldung.com/java-base64-encode-and-decode.
Should be able to merge this once that is done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror closed pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
xurror closed pull request #948:
URL: https://github.com/apache/fineract/pull/948
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on a change in pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #948:
URL: https://github.com/apache/fineract/pull/948#discussion_r432963716
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/api/ImagesApiResource.java
##########
@@ -140,7 +140,8 @@ public Response retrieveImage(@PathParam("entity") final String entityName, @Pat
imageDataURISuffix = ContentRepositoryUtils.ImageDataURIsuffix.PNG.getValue();
}
- final String clientImageAsBase64Text = imageDataURISuffix + Base64.encodeBytes(imageData.getContentOfSize(maxWidth, maxHeight));
+ byte[] resizedImage = imageData.getContentOfSize(maxWidth, maxHeight);
+ final String clientImageAsBase64Text = imageDataURISuffix + Base64.getEncoder().encode(resizedImage);
Review comment:
done! Thanks you, very much.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] fynmanoj commented on pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
fynmanoj commented on pull request #948:
URL: https://github.com/apache/fineract/pull/948#issuecomment-636497502
:)
On Sun, 31 May, 2020, 22:13 Michael Vorburger โ๏ธ, <no...@github.com>
wrote:
> *@vorburger* commented on this pull request.
> ------------------------------
>
> In
> fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/api/ImagesApiResource.java
> <https://github.com/apache/fineract/pull/948#discussion_r432963841>:
>
> > @@ -140,7 +140,8 @@ public Response retrieveImage(@PathParam("entity") final String entityName, @Pat
>
> imageDataURISuffix = ContentRepositoryUtils.ImageDataURIsuffix.PNG.getValue();
>
> }
>
>
>
> - final String clientImageAsBase64Text = imageDataURISuffix + Base64.encodeBytes(imageData.getContentOfSize(maxWidth, maxHeight));
>
> + byte[] resizedImage = imageData.getContentOfSize(maxWidth, maxHeight);
>
> + final String clientImageAsBase64Text = imageDataURISuffix + Base64.getEncoder().encode(resizedImage);
>
>
> @fynmanoj <https://github.com/fynmanoj> no you're absolutely right of
> course, it was (very) wrong - Thank You very much for spotting that!
>
> PS: It's totally OK to help code review and tell me "this is wrong!" more
> strongly than your very polite "Are you sure about" ๐
>
> โ
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/fineract/pull/948#discussion_r432963841>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ANQLL2SUUHFFDYOBGXKQLI3RUKCL7ANCNFSM4NNNXV4Q>
> .
>
--
Disclaimer:
Privileged & confidential information is contained in this
message (including all attachments). If you are not an intended recipient
of this message, please destroy this message immediately and kindly notify
the sender by reply e-mail. Any unauthorised use or dissemination of this
message in any manner whatsoever, in whole or in part, is strictly
prohibited. This e-mail, including all attachments hereto, (i) is for
discussion purposes only and shall not be deemed or construed to be a
professional opinion unless expressly stated otherwise, and (ii) is not
intended, written or sent to be used, and cannot and shall not be used, for
any unlawful purpose. This communication, including any attachments, may
not be free of viruses, interceptions or interference, and may not be
compatible with your systems. You should carry out your own virus checks
before opening any attachment to this e-mail. The sender of this e-mail andย
*Fynarfin Tech Private Limited* shall not be liable for any damage that
you may sustain as a result of viruses, incompleteness of this message, a
delay in receipt of this message or computer problems experienced.ย
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] fynmanoj commented on a change in pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
fynmanoj commented on a change in pull request #948:
URL: https://github.com/apache/fineract/pull/948#discussion_r433055427
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/api/ImagesApiResource.java
##########
@@ -140,7 +140,8 @@ public Response retrieveImage(@PathParam("entity") final String entityName, @Pat
imageDataURISuffix = ContentRepositoryUtils.ImageDataURIsuffix.PNG.getValue();
}
- final String clientImageAsBase64Text = imageDataURISuffix + Base64.encodeBytes(imageData.getContentOfSize(maxWidth, maxHeight));
+ byte[] resizedImage = imageData.getContentOfSize(maxWidth, maxHeight);
+ final String clientImageAsBase64Text = imageDataURISuffix + Base64.getEncoder().encode(resizedImage);
Review comment:
@vorburger , Sure ๐ ๐
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] xurror edited a comment on pull request #948: replace iText with openPDF [FINERACT-965]
Posted by GitBox <gi...@apache.org>.
xurror edited a comment on pull request #948:
URL: https://github.com/apache/fineract/pull/948#issuecomment-636499948
Build failed because of this:
```org.apache.fineract.integrationtests.FixedDepositTest > testFixedDepositAccountClosureTypeWithdrawal_WITH_HOLD_TAX SKIPPED
org.apache.fineract.integrationtests.ClientLoanIntegrationTest > testLoanRefundByCashCashBasedAccounting FAILED
java.lang.AssertionError: 1 expectation failed.
Expected status code <200> but was <403>.
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at ```
I've faced a couple of those locally too. Thought it was maybe related to some setting on my PC, but this could really be a problem.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org