Skip to content

Support multiple paths in DispatcherServletPathProvider #13603

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

Closed
mbhave opened this issue Jun 27, 2018 · 7 comments
Closed

Support multiple paths in DispatcherServletPathProvider #13603

mbhave opened this issue Jun 27, 2018 · 7 comments
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@mbhave
Copy link
Contributor

mbhave commented Jun 27, 2018

No description provided.

@mbhave mbhave added the type: enhancement A general enhancement label Jun 27, 2018
@mbhave mbhave added this to the 2.0.x milestone Jun 27, 2018
@mbhave mbhave self-assigned this Jun 27, 2018
@mbhave mbhave closed this as completed in fddc9e9 Jun 28, 2018
@mbhave mbhave modified the milestones: 2.0.x, 2.0.4 Jun 28, 2018
@filiphr
Copy link
Contributor

filiphr commented Jul 12, 2018

Isn't the change in DispatcherServletPathProvider a breaking change between 2.0.3 and 2.0.4 in the API? I am not sure how you are standing on such changes and whether you've noticed it (I am trying out 2.0.4.BUILD-SNAPSHOT for another issue and noticed it).

Maybe something like:

public interface DispatcherServletPathProvider {

        @Deprecated
        String getServletPath();

	default Set<String> getServletPaths() {
            String servletPath = getServletPath();
            Set<String> servletPaths = new HashSet<>();
            servletPaths.add(servletPath);
            return servletPaths;
        }
}

Would be more appropriate?

We are only a consumer of this interface (see this) and we can easily change it. However, when people upgrade to 2.0.4 and we don't have a new release in the same time as you then they will have compilation issues

@mbhave
Copy link
Contributor Author

mbhave commented Jul 12, 2018

@filiphr You're right, it is a breaking change between 2.0.3 and 2.0.4. However, the DispatcherServletPathProvider was introduced quite recently (in 2.0.2) and we didn't expect that there would be too many consumers of this class. This change originally stemmed from #13527.

I'll reopen the issue to see what the rest of the team thinks.

@mbhave mbhave reopened this Jul 12, 2018
@mbhave mbhave added the for: team-attention An issue we'd like other members of the team to review label Jul 12, 2018
@filiphr
Copy link
Contributor

filiphr commented Jul 12, 2018

Thanks for having a look at it. I noticed that it is quite new as well. My usage for it is due to #13183

@wilkinsona
Copy link
Member

I think the breaking change is OK as it is fixing a bug in the API. While the default method is a nice non-breaking solution, I don’t think it is warranted in this case.

@mbhave mbhave closed this as completed Jul 13, 2018
@mbhave mbhave removed the for: team-attention An issue we'd like other members of the team to review label Jul 13, 2018
@snicoll snicoll reopened this Jul 24, 2018
@philwebb philwebb self-assigned this Jul 24, 2018
@philwebb
Copy link
Member

Reopened because #12971 is making me reconsider if we should have multiple path support

@philwebb
Copy link
Member

See also #12934

@philwebb
Copy link
Member

We're going to roll back support for multiple paths because of the issues identified in #13834

@philwebb philwebb removed the type: enhancement A general enhancement label Jul 26, 2018
@philwebb philwebb removed this from the 2.0.4 milestone Jul 26, 2018
@philwebb philwebb added the status: declined A suggestion or change that we don't feel we should currently apply label Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

5 participants