Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConfigurationClassParser does not use ApplicationContext's ResourceLoader for its MetadataReaderFactory [SPR-14684] #19248

Closed
spring-projects-issues opened this issue Sep 9, 2016 · 10 comments
Assignees
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Sep 9, 2016

Denis Miorandi opened SPR-14684 and commented

Scenario description:

I've got a spring webapp like this

  • webapp is main context + main classloader
  • plugins are child contexts + child classloaders. Note that plugins read files from disk
    (via custom classloader, not from jar)
    this work fine in spring 3.2.x.
    When migrated to spring 4.3.2 + spring boot 1.4, I've got an issue starting up.
    In some cases it doesn't load file using the expected resourceLoader (classpath instead of expexted file)
    The only changing is I've used a spring boot annotated class starting instead a web.xml one.

Issue:

If you look at attacched files (stack), it seems that spring scan two times

  1. On scan 1 spring use the webapp/plugin ResourceLoader, so that file are loaded as FileSystemResource as expected
  2. On scan 2 (ConfigurationClassParser one) spring use DefaultResourceLoader instead of the webapp/plugin one. In this case it recognize resource as classpath/jar and it fails (due to application logic). It seems that ConfigurationClassParser doesn't take in account main context

My ugly patch

Actually the only solution I found is to patch SimpleMetadataReaderFactory forcing my logic

{{ @Override
public MetadataReader getMetadataReader(String className) throws IOException {
String resourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
ClassUtils.convertClassNameToResourcePath(className) + ClassUtils.CLASS_FILE_SUFFIX;
Resource resource = this.resourceLoader.getResource(resourcePath);

	// CLESIUS PATCH - BEGIN
	// Load from disk instead of jar, otherwise crash
	if(SingletonLoader.getInstance()!=null && className.startsWith("it.clesius")){
		Class<?> cPlugin=SingletonLoader.getInstance().getClass(className);
		if (cPlugin!=null){
			try{
				resourcePath=cPlugin.getClassLoader().getResource(ClassUtils.convertClassNameToResourcePath(className) + ClassUtils.CLASS_FILE_SUFFIX).toString().replace("file://", "//");
				resource=new FileSystemResource(resourcePath);
			}catch(Exception e){
				System.out.println("CLESIUS PATCH - Error on spring clesius Patch");
			}
		}
	}		
	// CLESIUS PATCH - END		
	
	if (!resource.exists()) {
		// Maybe an inner class name using the dot name syntax? Need to use the dollar syntax here...
		// ClassUtils.forName has an equivalent check for resolution into Class references later on.
		int lastDotIndex = className.lastIndexOf('.');
		if (lastDotIndex != -1) {
			String innerClassName =
					className.substring(0, lastDotIndex) + '$' + className.substring(lastDotIndex + 1);
			String innerClassResourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
					ClassUtils.convertClassNameToResourcePath(innerClassName) + ClassUtils.CLASS_FILE_SUFFIX;
			Resource innerClassResource = this.resourceLoader.getResource(innerClassResourcePath);
			if (innerClassResource.exists()) {
				resource = innerClassResource;
			}
		}
	}
	return getMetadataReader(resource);
}

}}


Affects: 4.3.2

Attachments:

Issue Links:

  • #19219 Cache ASM metadata at the context level
  • #19250 Consistent *Aware callbacks for TypeFilters, ImportSelectors and ImportBeanDefinitionRegistrars
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 9, 2016

Juergen Hoeller commented

ConfigurationClassPostProcessor instantiates its CachingMetadataReaderFactory via the ClassLoader only, not via the ResourceLoader, which leads to CachingMetadataReaderFactory internally creating its own DefaultResourceLoader for the given ClassLoader. This is clearly lacking and needs to be revised.

However, I'm surprised that this actually worked in 3.2.x! This is probably due to more defensive class loading in 4.x, making use of the CachingMetadataReaderFactory in places where it did not in 3.2.x. Or are you making use of configuration classes in places where you did not use them in the 3.2.x based version of your application?

In any case, we'll fix this for the upcoming 4.3.3.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 9, 2016

Denis Miorandi commented

In spring 3.2.x app boot from web.xml, now from spring boot. It's possible this code (ConfigurationClassPostProcessor ) was not used on my previous version of sw, before upgrade to spring 4.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 9, 2016

Juergen Hoeller commented

Alright, thanks for the clarification! I suppose a directly equivalent arrangement probably wouldn't have worked in 3.2.x either, so this isn't an immediate regression. In any case, we'll do our best to make this work in 4.3.3, scheduled for release late next week.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 9, 2016

Denis Miorandi commented

ok tks Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 15, 2016

Juergen Hoeller commented

It would be great if you could give the latest 4.3.3.BUILD-SNAPSHOT a try... We intend to release 4.3.3 next Monday but it'd be great to get your feedback ahead of that.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 15, 2016

Denis Miorandi commented

I should able to test it this afgternoon or at least on tomorrow

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 16, 2016

Denis Miorandi commented

I'm using springboot 1.4 + some spring 4.3.2 deps. I've replaced/forced spring-core+spring-context to snapshot.
It doesn't work. Is it enough? May I replace something else?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 16, 2016

Stéphane Nicoll commented

Denis Miorandi you must replace the whole spring framework and not individual jars. If you are using the spring-boot-starter-parent, just add <spring.version>4.3.2.BUILD-SNAPSHOT</spring.version> to your pom. Or just upgrade to Spring Boot 1.4.1.BUILD-SNAPSHOT that already uses the latest spring framework version.

Alway run mvn dependencies:list -D sort to make sure that you're using the expected version.

Or you can generate an app from start.spring.io and choose 1.4.1.BUILD-SNAPSHOT. That way you'll be on the latest release.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 16, 2016

Denis Miorandi commented

Probably you means <spring.version>4.3.3.BUILD-SNAPSHOT</spring.version> not 4.3.2..
Btw my spring-core and spring-context was 4.3.3.BUILD-SNAPSHOT on test, checked in WEB-INF lib. I miss just to update spring.boot to 1.4.1.BUILD-SNAPSHOT
but I suspect changes are in core/context not in spring boot.
Anyway I'll try during weekend if possible.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 18, 2016

Denis Miorandi commented

I've made test with full spring replacement 1.4.1.BUILD-SNAPSHOT + 4.3.3.BUILD-SNAPSHOT. Btw I've verified war file outcome and there was no contamination of older versions.
It didn't work. I've got same behavior (from external perspective) that in 4.3.2. Btw I should make a debug session to give your more informations. Before do it what kind of test did you do?
Are you able to replicate a scenario like the not working one in your automatic tests?

Actually without my patch it doesn't solve the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants