Is it possible to get rid of composite.readURL / composite.readLocation entirely?

Is it possible to remove this completely?

Can this be removed now?

Can we remove this one too?

Let's remove it.

Can we remove readPath and use Files.readString(path) directly instead?

Is this more a question of optimization than specific schemes?

There is different behaviour here.

In case of the file filesystem we want to watch files, but actually we can only watch directories. But our file can be a symlink or the directory can be a symlink or its parent etc... So Barnabas created a very sophisticated logic to cover all those cases, which means in a generic case we have to watch more than one object (directory) so we can make sure we are watching the changes of our file.

In case of http filesystem we just watch a URL and that's it. So it's a 1:1 mapping hence just one line of code here.

Can JwtGuardHandler use EngineContext.readPath(...) instead of guard.readPath?

The idea of using a FileSystem abstraction is to move away from implementation specific knowledge of the filesystem as much as is practical in the engine.

The FileSystem.newWatchService() method is specified as an optional method, throwing UnsupportedOperationException on implementations not supporting it.

So we should be tolerant of that both now for jar scheme (used during tests) and later for other filesystems that also may not support newWatchService(), without needing to mention jar explicitly.

        private static WatchService newWatchService(
            FileSystem fileSystem)
        {
            WatchService watcher = null;

            try
            {
                watcher = fileSystem.newWatchService();
            }
            catch (UnsupportedOperationException ex)
            {
                // no watcher
            }
            catch (Exception ex)
            {
                rethrowUnchecked(ex);
            }

            return watcher;
        }

Let's remove readLocation everywhere, and use Files.readString(path) instead at the call sites.

We should no longer need BindingConfig.readLocation directly given that EngineWorker.resolvePath(location) is available to all handlers, so BindingConfig.readLocation can be removed as well.

This method implementation should move to EngineWorker instead of passing a lambda from Engine.

Note: EngineWorker already has EngineConfig and can call config.configPath() in EngineWorker constructor instead of passing as an explicit constructor parameter.

None of these need to be passed to EngineManager constructor.

Only configPath is needed and can be obtained from EngineConfiguration (already passed).

I think we don't want to default to empty string here.

After moving Files.readString(path) to each call site, handling an IOException will let the caller decide the best remedy instead of assuming empty string is always appropriate.

Move resolvePath to EngineWorker implementation instead.

Looks like configPath is no longer needed as Engine field after resovlePath(...) method is moved to EngineWorker implementation directly.

This configPath() method looks good.

Let's keep the ENGINE_CONFIG_URL constant in place for now, to support zilla.properties defining zilla.engine.config.url property, but remove all calls to EngineConfiguration.configURL() from the code, and replace with corresponding EngineConfiguration.configPath() method calls instead.

Let's remove it.

This approach assumes global knowledge of all possible implementations and assumes their behavior, such that a catalog or vault other than filesystem cannot contribute resources to be watched.

We need a more general strategy instead.

Please include all resources from any binding, guard, vault, catalog or telemetry exporter.

Note: instead of calling a non-static method from the constructor that relies on constructor field assignment, please make this method static and pass all required state as parameters, such as bindings, guards, vaults, catalogs, and telemetry.