Mailing-List: contact issues-help@struts.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@struts.apache.org
Date: Wed, 6 Sep 2017 22:32:00 +0000 (UTC)
From: "Mitth'raw'nuruodo (JIRA)" <jira@apache.org>
To: issues@struts.apache.org
Message-ID: <JIRA.13100000.1504659698000.44159.1504737120548@Atlassian.JIRA>
In-Reply-To: <JIRA.13100000.1504659698000@Atlassian.JIRA>
References: <JIRA.13100000.1504659698000@Atlassian.JIRA> <JIRA.13100000.1504659698006@jira-lw-us.apache.org>
Subject: [jira] [Comment Edited] (WW-4849) ObjectFactory constructor
 signature change breaks extensions
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Wed, 06 Sep 2017 22:32:07 -0000


    [ https://issues.apache.org/jira/browse/WW-4849?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16156121#comment-16156121 ] 

Mitth'raw'nuruodo edited comment on WW-4849 at 9/6/17 10:31 PM:
----------------------------------------------------------------

[~aleksandr-m] I'm not sure how much of your previous comment was directed at me vs Lukasz...however:

"Required objects should be injected in the constructor" - this is difficult for any custom ObjectFactory to do, given that it [has to have a no-arg constructor|https://struts.apache.org/docs/objectfactory.html] and then has to immediately pass a Container to its superclass constructor.

"Have you tested it with some non default object factory e.g. spring?" - my pull request includes unit testing. As far as I can see, it will accept the first Container that gets injected, either via constructor or setter, and will then ignore further injection attempts.

*Note that the {{SpringObjectFactory}} - and the other factories provided by the project, like {{CdiObjectFactory}} and {{PlexusObjectFactory}} - are not compliant with the documentation about custom object factories at https://struts.apache.org/docs/objectfactory.html, since they lack no-arg constructors.*


was (Author: thrawnca):
[~aleksandr-m] I'm not sure how much of your previous comment was directed at me vs Lukasz...however:

"Required objects should be injected in the constructor" - this is difficult for any custom ObjectFactory to do, given that it [has to have a no-arg constructor|https://struts.apache.org/docs/objectfactory.html] and then has to immediately pass a Container to its superclass constructor.

"Have you tested it with some non default object factory e.g. spring?" - my pull request includes unit testing. As far as I can see, it will accept the first Container that gets injected, either via constructor or setter, and will then ignore further injection attempts.

*Note that the {{SpringObjectFactory}} - and the other factories provided by the project, like {{CdiObjectFactory}} and {{PlexusObjectFactory}} are not compliant with the documentation about custom object factories at https://struts.apache.org/docs/objectfactory.html, since they lack no-arg constructors.*

> ObjectFactory constructor signature change breaks extensions
> ------------------------------------------------------------
>
>                 Key: WW-4849
>                 URL: https://issues.apache.org/jira/browse/WW-4849
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 2.5.13
>            Reporter: Mitth'raw'nuruodo
>             Fix For: 2.5.14
>
>
> Commit {{6f91d0776a545c911ca4f2875ed9976614711ef9}} changed the signature of the {{ObjectFactory}} constructor, breaking all classes that extend {{ObjectFactory}} (as per https://struts.apache.org/docs/objectfactory.html). This affects eg the [{{guice-servlet}} Struts plugin| https://github.com/google/guice/blob/master/extensions/struts2/src/com/google/inject/struts2/Struts2Factory.java].
> This was not listed on the [2.5.13 version notes|https://struts.apache.org/docs/version-notes-2513.html] as a breaking change, and breaking changes should preferably be avoided in critical security updates.


--
This message was sent by Atlassian JIRA
(v6.4.14#64029)