GroovyDynamicElementReader tries to read a Groovy script as XML and fails [SPR-11627]

[spring-projects-issues]

Abhijit Sarkar opened SPR-11627 and commented

GroovyDynamicElementReader tries to read a Groovy script as XML and fails.

beans {
    xmlns([ctx:'http://www.springframework.org/schema/context'])
    ctx.'component-scan'('base-package':'name.abhijitsarkar.moviedatabase')
}

Caused by: org.springframework.beans.factory.BeanDefinitionStoreException: Failed to read XML document; nested exception is org.springframework.beans.factory.BeanDefinitionStoreException: Unable to determine validation mode for [Groovy]: cannot open InputStream. Did you attempt to load directly from a SAX InputSource without specifying the validationMode on your XmlBeanDefinitionReader instance?; nested exception is java.io.FileNotFoundException: Groovy cannot be opened because it does not point to a readable resource
    at org.springframework.beans.factory.xml.XmlReaderContext.readDocumentFromString(XmlReaderContext.java:98)
    at org.springframework.beans.factory.xml.XmlReaderContext$readDocumentFromString.call(Unknown Source)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:45)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:108)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:116)
    at org.springframework.beans.factory.groovy.GroovyDynamicElementReader.invokeMethod(GroovyDynamicElementReader.groovy:96)

---

Affects: 4.0.3

Reference URL: http://forum.spring.io/forum/spring-projects/boot/746962-groovydynamicelementreader-tries-to-read-a-groovy-script-as-xml-and-fails

Issue Links:

• #15858 Support Groovy scripts for bean definitions in the TestContext framework

[spring-projects-issues]

Abhijit Sarkar commented

Curious, how's this an "improvement"? A bare bone context loader handles the file just fine, seems more like a bug.

public class GenericGroovyContextLoader extends AbstractGenericContextLoader {
    @Override
    protected BeanDefinitionReader createBeanDefinitionReader(final GenericApplicationContext context) {
        final GroovyBeanDefinitionReader reader = new GroovyBeanDefinitionReader(context);

        final Binding binding = getBinding(reader);

        reader.setBinding(binding);

        return reader;
    }

    protected Binding getBinding(final GroovyBeanDefinitionReader reader) {
        return new Binding();
    }

    @Override
    protected String getResourceSuffix() {
        return ".groovy";
    }
}

[spring-projects-issues]

Sam Brannen commented

Abhijit Sarkar, you may be interested in watching #15858, since it will add explicit support for loading contexts from Groovy scripts in the testing framework.

[spring-projects-issues]

Juergen Hoeller commented

I suppose I'm missing something here: In regular operations, we're setting GroovyBeanDefinitionReader's internal XmlBeanDefinitionReader to non-validating, which means you'd never run into that validation mode exception to begin with. So I assumed that you're invoking GroovyDynamicElementReader using a different code path, which we could make more defensive (and that'd be an improvement)... Or are we talking about Boot calling into this in some custom way?

Juergen

[spring-projects-issues]

Abhijit Sarkar commented

I'm not invoking groovyDynamicElementReader directly all and I don't think I'm doing anything special. Please see my configuration below:

client-config.groovy

beans {
    importBeans('classpath:service-config.groovy')

    xmlns([ctx:'http://www.springframework.org/schema/context'])
    ctx.'component-scan'('base-package':'name.abhijitsarkar.moviedatabase')
}

MovieDatabaseRESTClientIntegrationTest.groovy

RunWith(SpringJUnit4ClassRunner)
@ContextConfiguration(value = ['classpath:client-config.groovy', 'classpath:integ-test-config.groovy'],
        loader = PatchedSpringApplicationContextLoader)
@SpringApplicationConfiguration(classes = MovieDatabaseApplication)
@WebAppConfiguration
@IntegrationTest
class MovieDatabaseRESTClientIntegrationTest {

MovieDatabaseApplication.groovy

@EnableAutoConfiguration
@ComponentScan
class MovieDatabaseApplication {
    static void main(String[] args) {
        final Set<Object> sources = [MovieDatabaseApplication, 'classpath:client-config.groovy'] as Set
        final SpringApplication app = new SpringApplication()
        app.setSources(sources)

        app.run(args)
    }
}

The PatchedSpringApplicationContextLoader is needed because of this bug #16251. It is really SpringApplicationContextLoader with a bugfix (spring-boot-603), as the name suggests. SpringApplicationContextLoader is the loader used by SpringApplicationConfiguration.

[spring-projects-issues]

Sam Brannen commented

FYI: I see several issues with your configuration.

1. Declaring both @ContextConfiguration and @SpringApplicationConfiguration directly on a test class is not supported. The first instance of @ContextConfiguration that is discovered for a given test class (either on a class in the test class hierarchy or as a meta-annotation) will be used. All others will be ignored.

Note that @SpringApplicationConfiguration is itself annotated with @ContextConfiguration. Therefore, the declaration of @SpringApplicationConfiguration must be sufficient on its own.

2. Declaring both classes and locations/value for @ContextConfiguration is not supported. You are only allowed to declare one or the other. See the "Testing" chapter of the reference manual as well as the Javadoc for @ContextConfiguration for details.

In general, you need to select "one entry point" into your configuration. This is how production code works, and this is how test code works. You can always import other styles. For example, JavaConfig can import XML; XML can import JavaConfig; Groovy can import XML; etc.

Hope this helps clarify things a bit for you.

Regards,

Sam

[spring-projects-issues]

Abhijit Sarkar commented

I understand the points you brought up; let me explain why I chose that configuration and you can tell me if I'm doing something wrong (which's possible).

1. @ContextConfiguration and @SpringApplicationConfiguration - @ContextConfiguration is required to load the beans config files. Is there any other way to load those?
   I understand that @SpringApplicationConfiguration is itself annotated with @ContextConfiguration but the problem is that it uses the SpringContextLoaderListener, which assumes that the application is either using all Java config or all XML (I don't think it even considers Groovy). Hence, if it sees classes or locations on the @ContextConfiguration, it assumes the class that starts the application doesn't need to be registered (MovieDatabaseApplication in my comment above). That class, in turn, has @EnableAutoConfiguration on it which then is skipped.

That's why I use @SpringApplicationConfiguration and provide a patched SpringContextLoaderListener loader to @ContextConfiguration that registers the MovieDatabaseApplication class, thereby enabling auto configuration. If @ContextConfiguration accepted both classes and locations/value, then @SpringApplicationConfiguration wouldn't be needed because the classes to register could be derived from the annotation alone.

2. "Declaring both classes and locations/value for @ContextConfiguration is not allowed". True. That's the exact reason I had to use both @ContextConfiguration and @SpringApplicationConfiguration as explained above.

To your last point, I don't want to use any XML or Java @Configuration at all. I want to build the app using 100% Groovy bean builder, which's supposed to be a flagship feature of Spring 4.

[spring-projects-issues]

Dave Syer commented

To your point (2), I see no reason why classes= and locations= should not be supported now (#16257). Historically it didn't make sense, but Spring Boot supports mixed config sources, so there is no reason to validate so aggressively now. This has nothing to do with the current issue "Groovy script as XML".

I think you are mistaken in point (1) (also not related to the current issue): @SpringApplicationConfiguration supports beans{} configuration from its locations= attribute (I'm not sure if @ContextConfiguration does or not).

I'm not even sure why you would use beans{} to declare a component scan, when your MovieDatabaseApplication is part of the application, but I guess that's also off topic.

The code you posted in the link on Stackoverflow (https://github.com/abhijitsarkar/groovy/tree/master/movie-database) might reveal more about your intentions, so I'll have a look.

[spring-projects-issues]

Dave Syer commented

Maybe the original issue here is just that the Spring beans{} processor does not support XML namespaces, but the error isn't particularly helpful? I didn't see any tests for namespace support (positive or negative) so maybe it was an oversight?

[spring-projects-issues]

Abhijit Sarkar commented

I've more clarification to make but first I'd like to know how to do inline code blocks? That'd help the readability; I already tried with backquotes once, it didn't work, don't want to do it again.

[spring-projects-issues]

Dave Syer commented

It's Textile markup so double-braces is inline code (e.g. {{code}} like that).

[spring-projects-issues]

Abhijit Sarkar commented

Thanks Dave. So what I wanted to clarify is that is to your point earlier that there's no test case to prove GroovyBeanDefinitionReader does process namespaces. Surprisingly, in another module of the same project, I find it does. The difference is that module doesn't use a Boot loader, it uses one I wrote. I give the code below along with links to the actual files.

AbstractSpringIntegrationTest.groovy

@RunWith(SpringJUnit4ClassRunner)
@GroovyContextConfiguration(locations = ['classpath:service-config.groovy', 'classpath:integ-test-config.groovy',
        'classpath:service-config-scan.groovy'])
abstract class AbstractSpringIntegrationTest {
}

service-config-scan.groovy

beans {
    xmlns([ctx:'http://www.springframework.org/schema/context'])
    ctx.'component-scan'('base-package':'name.abhijitsarkar.moviedatabase')
}

GroovyContextConfiguration

@ContextConfiguration(loader = GenericGroovyContextLoader.class)
@Documented
@Inherited
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
public @interface GroovyContextConfiguration {
// More stuff similar to SpringApplicationConfiguration

GenericGroovyContextLoader.java

public class GenericGroovyContextLoader extends AbstractGenericContextLoader {
    private static final Logger LOGGER = LoggerFactory.getLogger(GenericGroovyContextLoader.class);

    @Override
    protected BeanDefinitionReader createBeanDefinitionReader(final GenericApplicationContext context) {
        final GroovyBeanDefinitionReader reader = new GroovyBeanDefinitionReader(context);

        final Binding binding = getBinding(reader);

        reader.setBinding(binding);

        /* Don't do this - Spring will call refresh later.
         * GenericApplicationContext does not support multiple refresh attempts.
         */
        // context.refresh()

        return reader;
    }

    protected Binding getBinding(final GroovyBeanDefinitionReader reader) {
        Binding binding = reader.getBinding();

        binding = (binding == null ? new Binding() : binding);

        try {
            binding.setVariable("configFileURL", new ClassPathResource("config.groovy").getURL());
        } catch (IOException ioe) {
            LOGGER.warn("Unable to set binding variable 'configFileURL'.");
        }

        return binding;
    }

    @Override
    protected String getResourceSuffix() {
        return ".groovy";
    }
}

[spring-projects-issues]

Abhijit Sarkar commented

Tested today with 4.0.4.BUILD-SNAPSHOT and the following works. I wanted to confirm that if I specify the loader specifically as I'm doing below, I don't need @SpringApplicationConfiguration at all. Right?

@ContextConfiguration(value = ['classpath:client-config.groovy', 'classpath:integ-test-config.groovy'],
        classes = [MovieDatabaseApplication],
        loader = SpringApplicationContextLoader)

or the other way around:

@SpringApplicationConfiguration(locations = ['classpath:client-config.groovy', 'classpath:integ-test-config.groovy'],
        classes = [MovieDatabaseApplication])

[spring-projects-issues]

Dave Syer commented

I can confirm that the two forms above are equivalent (but that's more relevant to #16257 isn't it). I don't see what your custom loader is doing that is different. Can you explain why you think it works, and with what versions of Spring it works with namespaces?

[spring-projects-issues]

Abhijit Sarkar commented

I'm not sure why one works and the other doesn't. I wanted to put the information out hoping that it might give a clue to someone who knows the code more than me.
I can try to do some debugging and see what I can find. Might take a day or two.

[spring-projects-issues]

Sam Brannen commented

Hi guys,

I'm following up on Juergen's initial comments...

I suppose I'm missing something here: In regular operations, we're setting GroovyBeanDefinitionReader's internal XmlBeanDefinitionReader to non-validating, which means you'd never run into that validation mode exception to begin with. So I assumed that you're invoking GroovyDynamicElementReader using a different code path, which we could make more defensive (and that'd be an improvement)... Or are we talking about Boot calling into this in some custom way?

Abhijit's custom GenericGroovyContextLoader creates a GroovyBeanDefinitionReader as follows:

final GroovyBeanDefinitionReader reader = new GroovyBeanDefinitionReader(context);

This constructor creates its own XmlBeanDefinitionReader and turns off validation as follows:

this.xmlBeanDefinitionReader = new XmlBeanDefinitionReader(registry);
this.xmlBeanDefinitionReader.setValidating(false);

The difference is that Spring Boot's BeanDefinitionLoader - which is used internally by Spring Boot's SpringApplication and thus indirectly via SpringApplicationContextLoader - creates its GroovyBeanDefinitionReader as follows:

this.groovyReader = new GroovyBeanDefinitionReader(this.xmlReader);

And this constructor does not disable validation for the supplied XmlBeanDefinitionReader; it just uses it as is.

So I guess one solution would be for Spring Boot's BeanDefinitionLoader to instantiate the GroovyBeanDefinitionReader using the BeanDefinitionRegistry constructor instead of the XmlBeanDefinitionReader constructor.

Of course, I haven't tested it, but it's a thought... ;)

Regards,

Sam

[spring-projects-issues]

Abhijit Sarkar commented

Actually just setting the xmlReader.validationMode to false is enough; the GroovyBeanDefinitionReader instantiation need not be changed. I sent a pull 622, it works locally for the code I posted above.

[spring-projects-issues]

Sam Brannen commented

I don't think it's a good idea to set the xmlReader.validationMode to false.

Keep in mind that the xmlReader is used to read XML. So you don't want to disable validation for XML configuration files as an unintentional side effect.

That's why I recommended using the GroovyBeanDefinitionReader(BeanDefinitionRegistry) constructor.

Regards,

Sam