matrei commented on PR #16040:
URL: https://github.com/apache/grails-core/pull/16040#issuecomment-5744374772

   @codeconsole Thanks for the review. One clarification first: 
`GrailsControllersEnvironmentPostProcessor` does not maintain the legacy 
configuration. `grails.controllers.upload.*` is fully removed in this PR (the 
`Settings` constants, the configuration metadata and the Grails 
`MultipartConfigElement` bean are all gone). The post processor only *rejects* 
those properties at startup with migration instructions; nothing is translated 
or honored.
   
   I would prefer not to drop it outright, for two reasons:
   
   - Without it the removed properties are silently ignored, and an application 
that had set, say, `maxFileSize: 128000` quietly moves to Boot's 1 MB / 10 MB 
limits. The upload limits are a security-relevant knob in `THREAT_MODEL.md`, so 
a silent loosening on upgrade is the worst way for this to fail.
   - The guard was added at @jdaugherty's request in the earlier review round, 
as an `EnvironmentPostProcessor` specifically so the message surfaces before 
context creation.
   
   So I took your second option: the class is now `@Deprecated(forRemoval = 
true, since = "8.0")`, its Javadoc states it is a temporary migration aid that 
goes away in 9.0 without replacement, and the upgrade guide (ยง4.2) says the 
same.
   
   On the bean DSL conversion: the post processor is registered on its own in 
`META-INF/spring.factories` and `ControllersAutoConfiguration` does not 
reference it, so it should not stand in the way of converting the 
auto-configuration. If I am missing a coupling there, point me at it and I will 
sort 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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to