Skip to content

Resolve JS module on first use if empty #148

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.

Sign up for GitHub

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
jakub-id wants to merge 3 commits into from
Closed

Resolve JS module on first use if empty #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
945cdbd
Resolve JS module on first use if empty
jakub-id Jan 22, 2026
80cece7
Fix NPE
jakub-id Jan 22, 2026
4737b6c
Move update to Cache impl
jakub-id Jan 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/src/main/java/com/indexdata/reservoir/module/Module.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

public interface Module {

Future<Void> initialize(Vertx vertx, CodeModuleEntity entity);
Future<CodeModuleEntity> initialize(Vertx vertx, CodeModuleEntity entity);

Future<JsonObject> execute(String symbol, JsonObject input);

Expand Down
3 changes: 3 additions & 0 deletions server/src/main/java/com/indexdata/reservoir/module/ModuleCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.indexdata.reservoir.module;

import com.indexdata.reservoir.module.impl.ModuleCacheImpl;
import com.indexdata.reservoir.server.Storage;
import com.indexdata.reservoir.server.entity.CodeModuleEntity;
import io.vertx.core.Future;
import io.vertx.core.Vertx;
Expand All @@ -11,6 +12,8 @@ static ModuleCache getInstance() {
return ModuleCacheImpl.getInstance();
}

public Future<Module> lookup(Vertx vertx, Storage storage, CodeModuleEntity entity);

public Future<Module> lookup(Vertx vertx, String tenant, CodeModuleEntity entity);

void purge(String tenant, String id);
Expand Down
28 changes: 24 additions & 4 deletions server/src/main/java/com/indexdata/reservoir/module/impl/ModuleCacheImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

import com.indexdata.reservoir.module.Module;
import com.indexdata.reservoir.module.ModuleCache;
import com.indexdata.reservoir.server.Storage;
import com.indexdata.reservoir.server.entity.CodeModuleEntity;
import io.vertx.core.Future;
import io.vertx.core.Vertx;
import io.vertx.core.http.HttpMethod;
import java.util.HashMap;
import java.util.Map;

Expand Down Expand Up @@ -46,12 +48,22 @@ private Module createInstance(String type) {
}

@Override
public Future<Module> lookup(Vertx vertx, String tenantId, CodeModuleEntity entity) {
public Future<Module> lookup(Vertx vertx, String tenant, CodeModuleEntity entity) {
return lookup(vertx, tenant, null, entity);
}

@Override
public Future<Module> lookup(Vertx vertx, Storage storage, CodeModuleEntity entity) {
return lookup(vertx, storage.getTenant(), storage, entity);
}

private Future<Module> lookup(Vertx vertx, String tenant, Storage storage,
CodeModuleEntity entity) {
String moduleId = entity.getId();
if (moduleId == null) {
return Future.failedFuture("module config must include 'id'");
}
String cacheKey = tenantId + ":" + moduleId;
String cacheKey = tenant + ":" + moduleId;
CacheEntry entry = entries.get(cacheKey);
if (entry != null) {
if (entry.entity.equals(entity)) {
Expand All @@ -61,8 +73,16 @@ public Future<Module> lookup(Vertx vertx, String tenantId, CodeModuleEntity enti
entries.remove(cacheKey);
}
Module module = createInstance(entity.getType());
return module.initialize(vertx, entity).map(x -> {
CacheEntry e = new CacheEntry(module, entity);
return module.initialize(vertx, entity)
.compose(newEntity -> {
if (storage != null && newEntity != entity) {
return storage.updateCodeModuleEntity(newEntity).map(newEntity);
} else {
return Future.succeededFuture(newEntity);
}
})
.map(newEntity -> {
CacheEntry e = new CacheEntry(module, newEntity);
entries.put(cacheKey, e);
return module;
});
Expand Down
46 changes: 30 additions & 16 deletions server/src/main/java/com/indexdata/reservoir/module/impl/ModuleJavaScript.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package com.indexdata.reservoir.module.impl;

import com.indexdata.reservoir.module.Module;
import com.indexdata.reservoir.server.Storage;
import com.indexdata.reservoir.server.entity.CodeModuleEntity;
import io.vertx.core.Future;
import io.vertx.core.Vertx;
import io.vertx.core.http.HttpMethod;
import io.vertx.core.json.JsonObject;
import java.util.Collection;
import java.util.HashSet;
Expand All @@ -22,7 +24,7 @@ public class ModuleJavaScript implements Module {
private Vertx vertx;

@Override
public Future<Void> initialize(Vertx vertx, CodeModuleEntity entity) {
public Future<CodeModuleEntity> initialize(Vertx vertx, CodeModuleEntity entity) {
id = entity.getId();
if (id == null || id.isEmpty()) {
return Future.failedFuture(
Expand All @@ -31,29 +33,41 @@ public Future<Void> initialize(Vertx vertx, CodeModuleEntity entity) {
this.vertx = vertx;
String url = entity.getUrl();
String script = entity.getScript();
if (script == null || script.isEmpty()) {
var hasUrl = url != null && !url.isEmpty();
var hasScript = script != null && !script.isEmpty();
if (!hasUrl && !hasScript) {
return Future.failedFuture(
new IllegalArgumentException("Module config must include 'url' or 'script'"));
}
if (url != null && !url.isEmpty()) {
// url always points to an ES module
defaultFunctionName = entity.getFunction();
final boolean isModule = url.endsWith("mjs");
if (!isModule) {
return Future.failedFuture(new IllegalArgumentException(
"url must end with .mjs to designate ES module"));
if (hasUrl) {
//module was never resolved (migration)
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline comment "module was never resolved (migration)" explains the purpose but lacks detail about when this scenario occurs and what the expected behavior is. Consider expanding this comment to explain that this handles legacy modules that were stored with only URLs before automatic resolution was implemented, and that the code will fetch the script and persist it to the database for future use.

Suggested change
//module was never resolved (migration)
// Handle legacy modules that were stored with only a URL before automatic
// resolution was implemented. In this case the module has never been
// resolved: we fetch the script from the URL, persist the resolved script
// to the database for future use, and then initialize it as an ES module.

Copilot uses AI. Check for mistakes.
if (!hasScript) {
return new CodeModuleEntity.CodeModuleBuilder(entity.asJson())
.resolve(vertx)
.compose(this::initAsEsModule);
}
Context.Builder cb = Context.newBuilder("js")
.allowExperimentalOptions(true)
.option("js.esm-eval-returns-exports", "true");
context = cb.build();
String moduleName = url.substring(url.lastIndexOf("/") + 1);
module = context.eval(Source.newBuilder("js", script, moduleName).buildLiteral());
return this.initAsEsModule(entity);
} else {
context = Context.create("js");
function = context.eval("js", script);
return Future.succeededFuture(entity);
}
return Future.succeededFuture();
}

private Future<CodeModuleEntity> initAsEsModule(CodeModuleEntity entity) {
defaultFunctionName = entity.getFunction();
final boolean isModule = entity.getUrl().endsWith("mjs");
if (!isModule) {
return Future.failedFuture(new IllegalArgumentException(
"url must end with .mjs to designate ES module"));
}
Context.Builder cb = Context.newBuilder("js")
.allowExperimentalOptions(true)
.option("js.esm-eval-returns-exports", "true");
context = cb.build();
String moduleName = entity.getUrl().substring(entity.getUrl().lastIndexOf("/") + 1);
module = context.eval(Source.newBuilder("js", entity.getScript(), moduleName).buildLiteral());
Comment on lines +57 to +69
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initAsEsModule method assumes entity.getScript() and entity.getUrl() are non-null, but this is not validated. While the current call sites ensure this, adding defensive null checks would make the code more robust and prevent potential future bugs if this method is called from other contexts. Consider adding validation at the start of the method.

Copilot uses AI. Check for mistakes.
return Future.succeededFuture(entity);
}

private Value getFunction(String functionName) {
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/com/indexdata/reservoir/module/impl/ModuleJsonPath.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ public class ModuleJsonPath implements Module {
JsonPath jsonPath;

@Override
public Future<Void> initialize(Vertx vertx, CodeModuleEntity entity) {
public Future<CodeModuleEntity> initialize(Vertx vertx, CodeModuleEntity entity) {
String script = entity.getScript();
if (script == null) {
return Future.failedFuture("module config must include 'script'");
}
jsonPath = JsonPath.compile(script);
return Future.succeededFuture();
return Future.succeededFuture(entity);
}

public ModuleJsonPath() {
Expand Down
6 changes: 3 additions & 3 deletions server/src/main/java/com/indexdata/reservoir/server/ReservoirService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ Future<Void> reloadCodeModule(RoutingContext ctx) {
}
return new CodeModuleEntity.CodeModuleBuilder(res.asJson())
.resolve(ctx.vertx())
.compose(cm -> ModuleCache.getInstance().lookup(ctx.vertx(), tenant, cm)
.compose(cm -> ModuleCache.getInstance().lookup(ctx.vertx(), storage, cm)
.compose(module -> storage.updateCodeModuleEntity(cm))
.compose(x -> ctx.response().setStatusCode(204).end())
);
Expand Down Expand Up @@ -390,7 +390,7 @@ Future<Void> postCodeModule(RoutingContext ctx) {
JsonObject request = validatedRequest.getBody().getJsonObject();
return new CodeModuleEntity.CodeModuleBuilder(request)
.resolve(ctx.vertx())
.compose(cm -> ModuleCache.getInstance().lookup(ctx.vertx(), TenantUtil.tenant(ctx), cm)
.compose(cm -> ModuleCache.getInstance().lookup(ctx.vertx(), storage, cm)
.compose(module -> storage.insertCodeModuleEntity(cm))
.compose(res ->
HttpResponse.responseJson(ctx, 201)
Expand Down Expand Up @@ -421,7 +421,7 @@ Future<Void> putCodeModule(RoutingContext ctx) {
JsonObject request = validatedRequest.getBody().getJsonObject();
return new CodeModuleEntity.CodeModuleBuilder(request)
.resolve(ctx.vertx())
.compose(cm -> ModuleCache.getInstance().lookup(ctx.vertx(), TenantUtil.tenant(ctx), cm)
.compose(cm -> ModuleCache.getInstance().lookup(ctx.vertx(), storage, cm)
.compose(module -> storage.updateCodeModuleEntity(cm))
.compose(res -> {
if (Boolean.FALSE.equals(res)) {
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/com/indexdata/reservoir/server/Storage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ Future<IngestMatcher> createIngestMatcher(JsonObject matchKeyConfig, Vertx vertx
"Module '" + invocation.getModuleName()
+ "' does not exist for '" + invocation + "'");
}
return ModuleCache.getInstance().lookup(vertx, tenant, entity)
return ModuleCache.getInstance().lookup(vertx, this, entity)
.compose(module -> {
ingestMatcher.moduleExecutable = new ModuleExecutable(module, invocation);
return Future.succeededFuture(ingestMatcher);
Expand Down Expand Up @@ -1327,7 +1327,7 @@ Future<ModuleExecutable> getTransformer(RoutingContext ctx) {
return Future.failedFuture("Transformer module '"
+ invocation.getModuleName() + "' not found");
}
return ModuleCache.getInstance().lookup(ctx.vertx(), TenantUtil.tenant(ctx), entity)
return ModuleCache.getInstance().lookup(ctx.vertx(), this, entity)
.map(mod -> new ModuleExecutable(mod, invocation));
});
});
Expand Down