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

Clarify WebContentInterceptor path mappings and efficiently match them [SPR-15096] #19663

Closed
spring-issuemaster opened this issue Jan 5, 2017 · 3 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Jan 5, 2017

Hurelhuyag opened SPR-15096 and commented

I expected this method return on first ant match. But this method always checking all matcher and returns last one. I think this is wrong.

protected Integer lookupCacheSeconds(String urlPath) {
		// Direct match?
		Integer cacheSeconds = this.cacheMappings.get(urlPath);
		if (cacheSeconds == null) {
			// Pattern match?
			for (String registeredPath : this.cacheMappings.keySet()) {
				if (this.pathMatcher.match(registeredPath, urlPath)) {
					cacheSeconds = this.cacheMappings.get(registeredPath);
				}
			}
		}
		return cacheSeconds;
	}

Reference URL: https://github.com/spring-projects/spring-framework/blob/master/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/WebContentInterceptor.java#L241

Referenced from: commits 801b93a, af7289d

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 5, 2017

Hurelhuyag commented

Oh, Sorry I forget to select project on create.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 5, 2017

Mark Paluch commented

I moved the ticket into the Spring Framework project.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 17, 2017

Juergen Hoeller commented

I'm afraid there is no specific order to the cacheMappings to begin with since it is a Properties instance, i.e. a Hashtable with no well-defined order of keys. Effectively, the behavior for overlapping path mappings is undefined there; we do not intend for this to be used with overlapping mappings.

That said, since the paths are not supposed to overlap to begin with here, there is no point in matching against all registered paths. For efficiency, we should proceed with the first match found. Nevertheless, this does not mean reliable ordering, just a more efficient match of a unique pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.