Re: Experiences with upgrading our apps from 5.8.2 to 5.8.6

2024-05-03 Thread Thiago H. de Paula Figueiredo 
On Fri, May 3, 2024 at 3:16 AM Chris Poulsen  wrote:

> > It would help a lot to have an example of class hierarchy not being
> handled
> well. I'll definitely use it.
>
> OK, I'll try to find some time to see if I can create a minimal example
>

Much appreciated!


> > Regarding service classes, field injection is indeed supported
>


> I think both cases that caused these issues was related to (field)
> injecting an Asset (an icon or similar) and the error was something about
> RequestGlobals being null, IIRC
>

This makes sense. RequestGlobals only has its request and response
properties set in web threads (i.e. requests), so, when the service is
instantiated in a non-web-thread, you get an NPE for trying to use a null
request or response object. Another reason for using constructor injection.
:)

-- 
Thiago H. de Paula Figueiredo

Re: Experiences with upgrading our apps from 5.8.2 to 5.8.6

2024-05-02 Thread Chris Poulsen 
> It would help a lot to have an example of class hierarchy not being
handled
well. I'll definitely use it.

OK, I'll try to find some time to see if I can create a minimal example

> Regarding service classes, field injection is indeed supported

I think both cases that caused these issues was related to (field)
injecting an Asset (an icon or similar) and the error was something about
RequestGlobals being null, IIRC

-- 
Chris

On Tue, Apr 30, 2024 at 9:32 PM Thiago H. de Paula Figueiredo <
thiag...@gmail.com> wrote:

> Hello, Chris!
>
> Thanks for the information. We are aware that there are still some problems
> with 5.8.3 to 5.8.6, especially on projects with components having
> complex dependencies among them, and we're working on it. 5.8.3 introduces
> a lot of changes on how the Tapestry classloading and class transformation
> work and there's an awful lot of corner cases.
>
> It would help a lot to have an example of class hierarchy not being handled
> well. I'll definitely use it.
>
> Regarding service classes, field injection is indeed supported, but I
> consider that constructor-based injection is better. It keeps your service
> classes working even when not using Tapestry-IoC, making automated testing
> way easier, and, in most cases, you can even have your service classes
> completely independent of Tapestry itself.
>
> Cheers!
>
>
> On Tue, Apr 30, 2024 at 5:12 AM Chris Poulsen 
> wrote:
>
> > The upgrades went pretty smoothly in general, good job team!
> >
> > I did encounter a couple of issues that I think it is worth raising the
> > flag on.
> >
> > We run our products in different environments:
> >
> >- On plain Windows servers
> >- In Docker containers (Linux).
> >
> > Things are currently on Java 17, the development docker images are
> running
> > without issues on the Java 21 runtime.
> >
> > Issues encountered:
> >
> >1. TAP5-2605 JS minimizer regression bug in 5.5.0-beta-1
> > I did a simple fix,
> >mentioned in the comments, this would be nice to get in, so I don't
> > have to
> >decorate the Javascript resource minimizer, just to change the
> >"StreamableResource.toString()" to a dummy value
> >2. Field injection in 2 of our Tapestry services went from an obscure
> >warning on startup to an actual error in production mode (the two
> > services
> >had an "@Inject Asset icon" type of thing, which blew up in production
> >mode=true. It worked in development mode and in both modes in v5.8.2.
> > The
> >fix/workaround was to move to CTOR injection - (that feels more
> correct
> > to
> >me anyway). I don't know if this was ever supposed to work, but it did
> > and
> >it would be nice with some check/nice error in development mode, if
> > this is
> >not supposed to work
> >3. We had a single construct which ended up resulting in
> >java.lang.VerifyError: Bad type on operand stack (basically bad
> bytecode
> >was being produced as I understand it. I'll put a few more details on
> > this
> >below)
> >
> > The java.lang.VerifyError:
> >
> > Log:
> >
> > 2024-04-25T15:17:52.707+02:00 INFO
> >  [org.apache.tapestry5.services.pageload.PageClassLoaderContextManager]
> > (default task-79) Dependency information gathered in 6,126 ms
> > 2024-04-25T15:17:52.707+02:00 INFO
> >  [org.apache.tapestry5.services.pageload.PageClassLoaderContextManager]
> > (default task-79) Starting preloading page classloader contexts
> > 2024-04-25T15:17:52.864+02:00 ERROR [io.undertow.request] (default
> task-79)
> > UT005023: Exception handling request to /administration/:
> > java.lang.RuntimeException: java.lang.VerifyError: Bad type on operand
> > stack
> > Exception Details:
> >   Location:
> >
> >
> >
> com/dezide/author/gui/base/content/FaqIncludeAndConversationBase.onSelectFaqArticle()Ljava/lang/Object;
> > @5: invokevirtual
> >   Reason:
> > Type 'com/dezide/author/gui/pages/faq/SelectFaqArticle' (current
> frame,
> > stack[1]) is not assignable to
> > 'com/dezide/author/gui/base/conversation/ParentConversationPageBase'
> >   Current Frame:
> > bci: @5
> > flags: { }
> > locals: {
> > 'com/dezide/author/gui/base/content/FaqIncludeAndConversationBase' }
> > stack: {
> > 'com/dezide/author/gui/base/content/FaqIncludeAndConversationBase',
> > 'com/dezide/author/gui/pages/faq/SelectFaqArticle' }
> >   Bytecode:
> > 000: 2a2a b600 3fb6 0043 2ab6 003f 2ab6 0049
> > 010: 2ab4 004b 124d 03bd 0004 b900 5303 004c
> > 020: 2ab4 004b 1255 03bd 0004 b900 5303 004d
> > 030: 2ab6 003f 2bb6 0059 2ab6 003f 2cb6 005c
> > 040: 2ab6 003f b0
> >
> > at
> >
> >
> deployment.dezide-author-1.29.1-rc.1.ear.web.war//org.apache.tapestry5.ioc.internal.StartupDefImpl$1.run(StartupDefImpl.java:84)
> > at
> >
> >
> deployment.dezide-author-1.29.1-rc.1.ear.web.war//org.apache.tapestry5.ioc.internal.OperationTrackerImpl.run(OperationTrackerImpl.java:56)
> > at

Re: Experiences with upgrading our apps from 5.8.2 to 5.8.6

2024-04-30 Thread Thiago H. de Paula Figueiredo 
Hello, Chris!

Thanks for the information. We are aware that there are still some problems
with 5.8.3 to 5.8.6, especially on projects with components having
complex dependencies among them, and we're working on it. 5.8.3 introduces
a lot of changes on how the Tapestry classloading and class transformation
work and there's an awful lot of corner cases.

It would help a lot to have an example of class hierarchy not being handled
well. I'll definitely use it.

Regarding service classes, field injection is indeed supported, but I
consider that constructor-based injection is better. It keeps your service
classes working even when not using Tapestry-IoC, making automated testing
way easier, and, in most cases, you can even have your service classes
completely independent of Tapestry itself.

Cheers!


On Tue, Apr 30, 2024 at 5:12 AM Chris Poulsen 
wrote:

> The upgrades went pretty smoothly in general, good job team!
>
> I did encounter a couple of issues that I think it is worth raising the
> flag on.
>
> We run our products in different environments:
>
>- On plain Windows servers
>- In Docker containers (Linux).
>
> Things are currently on Java 17, the development docker images are running
> without issues on the Java 21 runtime.
>
> Issues encountered:
>
>1. TAP5-2605 JS minimizer regression bug in 5.5.0-beta-1
> I did a simple fix,
>mentioned in the comments, this would be nice to get in, so I don't
> have to
>decorate the Javascript resource minimizer, just to change the
>"StreamableResource.toString()" to a dummy value
>2. Field injection in 2 of our Tapestry services went from an obscure
>warning on startup to an actual error in production mode (the two
> services
>had an "@Inject Asset icon" type of thing, which blew up in production
>mode=true. It worked in development mode and in both modes in v5.8.2.
> The
>fix/workaround was to move to CTOR injection - (that feels more correct
> to
>me anyway). I don't know if this was ever supposed to work, but it did
> and
>it would be nice with some check/nice error in development mode, if
> this is
>not supposed to work
>3. We had a single construct which ended up resulting in
>java.lang.VerifyError: Bad type on operand stack (basically bad bytecode
>was being produced as I understand it. I'll put a few more details on
> this
>below)
>
> The java.lang.VerifyError:
>
> Log:
>
> 2024-04-25T15:17:52.707+02:00 INFO
>  [org.apache.tapestry5.services.pageload.PageClassLoaderContextManager]
> (default task-79) Dependency information gathered in 6,126 ms
> 2024-04-25T15:17:52.707+02:00 INFO
>  [org.apache.tapestry5.services.pageload.PageClassLoaderContextManager]
> (default task-79) Starting preloading page classloader contexts
> 2024-04-25T15:17:52.864+02:00 ERROR [io.undertow.request] (default task-79)
> UT005023: Exception handling request to /administration/:
> java.lang.RuntimeException: java.lang.VerifyError: Bad type on operand
> stack
> Exception Details:
>   Location:
>
>
> com/dezide/author/gui/base/content/FaqIncludeAndConversationBase.onSelectFaqArticle()Ljava/lang/Object;
> @5: invokevirtual
>   Reason:
> Type 'com/dezide/author/gui/pages/faq/SelectFaqArticle' (current frame,
> stack[1]) is not assignable to
> 'com/dezide/author/gui/base/conversation/ParentConversationPageBase'
>   Current Frame:
> bci: @5
> flags: { }
> locals: {
> 'com/dezide/author/gui/base/content/FaqIncludeAndConversationBase' }
> stack: {
> 'com/dezide/author/gui/base/content/FaqIncludeAndConversationBase',
> 'com/dezide/author/gui/pages/faq/SelectFaqArticle' }
>   Bytecode:
> 000: 2a2a b600 3fb6 0043 2ab6 003f 2ab6 0049
> 010: 2ab4 004b 124d 03bd 0004 b900 5303 004c
> 020: 2ab4 004b 1255 03bd 0004 b900 5303 004d
> 030: 2ab6 003f 2bb6 0059 2ab6 003f 2cb6 005c
> 040: 2ab6 003f b0
>
> at
>
> deployment.dezide-author-1.29.1-rc.1.ear.web.war//org.apache.tapestry5.ioc.internal.StartupDefImpl$1.run(StartupDefImpl.java:84)
> at
>
> deployment.dezide-author-1.29.1-rc.1.ear.web.war//org.apache.tapestry5.ioc.internal.OperationTrackerImpl.run(OperationTrackerImpl.java:56)
> at
>
> deployment.dezide-author-1.29.1-rc.1.ear.web.war//org.apache.tapestry5.ioc.internal.PerThreadOperationTracker.run(PerThreadOperationTracker.java:60)
> at
>
> deployment.dezide-author-1.29.1-rc.1.ear.web.war//org.apache.tapestry5.ioc.internal.RegistryImpl.run(RegistryImpl.java:1286)
>
> It says:
>
> Reason:
> Type 'com/dezide/author/gui/pages/faq/SelectFaqArticle' (current frame,
> stack[1]) is not assignable to
> 'com/dezide/author/gui/base/conversation/ParentConversationPageBase'
>
> but the class is defined as:
>
> public class SelectFaqArticle extends ParentConversationPageBase
>
> The code that failed was calling the following in
> ParentConversationPageBase:
>
> protected void setupParentConversationalSubPage(
> ParentConver