Skip to content

Commit d00434e

Browse files
ehenomaehenoma
committed
Fix execution and clean up code/documentation
1 parent d01c872 commit d00434e

21 files changed

Lines changed: 292 additions & 74 deletions

File tree

‎deploy/kubernetes/namespace.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
apiVersion: v1
2+
kind: Namespace
3+
metadata:
4+
name: jsheets

‎deploy/kubernetes/server.yml‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
apiVersion: apps/v1
2+
kind: Deployment
3+
metadata:
4+
name: server
5+
namespace: jsheets
6+
labels:
7+
app: server
8+
spec:
9+
selector:
10+
matchLabels:
11+
app: server
12+
replicas: 3
13+
template:
14+
metadata:
15+
name: server
16+
labels:
17+
app: server
18+
spec:
19+
containers:
20+
- name: server
21+
image: ehenoma/jsheets:latest
22+
ports:
23+
- containerPort: 8080
24+
hostPort: 8080
25+
env:
26+
- name: JSHEETS_SERVER_PORT
27+
value: "8080"
28+
---
29+
apiVersion: v1
30+
kind: Service
31+
metadata:
32+
name: server-service
33+
namespace: jsheets
34+
spec:
35+
selector:
36+
app: server
37+
clusterIP: None
38+
ports:
39+
- name: http
40+
port: 8080
41+
targetPort: 8080
42+
---
43+
apiVersion: networking.k8s.io/v1
44+
kind: Ingress
45+
metadata:
46+
name: server-ingress
47+
namespace: jsheets
48+
spec:
49+
rules:
50+
- http:
51+
paths:
52+
- path: "/"
53+
pathType: Prefix
54+
backend:
55+
service:
56+
name: server-service

‎deploy/minimal/README.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# Minimal Deployment
2+
3+
Contains the server itself and a document store.
4+
The server is responsible for evaluating snippets, which is not
5+
ideal for public/highly frequented websites.

docker-compose.yml renamed to deploy/minimal/docker-compose.yml

File renamed without changes.

‎evaluation/src/main/java/jsheets/evaluation/failure/FailedEvaluation.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package jsheets.evaluation.failure;
2+
3+
import jsheets.EvaluationError;
4+
5+
import java.util.Locale;
6+
import java.util.Optional;
7+
import java.util.stream.Stream;
8+
9+
public interface FailedEvaluation {
10+
Stream<EvaluationError> describe(Locale locale);
11+
12+
static Optional<FailedEvaluation> capture(Throwable failure) {
13+
while (failure != null) {
14+
if (failure instanceof FailedEvaluation) {
15+
return Optional.of((FailedEvaluation) failure);
16+
}
17+
failure = failure.getCause();
18+
}
19+
return Optional.empty();
20+
}
21+
}

‎evaluation/src/main/java/jsheets/evaluation/sandbox/SandboxLoader.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,11 @@ private SandboxLoader(Collection<Rule> rules) {
4747
this.rules = rules;
4848
}
4949

50-
public void install() {
51-
Thread.currentThread().setContextClassLoader(loader);
50+
public Runnable install() {
51+
var thread = Thread.currentThread();
52+
var previousLoader = thread.getContextClassLoader();
53+
thread.setContextClassLoader(loader);
54+
return () -> thread.setContextClassLoader(previousLoader);
5255
}
5356

5457
@Override

‎evaluation/src/main/java/jsheets/evaluation/sandbox/access/MethodSignature.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,12 @@ public String format() {
9797
);
9898
}
9999

100+
private static final String constructorName = "<init>";
101+
102+
public boolean isConstructor() {
103+
return methodName.equals(constructorName);
104+
}
105+
100106
public String formatNameAndParameters() {
101107
return "%s(%s)".formatted(methodName, String.join(", ", parameterTypes));
102108
}

‎evaluation/src/main/java/jsheets/evaluation/sandbox/validation/Analysis.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package jsheets.evaluation.sandbox.validation;
22

3+
import jsheets.EvaluationError;
4+
import jsheets.evaluation.failure.FailedEvaluation;
5+
36
import java.util.Collection;
7+
import java.util.Locale;
48
import java.util.Set;
59
import java.util.concurrent.ConcurrentLinkedQueue;
610
import java.util.stream.Collectors;
@@ -11,7 +15,7 @@ public static Analysis create() {
1115
return new Analysis();
1216
}
1317

14-
public interface Violation {}
18+
public interface Violation extends FailedEvaluation {}
1519

1620
private final Collection<Violation> violations =
1721
new ConcurrentLinkedQueue<>();
@@ -28,7 +32,7 @@ public void reportViolations() {
2832
}
2933
}
3034

31-
static final class FailedAnalysis extends RuntimeException {
35+
static final class FailedAnalysis extends RuntimeException implements FailedEvaluation {
3236
private final Collection<Violation> violations;
3337

3438
private FailedAnalysis(Collection<Violation> violations) {
@@ -43,6 +47,12 @@ private FailedAnalysis(Collection<Violation> violations) {
4347
public Stream<Violation> violations() {
4448
return violations.stream();
4549
}
50+
51+
@Override
52+
public Stream<EvaluationError> describe(Locale locale) {
53+
return violations.stream()
54+
.flatMap(violation -> violation.describe(locale));
55+
}
4656
}
4757

4858
public static Stream<Violation> captureViolations(Throwable failure) {

‎evaluation/src/main/java/jsheets/evaluation/sandbox/validation/ForbiddenMemberFilter.java‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
package jsheets.evaluation.sandbox.validation;
22

33
import java.util.Arrays;
4+
import java.util.Locale;
45
import java.util.Objects;
6+
import java.util.stream.Stream;
57

8+
import jsheets.EvaluationError;
69
import jsheets.evaluation.sandbox.access.AccessGraph;
710
import jsheets.evaluation.sandbox.access.AccessKey;
811
import jsheets.evaluation.sandbox.access.MethodSignature;
@@ -21,10 +24,35 @@ private ForbiddenMemberFilter(AccessGraph accessGraph) {
2124
this.accessGraph = accessGraph;
2225
}
2326

24-
public record ForbiddenMethod(MethodSignature method) implements Analysis.Violation {}
27+
public record ForbiddenMethod(MethodSignature method) implements Analysis.Violation {
28+
@Override
29+
public Stream<EvaluationError> describe(Locale locale) {
30+
var message = method.isConstructor()
31+
? formatForConstructor(locale)
32+
: formatForMethod(locale);
33+
return Stream.of(
34+
EvaluationError.newBuilder()
35+
.setKind("sandbox")
36+
.setMessage(message)
37+
.build()
38+
);
39+
}
40+
41+
private String formatForConstructor(Locale locale) {
42+
return "The class %s is not allowed".formatted(method.className());
43+
}
44+
45+
private String formatForMethod(Locale locale) {
46+
return "The method %s in %s is not allowed"
47+
.formatted(method.methodName(), method.className());
48+
}
49+
}
2550

2651
@Override
2752
public void visitCall(Analysis analysis, MethodCall call) {
53+
if (isClassExcluded(call.owner())) {
54+
return;
55+
}
2856
var signature = createSignatureOfCall(call);
2957
if (!accessGraph.isMethodPermitted(signature)) {
3058
analysis.report(new ForbiddenMethod(signature));
@@ -43,13 +71,32 @@ private MethodSignature createSignatureOfCall(MethodCall call) {
4371
).build();
4472
}
4573

46-
public record ForbiddenField(String owner, String field) implements Analysis.Violation {}
74+
public record ForbiddenField(String owner, String field) implements Analysis.Violation {
75+
@Override
76+
public Stream<EvaluationError> describe(Locale locale) {
77+
return Stream.of(
78+
EvaluationError.newBuilder()
79+
.setKind("sandbox")
80+
.setMessage("The field %s in %s is not allowed".formatted(field, owner))
81+
.build()
82+
);
83+
}
84+
}
4785

4886
@Override
4987
public void visitFieldAccess(Analysis analysis, FieldAccess access) {
88+
if (isClassExcluded(access.owner())) {
89+
return;
90+
}
5091
var key = "%s.%s".formatted(access.owner(), access.field());
5192
if (!accessGraph.isPermitted(AccessKey.dotSeparated(key))) {
5293
analysis.report(new ForbiddenField(access.owner(), access.field()));
5394
}
5495
}
96+
97+
private static final String generatedSnippetClassPrefix = "REPL.$JShell$";
98+
99+
private boolean isClassExcluded(String className) {
100+
return className.startsWith(generatedSnippetClassPrefix);
101+
}
55102
}

‎evaluation/src/main/java/jsheets/evaluation/shell/ShellEvaluation.java‎

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@
2121
import jsheets.SnippetSources;
2222
import jsheets.StartEvaluationRequest;
2323
import jsheets.evaluation.Evaluation;
24+
import jsheets.evaluation.failure.FailedEvaluation;
2425
import jsheets.evaluation.shell.environment.ExecutionEnvironment;
2526
import jsheets.evaluation.shell.execution.ExecutionMethod;
26-
import jsheets.source.SharedSources;
2727

2828
final class ShellEvaluation implements Evaluation {
2929
private enum Stage { Initial, Starting, Evaluating, Terminated }
@@ -33,14 +33,12 @@ private enum Stage { Initial, Starting, Evaluating, Terminated }
3333
private volatile JShell shell;
3434
private volatile ExecutionMethod executionMethod;
3535
private final Evaluation.Listener listener;
36-
private final SharedSources sources;
3736
private final ExecutionEnvironment environment;
3837
private final ExecutionMethod.Factory executionMethodFactory;
3938
private final MessageOutput messageOutput;
4039
private final AtomicReference<Stage> stage = new AtomicReference<>(Stage.Initial);
4140

4241
ShellEvaluation(
43-
Clock clock,
4442
ExecutionEnvironment environment,
4543
ExecutionMethod.Factory executionMethodFactory,
4644
Evaluation.Listener listener,
@@ -50,7 +48,6 @@ private enum Stage { Initial, Starting, Evaluating, Terminated }
5048
this.messageOutput = messageOutput;
5149
this.executionMethodFactory = executionMethodFactory;
5250
this.environment = environment;
53-
sources = SharedSources.createEmpty(clock);
5451
}
5552

5653
public void start(StartEvaluationRequest request) {
@@ -82,19 +79,49 @@ private void evaluateComponent(
8279
reportEvent(component.getId(), snippet, response);
8380
}
8481
messageOutput.flush();
85-
} catch (Exception failedEvaluation) {
86-
log.atWarning()
87-
.atMostEvery(5, TimeUnit.SECONDS)
88-
.withCause(failedEvaluation).log("shell evaluation failed");
89-
response.addError(
90-
EvaluationError.newBuilder()
91-
.setComponentId(component.getId())
92-
.setKind("internal")
93-
.build()
94-
);
82+
} catch (Throwable failedEvaluation) {
83+
reportError(component, response, failedEvaluation);
9584
}
9685
}
9786

87+
private void reportError(
88+
SnippetSources.CodeComponent component,
89+
EvaluateResponse.Builder response,
90+
Throwable failure
91+
) {
92+
FailedEvaluation.capture(failure).ifPresentOrElse(
93+
value -> reportFailedEvaluation(component, response, value),
94+
() -> reportInternalFailure(component, response, failure)
95+
);
96+
}
97+
98+
private void reportFailedEvaluation(
99+
SnippetSources.CodeComponent component,
100+
EvaluateResponse.Builder response,
101+
FailedEvaluation failedEvaluation
102+
) {
103+
failedEvaluation.describe(Locale.ENGLISH)
104+
.distinct()
105+
.map(error -> error.toBuilder().setComponentId(component.getId()))
106+
.forEach(response::addError);
107+
}
108+
109+
private void reportInternalFailure(
110+
SnippetSources.CodeComponent component,
111+
EvaluateResponse.Builder response,
112+
Throwable failure
113+
) {
114+
log.atWarning()
115+
.atMostEvery(5, TimeUnit.SECONDS)
116+
.withCause(failure).log("shell evaluation failed");
117+
response.addError(
118+
EvaluationError.newBuilder()
119+
.setComponentId(component.getId())
120+
.setKind("internal")
121+
.build()
122+
);
123+
}
124+
98125
private void reportEvent(
99126
String componentId,
100127
SnippetEvent event,
@@ -108,6 +135,7 @@ private void reportEvent(
108135
response.addResult(
109136
EvaluationResult.newBuilder()
110137
.setComponentId(componentId)
138+
.setKind(EvaluationResult.Kind.INFO)
111139
.setOutput(event.value())
112140
.build()
113141
);
@@ -159,7 +187,6 @@ private void cleanUp() {
159187
public String toString() {
160188
return MoreObjects.toStringHelper(this)
161189
.add("shell", shell)
162-
.add("sources", sources)
163190
.add("environment", environment)
164191
.add("stage", stage)
165192
.toString();

0 commit comments

Comments
 (0)