You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "solomax (via GitHub)" <gi...@apache.org> on 2023/03/20 09:24:52 UTC
[GitHub] [wicket] solomax opened a new pull request, #561: fileCountMax is added
solomax opened a new pull request, #561:
URL: https://github.com/apache/wicket/pull/561
POC, please review :)
I'll add some tests a bit later
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax closed pull request #561: [WICKET-7035] fileCountMax is added
Posted by "solomax (via GitHub)" <gi...@apache.org>.
solomax closed pull request #561: [WICKET-7035] fileCountMax is added
URL: https://github.com/apache/wicket/pull/561
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on a diff in pull request #561: [WICKET-7035] fileCountMax is added
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #561:
URL: https://github.com/apache/wicket/pull/561#discussion_r1145816412
##########
wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java:
##########
@@ -1866,6 +1904,29 @@ protected void onBeforeRender()
// clear multipart hint, it will be reevaluated by #isMultiPart()
this.multiPart &= MULTIPART_HARD;
+ if (!fileCountMax.isPresent())
+ {
+ AtomicLong accumulator = new AtomicLong(0);
+ visitChildren(MultiFileUploadField.class, new IVisitor<MultiFileUploadField, Void>()
Review Comment:
OK!
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on pull request #561: [WICKET-7035] fileCountMax is added
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #561:
URL: https://github.com/apache/wicket/pull/561#issuecomment-1482300385
+1
On Fri, Mar 24, 2023, 07:27 Maxim Solodovnik ***@***.***>
wrote:
> @martin-g <https://github.com/martin-g>, @bitstorm
> <https://github.com/bitstorm> I guess this one should be backported to
> 8.x as well?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/wicket/pull/561#issuecomment-1482271919>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AABYUQUUD2EI5CNUSNFPD3TW5UWFZANCNFSM6AAAAAAWA2AIVM>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on a diff in pull request #561: [WICKET-7035] fileCountMax is added
Posted by "solomax (via GitHub)" <gi...@apache.org>.
solomax commented on code in PR #561:
URL: https://github.com/apache/wicket/pull/561#discussion_r1145792295
##########
wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java:
##########
@@ -1866,6 +1904,29 @@ protected void onBeforeRender()
// clear multipart hint, it will be reevaluated by #isMultiPart()
this.multiPart &= MULTIPART_HARD;
+ if (!fileCountMax.isPresent())
+ {
+ AtomicLong accumulator = new AtomicLong(0);
+ visitChildren(MultiFileUploadField.class, new IVisitor<MultiFileUploadField, Void>()
Review Comment:
I've added the code, but some tests are broken :(
I guess the better option would be to revert latest changes ....
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on pull request #561: [WICKET-7035] fileCountMax is added
Posted by "solomax (via GitHub)" <gi...@apache.org>.
solomax commented on PR #561:
URL: https://github.com/apache/wicket/pull/561#issuecomment-1482271919
@martin-g, @bitstorm I guess this one should be backported to 8.x as well?
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on pull request #561: fileCountMax is added
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #561:
URL: https://github.com/apache/wicket/pull/561#issuecomment-1479267796
In that case maybe it would be better to not do anything at line 225 but add logic to `Form#onBeforeRender()` (?!) that does something like:
```
# pseudo code
if (maxFileCount == -1) {
AtomicLong accumulator = new AtomicLong(0);
visitAllMultipartFormComponents((mpfc) -> {accumulator.add(mpfc.getMax())});
setMaxFileCount(accumulator.get());
}
```
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on a diff in pull request #561: [WICKET-7035] fileCountMax is added
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #561:
URL: https://github.com/apache/wicket/pull/561#discussion_r1145750830
##########
wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java:
##########
@@ -1866,6 +1904,29 @@ protected void onBeforeRender()
// clear multipart hint, it will be reevaluated by #isMultiPart()
this.multiPart &= MULTIPART_HARD;
+ if (!fileCountMax.isPresent())
+ {
+ AtomicLong accumulator = new AtomicLong(0);
+ visitChildren(MultiFileUploadField.class, new IVisitor<MultiFileUploadField, Void>()
Review Comment:
What about `FileUploadField` ?
It is highly unlikely that an application will use both `MultiFileUploadField` and `FileUploadField` in the same Form but it is possible.
Also it is possible that the Form contains one or more `FileUploadField` (without `MultiFileUploadField`) and an `<input type="file" multiple="multiple">` can upload unknown number of files. I don't see a way to support 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.
To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on pull request #561: fileCountMax is added
Posted by "solomax (via GitHub)" <gi...@apache.org>.
solomax commented on PR #561:
URL: https://github.com/apache/wicket/pull/561#issuecomment-1478905501
@martin-g I would like to add `setFileCountMax()` to `MultiFileUploadField` (at line 225)
Shall I add logic for the case of multiple `MultiFileUploadField` in the form?
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on a diff in pull request #561: [WICKET-7035] fileCountMax is added
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #561:
URL: https://github.com/apache/wicket/pull/561#discussion_r1145771449
##########
wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java:
##########
@@ -1866,6 +1904,29 @@ protected void onBeforeRender()
// clear multipart hint, it will be reevaluated by #isMultiPart()
this.multiPart &= MULTIPART_HARD;
+ if (!fileCountMax.isPresent())
+ {
+ AtomicLong accumulator = new AtomicLong(0);
+ visitChildren(MultiFileUploadField.class, new IVisitor<MultiFileUploadField, Void>()
Review Comment:
Another option is to visit the FileUploadFields first and if it does not have `multiple` attribute then count it as `1`. It it has `multiple` then stop and do nothing more.
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on pull request #561: [WICKET-7035] fileCountMax is added
Posted by "solomax (via GitHub)" <gi...@apache.org>.
solomax commented on PR #561:
URL: https://github.com/apache/wicket/pull/561#issuecomment-1483785585
I'll merge this one into master after https://github.com/apache/wicket/pull/565
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on pull request #561: fileCountMax is added
Posted by "solomax (via GitHub)" <gi...@apache.org>.
solomax commented on PR #561:
URL: https://github.com/apache/wicket/pull/561#issuecomment-1478915316
Other than that everything seems to work :)
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on pull request #561: fileCountMax is added
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #561:
URL: https://github.com/apache/wicket/pull/561#issuecomment-1479159277
> I would like to add `setFileCountMax()` to `MultiFileUploadField` (at line 225)
You mean to call `form.setFileCountMax(this.max)` ?
Why ? I think this is responsibility of the developer to set the max per Form. Wicket should not try to calculate it. Or should it ?!
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on pull request #561: fileCountMax is added
Posted by "solomax (via GitHub)" <gi...@apache.org>.
solomax commented on PR #561:
URL: https://github.com/apache/wicket/pull/561#issuecomment-1479201460
> You mean to call form.setFileCountMax(this.max) ?
yes
> Why ? I think this is responsibility of the developer to set the max per Form. Wicket should not try to calculate it. Or should it ?!
in this particular case `max` is set for `MultiFileUploadField` but *not* for the form
so now developer should pass max for both form and field, which is Ok in case there are multiple file inputs, but seems to be sort-of-code-duplication in case there is only one `MultiFileUploadField` for the form
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on a diff in pull request #561: [WICKET-7035] fileCountMax is added
Posted by "solomax (via GitHub)" <gi...@apache.org>.
solomax commented on code in PR #561:
URL: https://github.com/apache/wicket/pull/561#discussion_r1145758595
##########
wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java:
##########
@@ -1866,6 +1904,29 @@ protected void onBeforeRender()
// clear multipart hint, it will be reevaluated by #isMultiPart()
this.multiPart &= MULTIPART_HARD;
+ if (!fileCountMax.isPresent())
+ {
+ AtomicLong accumulator = new AtomicLong(0);
+ visitChildren(MultiFileUploadField.class, new IVisitor<MultiFileUploadField, Void>()
Review Comment:
This is fair
Additional case would be inline Forms with different file inputs ...
Shall I revert my last commits, so the user will take care of this? :)
--
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@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org