[Geoserver-devel] geofence-server broken due to changes in GeoServerTestSupport

Hello Andrea and Jody,

There are two tests in geofence-server that are based on GeoServerTestSupport to initialize the spring context.

They used to work fine but only since recently they started breaking, and I traced it back to this commit you guys did: https://github.com/geoserver/geoserver/commit/ed613876cb6aba59fd922e25fd957f18a85815a2 ("Build speed up with explicitly setting which beans need early initia……lization instead).

So I don't understand it 100% yet, but it seems that 'default-autowire="byName"' doesn't function any longer in the test cases, and since geofence beans rely on that they aren't loading properly any more .

Before I spend a lot of more time on this I wanted to check if you guys had an easy solution. I noticed the commit adds a lot of 'lazy-init="false"' to the bean definitions. Now the problem is that this regards beans defined in the geofence modules. Does that mean we need to add that stuff to the geofence modules as well?

Regards
Niels

On Fri, May 22, 2015 at 12:17 PM, Niels Charlier <niels@anonymised.com> wrote:

Hello Andrea and Jody,

There are two tests in geofence-server that are based on
GeoServerTestSupport to initialize the spring context.

They used to work fine but only since recently they started breaking, and
I traced it back to this commit you guys did:
https://github.com/geoserver/geoserver/commit/ed613876cb6aba59fd922e25fd957f18a85815a2
("Build speed up with explicitly setting which beans need early
initia……lization instead).

So I don't understand it 100% yet, but it seems that
'default-autowire="byName"' doesn't function any longer in the test cases,
and since geofence beans rely on that they aren't loading properly any more
.

Before I spend a lot of more time on this I wanted to check if you guys
had an easy solution. I noticed the commit adds a lot of
'lazy-init="false"' to the bean definitions. Now the problem is that this
regards beans defined in the geofence modules. Does that mean we need to
add that stuff to the geofence modules as well?

Only to the beans whose creation has side effects I'd say.
In GeoServer we marked as lazy-init=false those beans whose initialization
attaches listeners, and otherwise changes
the enviroment around them. All other beans should be loadable as they are
actually needed.

Cheers
Andrea

--

Meet us at the INSPIRE Conference in Lisbon 25-29 May 2015! Visit
http://goo.gl/WHKDXT for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*

Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
loro utilizzo è consentito esclusivamente al destinatario del messaggio,
per le finalità indicate nel messaggio stesso. Qualora riceviate questo
messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
darcene notizia via e-mail e di procedere alla distruzione del messaggio
stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso,
divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od
utilizzarlo per finalità diverse, costituisce comportamento contrario ai
principi dettati dal D.Lgs. 196/2003.

The information in this message and/or attachments, is intended solely for
the attention and use of the named addressee(s) and may be confidential or
proprietary in nature or covered by the provisions of privacy act
(Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection
Code).Any use not in accord with its purpose, any disclosure, reproduction,
copying, distribution, or either dissemination, either whole or partial, is
strictly forbidden except previous formal approval of the named
addressee(s). If you are not the intended recipient, please contact
immediately the sender by telephone, fax or e-mail and delete the
information in this message that has been received in error. The sender
does not give any warranty or accept liability as the content, accuracy or
completeness of sent messages and accepts no responsibility for changes
made after they were sent or for other risks which arise as a result of
e-mail transmission, viruses, etc.

-------------------------------------------------------

For all clarity, with geofence modules I was talking about the actual geofence backend modules themselves, not the community modules in geoserver. Because the problem concerns beans declared in geofence itself, not the community modules. So this would mean geofence itself needs to be changed. I don’t think the problem has anything to do with beans having effects that change the environment. The problem is very clearly that 'default-autowire=“byName” is not working any longer for any context that is loaded. I have confirmed this. Is this a desired or accidental effect of your changes? Because geofence relies on autowire byname. Regards Niels

···

On 22-05-15 12:28, Andrea Aime wrote:

Before I spend a lot of more time on this I wanted to check if you guys had an easy solution. I noticed the commit adds a lot of ‘lazy-init=“false”’ to the bean definitions. Now the problem is that this regards beans defined in the geofence modules. Does that mean we need to add that stuff to the geofence modules as well?

