Skip to content

[WICKET-7035] fileCountMax is added#561

Closed
solomax wants to merge 11 commits into
wicket-9.xfrom
fileCountMax-9.x
Closed

[WICKET-7035] fileCountMax is added#561
solomax wants to merge 11 commits into
wicket-9.xfrom
fileCountMax-9.x

Conversation

@solomax

@solomax solomax commented Mar 20, 2023

Copy link
Copy Markdown
Contributor

POC, please review :)

I'll add some tests a bit later

@martin-g martin-g left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be also fine to use long (primitive) as https://github.com/apache/commons-fileupload/blob/c4f32a13298f6b9561c136b61b23c9e2c6d82368/src/main/java/org/apache/commons/fileupload/FileUploadBase.java#L172
I doubt the default value of -1 will ever change.

@solomax

solomax commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

@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?

@solomax

solomax commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

Other than that everything seems to work :)

@martin-g

Copy link
Copy Markdown
Member

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 ?!

@solomax

solomax commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

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

@martin-g

Copy link
Copy Markdown
Member

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());
}

@solomax solomax changed the title fileCountMax is added [WICKET-7035] fileCountMax is added Mar 23, 2023
Comment thread wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java Outdated
@solomax

solomax commented Mar 24, 2023

Copy link
Copy Markdown
Contributor Author

@martin-g, @bitstorm I guess this one should be backported to 8.x as well?

@martin-g

martin-g commented Mar 24, 2023 via email

Copy link
Copy Markdown
Member

@solomax solomax closed this Mar 25, 2023
@solomax solomax deleted the fileCountMax-9.x branch March 25, 2023 03:34
@solomax

solomax commented Mar 25, 2023

Copy link
Copy Markdown
Contributor Author

I'll merge this one into master after #565

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants