Search for conf/application.conf and conf/routes from classpath (#198) #202

xabolcs · 2023-06-27T01:09:00Z

in case they don't exists in the working directory.

Running RePlay apps in a distributed form (zip, tar, docker) the conf/ directory doesn't always exists in the working directory, but in the classpath.

Fixes: #198

xabolcs · 2023-06-27T01:20:38Z

Hi @asolntsev,

here is my naive try for fixing #198. 😅

About the protected methods in PropertiesConfLoader: I added an extension point if anybody want their PropertiesConfLoader implementation. Sure, can be dropped. Feel free to edit / extend my commits!

About the solution: as you can see the pattern is the following. Those startup files are resolved against Play.applicationPath which defined as the current dir (System.getProperty("user.dir")) and if doesn't exists I try to resolve them against Classpath.
If they not found on the Classpath I leave the workflow intact - the VirtualFile's UnexpectedException is more descriptive.

Could we improve something about this pattern? How about extending VirtualFile? One resolves the files against Play.applicationPath the other uses Classpath. 🙂

framework/src/play/PropertiesConfLoader.java

xabolcs · 2023-06-27T08:42:08Z

It would be nice , but no tests added. 😅

xabolcs · 2023-06-27T20:57:45Z

framework/src/play/Play.java

@@ -158,6 +153,13 @@ public void init(String id) {
    setupApplicationMode();
    VirtualFile appRoot = setupAppRoot();
    routes = appRoot.child("conf/routes");
+    if (!routes.exists()) {
+      try {
+        routes = VirtualFile.open(Objects.requireNonNull(Thread.currentThread().getContextClassLoader().getResource("conf/routes")).getFile());


Which is better?

......getContextLoader().getResource() + relative path: conf/..., or

getClass().getResource() + absolute path: /conf/...?

Both works. 🤷‍♀️

I am not sure, but it seems to me reasonable to use getContextLoader() because it allows users to use some custom classloader (for whatever reason).

xabolcs · 2023-06-28T00:08:09Z

framework/src/play/PropertiesConfLoader.java

+  }
+
+  protected final VirtualFile resolveFileNameWithClasspath(String filename) {
+    return VirtualFile.open(Objects.requireNonNull(Thread.currentThread().getContextClassLoader().getResource("conf/" + filename)).getPath());


VirtualFile needs more love: it doesn't know jars. 😕

Exception in thread "main" play.exceptions.UnexpectedException: Failed to read /file:/app/classpath/app.jar!/conf/application.conf at play.vfs.VirtualFile.inputstream(VirtualFile.java:106) at play.libs.IO.readUtf8Properties(IO.java:17) at play.PropertiesConfLoader.readOneConfigurationFile(PropertiesConfLoader.java:63) at play.PropertiesConfLoader.readOneConfigurationFile(PropertiesConfLoader.java:34) at play.PropertiesConfLoader.readOneConfigurationFile(PropertiesConfLoader.java:29) at play.PropertiesConfLoader.readConfiguration(PropertiesConfLoader.java:25) at play.Play.readConfiguration(Play.java:260) at play.Play.init(Play.java:149)

Another breaking point:

Exception in thread "main" play.exceptions.UnexpectedException: Failed to read /file:/app/classpath/app.jar!/conf/routes at play.vfs.VirtualFile.contentAsString(VirtualFile.java:185) at play.mvc.routing.RoutesParser.parse(RoutesParser.java:28) at play.mvc.routing.RoutesParser.parse(RoutesParser.java:17) at play.mvc.Router.loadRoutesFromFile(Router.java:69) at play.mvc.Router.detectChanges(Router.java:131) at play.Play.start(Play.java:280)

@xabolcs Yes, initially in Play! framework VirtualFile intention was to resolve files from the project itself and its modules. It was not intended to find files in classpath.

@asolntsev, thanks for replying.

Yeah, these exceptions suggest that too.
How could we continue? For me it would be the best that the Framework could work with jars and classpath also ... without breaking VirtualFile's Play! 1 compatibility.
Of course the easiest way is to document these limitations for the RePlay App developers.
Of course, there are solutions in between too, like the current state! 🙂

Yeah, as in the log4j.properties PR, I use and pack my App's conf folder as a resource folder (becasuse my Idea hides a few folder if "they are too overlapping")

@xabolcs I started reviewing the PR. Sorry for a long delay.

I have lots of doubts at the moment...

When you call openWithEntryName, you always pass the same name twice its parameters. Duplication.

this change actually breaks VirtualFile API in that sense that many its methods will not work anymore.

@Test void broken() { File file = new File(Thread.currentThread().getContextClassLoader().getResource("org/opentest4j/AssertionFailedError.class").getFile()); VirtualFile virtualFile = VirtualFile.openWithEntryName(file, "org/opentest4j/AssertionFailedError.class"); virtualFile.child("some.txt"); // ? virtualFile.length(); // 0 virtualFile.hashCode(); // ? virtualFile.lastModified(); // 0 virtualFile.exists(); // FALSE! virtualFile.list(); // null ? }

Anyway, I'll try to incorporate this failing test and try to not break it with the first commit. 🙂

@xabolcs The more I think about it, the more I come to a conclusion that VirtualFile is an alternative for classpath. Either you find a file from classpath, or from VirtualFile.

So my suggestion is:

It's generally a good idea to find resources from classpath, but

Let's try to find them from two sources: classpath and VirtualFile.
2.1. I mean, method for searching in classpath should be outside of VirtualFile.

Yeah, and we are back to my first comment:

About the solution: as you can see the pattern is the following. Those startup files are resolved against Play.applicationPath which defined as the current dir (System.getProperty("user.dir")) and if doesn't exists I try to resolve them against Classpath.
If they not found on the Classpath I leave the workflow intact - the VirtualFile's UnexpectedException is more descriptive.

Could we improve something about this pattern? How about extending VirtualFile? One resolves the files against Play.applicationPath the other uses Classpath. 🙂

As I'm just a Java copy-paster and not a developer, I don't have solid thoughts how to achieve this "try to resolve the file first by VirtualFile then search it on the Classpath" thing.

How about extracting VirtualFile's methods of read-only functionality (as I don't want to write files on the classpath) to an interface and create an AbstractFileResolver which resolves to VirtualFile or ClasspathFile? 😀 Or just create a parent class for VirtualFile and the other class?

Sure there are other and better solution to this.

Yes, I was thinking something similar.
But maybe it's even more simpler. Currently you only need to find one file (e.g. conf/routes) - either from classpath or from VirtualFile.

Maybe we could just add a static method to VirtualFile? Say, find:

public static URL routes; ... routes = appRoot.find("conf/routes");

class VirtualFile { public static URL find(String name) { URL result = // find from classpath if (result == null) { result = // find from VirtualFile as previously } return result; } }

Cool! I'll go that way and see what happens!
Thanks for the hint!

xabolcs · 2023-09-30T20:27:02Z

@asolntsev, thanks for the hint about introducing VirtualFile.find(). It works nicely! 👌

Just to note, there is a public static VirtualFile search(Collection<VirtualFile> roots, String path) already (but I don't care, as I'm too bad at naming 😅 ).

Would you mind adding some tests?

one for file relative to working directory and
another from classpath and
third for the working dir file overrides classpath file.

As I wrote in the first commit, this change doesn't help with files found in jars:

Caused by: java.io.FileNotFoundException: file:/app/classpath/my-replay-app.jar!/conf/application.conf (No such file or directory)
        at java.base/java.io.FileInputStream.open0(Native Method)
        at java.base/java.io.FileInputStream.open(FileInputStream.java:219)
        at java.base/java.io.FileInputStream.<init>(FileInputStream.java:157)
        at play.vfs.VirtualFile.inputstream(VirtualFile.java:106)
        ... 8 more

Sure, I don't want to fix it right here, but as an improvement it would be nice to see as fixed once.

For example it helps containerizing, as it is able to find files on classpath, not just relative to working directory. While it improves compatibility with files found on classpath, VirtualFile still doesn't support files inside jars, e.g.: Caused by: java.io.FileNotFoundException: file:/app/classpath/my-replay-app.jar!/conf/application.conf (No such file or directory) at java.base/java.io.FileInputStream.open0(Native Method) at java.base/java.io.FileInputStream.open(FileInputStream.java:219) at java.base/java.io.FileInputStream.<init>(FileInputStream.java:157) at play.vfs.VirtualFile.inputstream(VirtualFile.java:106) ... 8 more

…-framework#198) in case they don't exists in the working directory. Running RePlay apps in a distributed form (zip, tar, docker) the conf/ directory doesn't always exists in the working directory, but in the classpath. Fixes: replay-framework#198

asolntsev · 2023-11-20T07:05:49Z

@xabolcs Hi. I finally looked at this PR with fresh eyes, and realised that VirtualFile is not needed at all.
It was used in Play1! framework because it had its own dependency resolution mechanism (play dependencies command) which created a folder modules with links to "module"-like dependencies.

RePlay replaced this mechanism with old good Gradle/Maven dependencies. Replay doesn't create modules folder anymore.

So we can just replace all usages of VirtualFile with the standard ClassLoader.getResource("conf/routes").

I can prepare a pull request.

xabolcs · 2023-11-20T07:14:43Z

Hi. I finally looked at this PR with fresh eyes, and realised that VirtualFile is not needed at all

🤔

Interesting news, but cleaning up the things is always better!

I can prepare a pull request.

That would be nice! Thanks!

Wouldn't be it a big boom to nuke th VirtualFile out from Replay? 🙃

xabolcs · 2023-11-20T07:54:53Z

RePlay replaced this mechanism with old good Gradle/Maven dependencies. Replay doesn't create modules folder anymore.

So we can just replace all usages of VirtualFile with the standard ClassLoader.getResource("conf/routes").
I can prepare a pull request.

This would help on #64 a little bit. 👌

xabolcs · 2023-11-21T18:24:05Z

Closing in favor of #291.

xabolcs force-pushed the 198-conf-application-from-classpath branch 2 times, most recently from a1edc32 to 8b9dbef Compare June 27, 2023 05:51

xabolcs commented Jun 27, 2023

View reviewed changes

framework/src/play/PropertiesConfLoader.java Outdated Show resolved Hide resolved

xabolcs commented Jun 27, 2023

View reviewed changes

xabolcs commented Jun 28, 2023

View reviewed changes

xabolcs force-pushed the 198-conf-application-from-classpath branch from 5b092aa to ac7f441 Compare June 28, 2023 01:53

xabolcs marked this pull request as draft June 28, 2023 12:03

xabolcs marked this pull request as ready for review June 28, 2023 12:06

xabolcs force-pushed the 198-conf-application-from-classpath branch from ac7f441 to 948c809 Compare July 4, 2023 09:52

xabolcs force-pushed the 198-conf-application-from-classpath branch 2 times, most recently from 014c7a7 to 5e8cbca Compare August 18, 2023 11:38

xabolcs force-pushed the 198-conf-application-from-classpath branch from 5e8cbca to 22bf47b Compare September 30, 2023 20:14

xabolcs force-pushed the 198-conf-application-from-classpath branch from 22bf47b to b681919 Compare September 30, 2023 20:33

xabolcs changed the title ~~Resolve conf/application.conf and conf/routes from classpath (#198)~~ Search for conf/application.conf and conf/routes from classpath (#198) Sep 30, 2023

xabolcs added 2 commits October 14, 2023 15:49

xabolcs force-pushed the 198-conf-application-from-classpath branch from b681919 to 3389a45 Compare October 14, 2023 13:51

xabolcs marked this pull request as draft November 20, 2023 09:41

xabolcs closed this Nov 21, 2023

xabolcs mentioned this pull request Apr 24, 2024

Improve play.PropertiesConfLoader to ease conf/ directory usage #375

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Search for conf/application.conf and conf/routes from classpath (#198) #202

Search for conf/application.conf and conf/routes from classpath (#198) #202

xabolcs commented Jun 27, 2023 •

edited

Loading

xabolcs commented Jun 27, 2023 •

edited

Loading

xabolcs commented Jun 27, 2023

xabolcs Jun 27, 2023 •

edited

Loading

asolntsev Aug 9, 2023 •

edited

Loading

xabolcs Jun 28, 2023 •

edited

Loading

xabolcs Jun 28, 2023

asolntsev Aug 9, 2023

xabolcs Aug 9, 2023 •

edited

Loading

asolntsev Sep 13, 2023

xabolcs Sep 13, 2023

asolntsev Sep 14, 2023

xabolcs Sep 14, 2023 •

edited

Loading

asolntsev Sep 29, 2023

xabolcs Sep 30, 2023

xabolcs commented Sep 30, 2023 •

edited

Loading

asolntsev commented Nov 20, 2023

xabolcs commented Nov 20, 2023

xabolcs commented Nov 20, 2023 •

edited

Loading

xabolcs commented Nov 21, 2023

Search for conf/application.conf and conf/routes from classpath (#198) #202

Search for conf/application.conf and conf/routes from classpath (#198) #202

Conversation

xabolcs commented Jun 27, 2023 • edited Loading

xabolcs commented Jun 27, 2023 • edited Loading

xabolcs commented Jun 27, 2023

xabolcs Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

asolntsev Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

xabolcs Jun 28, 2023 • edited Loading

Choose a reason for hiding this comment

xabolcs Jun 28, 2023

Choose a reason for hiding this comment

asolntsev Aug 9, 2023

Choose a reason for hiding this comment

xabolcs Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

asolntsev Sep 13, 2023

Choose a reason for hiding this comment

xabolcs Sep 13, 2023

Choose a reason for hiding this comment

asolntsev Sep 14, 2023

Choose a reason for hiding this comment

xabolcs Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

asolntsev Sep 29, 2023

Choose a reason for hiding this comment

xabolcs Sep 30, 2023

Choose a reason for hiding this comment

xabolcs commented Sep 30, 2023 • edited Loading

asolntsev commented Nov 20, 2023

xabolcs commented Nov 20, 2023

xabolcs commented Nov 20, 2023 • edited Loading

xabolcs commented Nov 21, 2023

xabolcs commented Jun 27, 2023 •

edited

Loading

xabolcs commented Jun 27, 2023 •

edited

Loading

xabolcs Jun 27, 2023 •

edited

Loading

asolntsev Aug 9, 2023 •

edited

Loading

xabolcs Jun 28, 2023 •

edited

Loading

xabolcs Aug 9, 2023 •

edited

Loading

xabolcs Sep 14, 2023 •

edited

Loading

xabolcs commented Sep 30, 2023 •

edited

Loading

xabolcs commented Nov 20, 2023 •

edited

Loading