Only to the beans whose creation has side effects I’d say.
In GeoServer we marked as lazy-init=false those beans whose initialization attaches listeners, and otherwise changes
the enviroment around them. All other beans should be loadable as they are actually needed.

On Fri, May 22, 2015 at 1:23 PM, Niels Charlier <niels@anonymised.com> wrote:

For all clarity, with geofence modules I was talking about the actual
*geofence* backend modules themselves, not the community modules in
geoserver. Because the problem concerns beans declared in geofence itself,
not the community modules. So this would mean geofence itself needs to be
changed.

I don't think the problem has anything to do with beans having effects
that change the environment. The problem is very clearly that
'default-autowire="byName" is not working any longer for any context that
is loaded. I have confirmed this. Is this a desired or accidental effect of
your changes? Because geofence relies on autowire byname.

The thing is, I never used autowiring, so I don't know. A generic search on
the internet might help though.
Also, it should be fairly easy to add a method to
GeoServerSystemTestSupport that
forces GeoServerTestApplicationContext not to use lazy loading for test
that cannot handle it.

Cheers
Andrea

--

Meet us at the INSPIRE Conference in Lisbon 25-29 May 2015! Visit
http://goo.gl/WHKDXT for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*

Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
loro utilizzo è consentito esclusivamente al destinatario del messaggio,
per le finalità indicate nel messaggio stesso. Qualora riceviate questo
messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
darcene notizia via e-mail e di procedere alla distruzione del messaggio
stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso,
divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od
utilizzarlo per finalità diverse, costituisce comportamento contrario ai
principi dettati dal D.Lgs. 196/2003.

The information in this message and/or attachments, is intended solely for
the attention and use of the named addressee(s) and may be confidential or
proprietary in nature or covered by the provisions of privacy act
(Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection
Code).Any use not in accord with its purpose, any disclosure, reproduction,
copying, distribution, or either dissemination, either whole or partial, is
strictly forbidden except previous formal approval of the named
addressee(s). If you are not the intended recipient, please contact
immediately the sender by telephone, fax or e-mail and delete the
information in this message that has been received in error. The sender
does not give any warranty or accept liability as the content, accuracy or
completeness of sent messages and accepts no responsibility for changes
made after they were sent or for other risks which arise as a result of
e-mail transmission, viruses, etc.

-------------------------------------------------------

Okay, I found nothing on google but I have further debugged, as I had expected it appears the problems are not caused by lazy initialization, but by the following single line *https://github.com/geoserver/geoserver/blob/ed613876cb6aba59fd922e25fd957f18a85815a2/src/main/src/test/java/org/geoserver/test/GeoServerTestApplicationContext.java#L44**

*

setValidating(false);

Surprisingly turning validation off causes serious malfunctions as spring does not interpret the xml correctly.

If you look in the schema at http://www.springframework.org/schema/beans/spring-beans-3.0.xsd you find the following bean attribute definition:

<xsd:attribute name="autowire" default="default">

Now what normally happens in the spring code is that when you don't specify an autowire attribute in your bean definition, it is supposed to be set to "default" which tells spring to use the autowire attribute that has been defined for the whole context, which is in our case "byname".

Without validation the xsd isn't processed and the default is apparently ignored, causing autowire to be an empty string for each bean instead of the string "default". The spring code cannot handle this empty string and reverts to no autowiring.

It's a bit dodgy that spring works that way, but apparently it does so. Note that other problems may occur as well because there are other attributes with default values.

I would recommend removing that line altogether since it seems a malfunction to me. Otherwise it must at least be made optional somehow because at this point it is already too late to change when oneTimeSetUp is run in my derived class.

Regards
Niels

On 22-05-15 13:51, Andrea Aime wrote:

On Fri, May 22, 2015 at 1:23 PM, Niels Charlier <niels@anonymised.com <mailto:niels@anonymised.com>> wrote:

    For all clarity, with geofence modules I was talking about the
    actual *geofence* backend modules themselves, not the community
    modules in geoserver. Because the problem concerns beans declared
    in geofence itself, not the community modules. So this would mean
    geofence itself needs to be changed.

    I don't think the problem has anything to do with beans having
    effects that change the environment. The problem is very clearly
    that 'default-autowire="byName" is not working any longer for any
    context that is loaded. I have confirmed this. Is this a desired
    or accidental effect of your changes? Because geofence relies on
    autowire byname.

The thing is, I never used autowiring, so I don't know. A generic search on the internet might help though.
Also, it should be fairly easy to add a method to GeoServerSystemTestSupport that
forces GeoServerTestApplicationContext not to use lazy loading for test that cannot handle it.

Cheers
Andrea

--

Meet us at the INSPIRE Conference in Lisbon 25-29 May 2015! Visit http://goo.gl/WHKDXT for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*

Le informazioni contenute in questo messaggio di posta elettronica e/o nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il loro utilizzo è consentito esclusivamente al destinatario del messaggio, per le finalità indicate nel messaggio stesso. Qualora riceviate questo messaggio senza esserne il destinatario, Vi preghiamo cortesemente di darcene notizia via e-mail e di procedere alla distruzione del messaggio stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso, divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità diverse, costituisce comportamento contrario ai principi dettati dal D.Lgs. 196/2003.

The information in this message and/or attachments, is intended solely for the attention and use of the named addressee(s) and may be confidential or proprietary in nature or covered by the provisions of privacy act (Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection Code).Any use not in accord with its purpose, any disclosure, reproduction, copying, distribution, or either dissemination, either whole or partial, is strictly forbidden except previous formal approval of the named addressee(s). If you are not the intended recipient, please contact immediately the sender by telephone, fax or e-mail and delete the information in this message that has been received in error. The sender does not give any warranty or accept liability as the content, accuracy or completeness of sent messages and accepts no responsibility for changes made after they were sent or for other risks which arise as a result of e-mail transmission, viruses, etc.

-------------------------------------------------------

On Fri, May 22, 2015 at 4:26 PM, Niels Charlier <niels@anonymised.com> wrote:

I would recommend removing that line altogether since it seems a
malfunction to me. Otherwise it must at least be made optional somehow
because at this point it is already too late to change when oneTimeSetUp is
run in my derived class.

Please make it optional, we were spending significant time validating over
and over the spring xml files during the build

Cheers
Andrea

--

Meet us at the INSPIRE Conference in Lisbon 25-29 May 2015! Visit
http://goo.gl/WHKDXT for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*

Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
loro utilizzo è consentito esclusivamente al destinatario del messaggio,
per le finalità indicate nel messaggio stesso. Qualora riceviate questo
messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
darcene notizia via e-mail e di procedere alla distruzione del messaggio
stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso,
divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od
utilizzarlo per finalità diverse, costituisce comportamento contrario ai
principi dettati dal D.Lgs. 196/2003.

The information in this message and/or attachments, is intended solely for
the attention and use of the named addressee(s) and may be confidential or
proprietary in nature or covered by the provisions of privacy act
(Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection
Code).Any use not in accord with its purpose, any disclosure, reproduction,
copying, distribution, or either dissemination, either whole or partial, is
strictly forbidden except previous formal approval of the named
addressee(s). If you are not the intended recipient, please contact
immediately the sender by telephone, fax or e-mail and delete the
information in this message that has been received in error. The sender
does not give any warranty or accept liability as the content, accuracy or
completeness of sent messages and accepts no responsibility for changes
made after they were sent or for other risks which arise as a result of
e-mail transmission, viruses, etc.

-------------------------------------------------------

Done Regards Niels

···

On 22-05-15 16:28, Andrea Aime wrote:

On Fri, May 22, 2015 at 4:26 PM, Niels Charlier <niels@anonymised.com> wrote:

I would recommend removing that line altogether since it seems a malfunction to me. Otherwise it must at least be made optional somehow because at this point it is already too late to change when oneTimeSetUp is run in my derived class.

Please make it optional, we were spending significant time validating over and over the spring xml files during the build

https://github.com/geoserver/geoserver/commit/bee15afc361c472228d8a314f7f56fd8f479af7f