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

Let ResourceAccessor handle unchanged resource paths #3105

Closed
andrus opened this issue Jul 23, 2022 · 5 comments · Fixed by #3064
Closed

Let ResourceAccessor handle unchanged resource paths #3105

andrus opened this issue Jul 23, 2022 · 5 comments · Fixed by #3064

Comments

@andrus
Copy link

andrus commented Jul 23, 2022

Environment

Liquibase Version: 4.x
Liquibase Integration & Version: Bootique.io

Description

I am a bit late to the party, but I am finally upgrading the Bootique framework to Liquibase 4.x from 3.x. The new 4.x LB resource resolving mechanism makes it impossible to implement the "protocol-based" strategy used across Bootique for various types of resources:

  • a resource path starting with classpath: is handled as a classpath resource
  • a resource path starting with a known network protocol (say https://) is treated as a URL
  • any other path is treated as a filesystem path

We were able to maintain this abstraction with LB 3.x via a custom ResourceAccessor. In 4.x, DatabaseChangeLog strips the classpath: prefix before sending the path to the ResourceAccessor, so the ResourceAccessor can no longer distinguish between a classpath and filesystem-based resource.

The closest alternative that I found is CompositeResourceAccessor, but it makes its choice by eliminating all other choices (e.g. check the classpath, if not found, look in the filesystem), which is more wasteful and may cause conflicts (e.g. a user may ask specifically for classpath:x.yml, and the system would find an entirely different ./x.yml and vice versa).

Would be great to move all the path transformation logic inside ResourceAccessor to allow custom protocol-based resolving like the one described here.

If this is impossible under the current design, one other alternative would be to switch all constructor invocations of DatabaseChangeLog to be done via a factory. This is not as clean, but would allow to override the stripping behavior in a custom DatabaseChangeLog subclass.

@nvoxland
Copy link
Contributor

nvoxland commented Aug 2, 2022

@andrus We've been working on the ResourceAccessor stuff a bit lately and hoping it addresses your problems. Both with the latest 4.14 release and also in an in-progress PR. If it's not and/or you have suggestions on how to improve it, now is a great time to help us with it.

The ResourceAccessor implementations are really around are mainly designed around how a particular location can be checked for files. But before 4.14 there wasn't a way for end-users to configure what to search, it was really up to the the integration configuring liquibase.

With 4.14, we added a "searchPath" configuration which is a way for users to specify the locations, with a pluggable liquibase.resource.PathHandler interface for recognizing different schemes.

For example, you can maybe create a new liquibase.resource.SearchPathResourceAccessor and pass in the searchPath configuration which is a comma-separated list of whatever descriptive locations and those are each parsed with liquibase.resource.PathHandlerFactory to find the correct liquibase.resource.PathHandler implementation to use. We ship with PathHandlers for files but you can create custom ones and/or contribute others back to us if they're generally useful.

However, I'm also working on wrapping up #3064 which will not make it into 4.15 in a couple days but should be in 4.16 in a couple weeks. That introduces a new liquibase.liquibase.Resource object mainly for the purpose of allowing writing from the ResourceAccessor, but it also forces us to make some changes to the ResourceAccessor and PathHandler interfaces.

I'm thinking the PathHandler extension point is what you need? But let me know if it's not and we can make sure something gets in there with the overall ResourceAccessor work.

@kataggart kataggart linked a pull request Aug 3, 2022 that will close this issue
3 tasks
@andrus
Copy link
Author

andrus commented Aug 4, 2022

Hi, @nvoxland , thanks for the update. Unfortunately SearchPathResourceAccessor does not solve the problem for me, as classpath: stripping still occurs before the path is passed to any ResourceAccessor. Will be happy to try the new PathHandler when it becomes available.

@nvoxland
Copy link
Contributor

nvoxland commented Aug 7, 2022

Makes sense. I'll take a look at that

@nvoxland
Copy link
Contributor

I maybe see what you're getting at: you are looking to be able to have users pass along changelogfile=classpath:/my/changelog.xml or include path="http://hostname/other/changelog.xml" ?

If so, the reason that ended up breaking with 4.x is because we've always tried to exclude that "complete path specified" style for changelog files in favor of always listing files relative to the search path. The idea is that the search path is the set of "absolute" base locations which may change depending on the environment and the paths of the changelogs are therefore always environment independent. We get this by setting the searchPath/resourceAccessor to know how how and where to look up the "simple" paths to the changelogs given.

We're continuing to try to improve that with the better "SearchPath" naming and also the new PathHandler interface that lets you more easily separate the ResourceAccessor logic from how to handle different url schemes within that.

Am I right in my understanding of where you are wanting to specify those complete "urls"?

@andrus
Copy link
Author

andrus commented Sep 15, 2022

Correct. My view (and 3.x view) of ResourceAccessor is something that can process an unaltered URL from a config. This gives maximum flexibility to customize URL resolving. 4.x uses "base locations", and resolver can only work with the tail end of the URL, so many customizations are no longer possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants