Skip to content

Commit dc7de48

Browse files
ricardozaniniricardozanini
authored
bug: HttpExecutor was swallowing server URL path; OpenAPIExecutor caching same server URL and operationId (#993)
* bug: OpenAPIExecutor was swallowing server URL path; Caching same server + op; Signed-off-by: Ricardo Zanini <[email protected]> * Explicit test scope Signed-off-by: Ricardo Zanini <[email protected]> * Moving buildTargetUrl to WorkflowUtils Signed-off-by: Ricardo Zanini <[email protected]> * Fix policyBuilder Signed-off-by: Ricardo Zanini <[email protected]> * Adjust based on javi's review Signed-off-by: Ricardo Zanini <[email protected]> --------- Signed-off-by: Ricardo Zanini <[email protected]>
1 parent c8100c0 commit dc7de48

File tree

11 files changed

+295
-99
lines changed

11 files changed

+295
-99
lines changed

‎experimental/fluent/func/src/test/java/io/serverlessworkflow/fluent/func/FuncDSLTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static io.serverlessworkflow.fluent.func.dsl.FuncDSL.listen;
2525
import static io.serverlessworkflow.fluent.func.dsl.FuncDSL.toOne;
2626
import static io.serverlessworkflow.fluent.spec.dsl.DSL.auth;
27+
import static io.serverlessworkflow.fluent.spec.dsl.DSL.use;
2728
import static org.junit.jupiter.api.Assertions.assertEquals;
2829
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
2930
import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -263,7 +264,7 @@ void get_convenience_creates_http_get() {
263264
void get_named_with_authentication_uses_auth_policy() {
264265
Workflow wf =
265266
FuncWorkflowBuilder.workflow("http-get-auth")
266-
.tasks(get("fetchUsers", "http://service/api/users", auth("user-service-auth")))
267+
.tasks(get("fetchUsers", "http://service/api/users", use("user-service-auth")))
267268
.build();
268269

269270
List<TaskItem> items = wf.getDo();

‎fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/ReferenceableAuthenticationPolicyBuilder.java‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,51 +21,44 @@
2121
import java.util.function.Consumer;
2222

2323
public class ReferenceableAuthenticationPolicyBuilder {
24-
final AuthenticationPolicyUnion authenticationPolicy;
25-
final AuthenticationPolicyReference authenticationPolicyReference;
24+
private AuthenticationPolicyUnion authenticationPolicy;
25+
private AuthenticationPolicyReference authenticationPolicyReference;
2626

27-
public ReferenceableAuthenticationPolicyBuilder() {
28-
this.authenticationPolicy = new AuthenticationPolicyUnion();
29-
this.authenticationPolicyReference = new AuthenticationPolicyReference();
30-
}
27+
public ReferenceableAuthenticationPolicyBuilder() {}
3128

3229
public ReferenceableAuthenticationPolicyBuilder basic(
3330
Consumer<BasicAuthenticationPolicyBuilder> basicConsumer) {
34-
final BasicAuthenticationPolicyBuilder basicAuthenticationPolicyBuilder =
35-
new BasicAuthenticationPolicyBuilder();
36-
basicConsumer.accept(basicAuthenticationPolicyBuilder);
37-
this.authenticationPolicy.setBasicAuthenticationPolicy(
38-
basicAuthenticationPolicyBuilder.build());
31+
final BasicAuthenticationPolicyBuilder builder = new BasicAuthenticationPolicyBuilder();
32+
basicConsumer.accept(builder);
33+
this.authenticationPolicy =
34+
new AuthenticationPolicyUnion().withBasicAuthenticationPolicy(builder.build());
3935
return this;
4036
}
4137

4238
public ReferenceableAuthenticationPolicyBuilder bearer(
4339
Consumer<BearerAuthenticationPolicyBuilder> bearerConsumer) {
44-
final BearerAuthenticationPolicyBuilder bearerAuthenticationPolicyBuilder =
45-
new BearerAuthenticationPolicyBuilder();
46-
bearerConsumer.accept(bearerAuthenticationPolicyBuilder);
47-
this.authenticationPolicy.setBearerAuthenticationPolicy(
48-
bearerAuthenticationPolicyBuilder.build());
40+
final BearerAuthenticationPolicyBuilder builder = new BearerAuthenticationPolicyBuilder();
41+
bearerConsumer.accept(builder);
42+
this.authenticationPolicy =
43+
new AuthenticationPolicyUnion().withBearerAuthenticationPolicy(builder.build());
4944
return this;
5045
}
5146

5247
public ReferenceableAuthenticationPolicyBuilder digest(
5348
Consumer<DigestAuthenticationPolicyBuilder> digestConsumer) {
54-
final DigestAuthenticationPolicyBuilder digestAuthenticationPolicyBuilder =
55-
new DigestAuthenticationPolicyBuilder();
56-
digestConsumer.accept(digestAuthenticationPolicyBuilder);
57-
this.authenticationPolicy.setDigestAuthenticationPolicy(
58-
digestAuthenticationPolicyBuilder.build());
49+
final DigestAuthenticationPolicyBuilder builder = new DigestAuthenticationPolicyBuilder();
50+
digestConsumer.accept(builder);
51+
this.authenticationPolicy =
52+
new AuthenticationPolicyUnion().withDigestAuthenticationPolicy(builder.build());
5953
return this;
6054
}
6155

6256
public ReferenceableAuthenticationPolicyBuilder oauth2(
6357
Consumer<OAuth2AuthenticationPolicyBuilder> oauth2Consumer) {
64-
final OAuth2AuthenticationPolicyBuilder oauth2AuthenticationPolicyBuilder =
65-
new OAuth2AuthenticationPolicyBuilder();
66-
oauth2Consumer.accept(oauth2AuthenticationPolicyBuilder);
67-
this.authenticationPolicy.setOAuth2AuthenticationPolicy(
68-
oauth2AuthenticationPolicyBuilder.build());
58+
final OAuth2AuthenticationPolicyBuilder builder = new OAuth2AuthenticationPolicyBuilder();
59+
oauth2Consumer.accept(builder);
60+
this.authenticationPolicy =
61+
new AuthenticationPolicyUnion().withOAuth2AuthenticationPolicy(builder.build());
6962
return this;
7063
}
7164

@@ -74,12 +67,13 @@ public ReferenceableAuthenticationPolicyBuilder openIDConnect(
7467
final OpenIdConnectAuthenticationPolicyBuilder builder =
7568
new OpenIdConnectAuthenticationPolicyBuilder();
7669
openIdConnectConsumer.accept(builder);
77-
this.authenticationPolicy.setOpenIdConnectAuthenticationPolicy(builder.build());
70+
this.authenticationPolicy =
71+
new AuthenticationPolicyUnion().withOpenIdConnectAuthenticationPolicy(builder.build());
7872
return this;
7973
}
8074

8175
public ReferenceableAuthenticationPolicyBuilder use(String use) {
82-
this.authenticationPolicyReference.setUse(use);
76+
this.authenticationPolicyReference = new AuthenticationPolicyReference().withUse(use);
8377
return this;
8478
}
8579

‎fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/dsl/DSL.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ public static Consumer<TryTaskBuilder.CatchErrorsBuilder> errorFilter(
319319
* @param authName the name of a reusable authentication policy
320320
* @return an {@link AuthenticationConfigurer} that sets {@code use(authName)}
321321
*/
322-
public static AuthenticationConfigurer auth(String authName) {
322+
public static AuthenticationConfigurer use(String authName) {
323323
return a -> a.use(authName);
324324
}
325325

‎fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/CallHttpTaskFluent.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import io.serverlessworkflow.api.types.CallHTTP;
2020
import io.serverlessworkflow.api.types.Endpoint;
2121
import io.serverlessworkflow.api.types.EndpointConfiguration;
22+
import io.serverlessworkflow.api.types.EndpointUri;
2223
import io.serverlessworkflow.api.types.HTTPArguments;
2324
import io.serverlessworkflow.api.types.HTTPHeaders;
2425
import io.serverlessworkflow.api.types.HTTPQuery;
@@ -59,14 +60,17 @@ default SELF endpoint(URI endpoint) {
5960
default SELF endpoint(URI endpoint, Consumer<ReferenceableAuthenticationPolicyBuilder> auth) {
6061
final ReferenceableAuthenticationPolicyBuilder policy =
6162
new ReferenceableAuthenticationPolicyBuilder();
63+
final UriTemplate uriTemplate = new UriTemplate().withLiteralUri(endpoint);
6264
auth.accept(policy);
6365
((CallHTTP) this.self().getTask())
6466
.getWith()
6567
.setEndpoint(
6668
new Endpoint()
6769
.withEndpointConfiguration(
68-
new EndpointConfiguration().withAuthentication(policy.build()))
69-
.withUriTemplate(new UriTemplate().withLiteralUri(endpoint)));
70+
new EndpointConfiguration()
71+
.withUri(new EndpointUri().withLiteralEndpointURI(uriTemplate))
72+
.withAuthentication(policy.build()))
73+
.withUriTemplate(uriTemplate));
7074
return self();
7175
}
7276

‎impl/core/pom.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,10 @@
2828
<groupId>com.cronutils</groupId>
2929
<artifactId>cron-utils</artifactId>
3030
</dependency>
31+
<dependency>
32+
<groupId>org.junit.jupiter</groupId>
33+
<artifactId>junit-jupiter-engine</artifactId>
34+
<scope>test</scope>
35+
</dependency>
3136
</dependencies>
3237
</project>

‎impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowUtils.java‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import io.serverlessworkflow.impl.schema.SchemaValidator;
3131
import io.serverlessworkflow.impl.schema.SchemaValidatorFactory;
3232
import java.net.URI;
33+
import java.net.URISyntaxException;
3334
import java.time.Duration;
3435
import java.util.Map;
3536
import java.util.Optional;
@@ -221,11 +222,42 @@ public static final String checkSecret(
221222
.orElseThrow(() -> new IllegalStateException("Secret " + secretName + " does not exist"));
222223
}
223224

224-
public static URI concatURI(URI uri, String pathToAppend) {
225-
return uri.getPath().endsWith("/")
226-
? uri.resolve(pathToAppend)
227-
: URI.create(
228-
uri.toString() + (pathToAppend.startsWith("/") ? pathToAppend : "/" + pathToAppend));
225+
public static URI concatURI(URI base, String pathToAppend) {
226+
if (!isValid(pathToAppend)) {
227+
return base;
228+
}
229+
230+
final URI child = URI.create(pathToAppend);
231+
if (child.isAbsolute()) {
232+
return child;
233+
}
234+
235+
String basePath = base.getPath();
236+
if (!isValid(basePath)) {
237+
basePath = "/";
238+
} else if (!basePath.endsWith("/")) {
239+
basePath = basePath + "/";
240+
}
241+
242+
String relPath = child.getPath();
243+
if (relPath == null) {
244+
relPath = "";
245+
}
246+
while (relPath.startsWith("/")) {
247+
relPath = relPath.substring(1);
248+
}
249+
250+
String finalPath = basePath + relPath;
251+
252+
String query = child.getQuery();
253+
String fragment = child.getFragment();
254+
255+
try {
256+
return new URI(base.getScheme(), base.getAuthority(), finalPath, query, fragment);
257+
} catch (URISyntaxException e) {
258+
throw new IllegalArgumentException(
259+
"Failed to build combined URI from base=" + base + " and path=" + pathToAppend, e);
260+
}
229261
}
230262

231263
public static WorkflowValueResolver<URI> getURISupplier(

‎impl/core/src/test/java/io/serverlessworkflow/impl/WorkflowUtilsTest.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Copyright 2020-Present The Serverless Workflow Specification Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.serverlessworkflow.impl;
17+
18+
import static org.junit.jupiter.api.Assertions.assertEquals;
19+
20+
import java.net.URI;
21+
import org.junit.jupiter.api.Test;
22+
23+
public class WorkflowUtilsTest {
24+
@Test
25+
void openApiServerWithTrailingSlashAndRootPath() {
26+
URI base = URI.create("https://petstore3.swagger.io/api/v3/");
27+
URI path = URI.create("/pet/findByStatus");
28+
29+
URI result = WorkflowUtils.concatURI(base, path.toString());
30+
31+
assertEquals("https://petstore3.swagger.io/api/v3/pet/findByStatus", result.toString());
32+
}
33+
34+
@Test
35+
void openApiServerWithoutTrailingSlashAndRootPath() {
36+
URI base = URI.create("https://petstore3.swagger.io/api/v3");
37+
URI path = URI.create("/pet/findByStatus");
38+
39+
URI result = WorkflowUtils.concatURI(base, path.toString());
40+
41+
assertEquals("https://petstore3.swagger.io/api/v3/pet/findByStatus", result.toString());
42+
}
43+
44+
@Test
45+
void baseWithSlashAndRelativePath() {
46+
URI base = URI.create("https://example.com/api/v1/");
47+
URI path = URI.create("pets");
48+
49+
URI result = WorkflowUtils.concatURI(base, path.toString());
50+
51+
assertEquals("https://example.com/api/v1/pets", result.toString());
52+
}
53+
54+
@Test
55+
void baseWithoutPathAndRootPath() {
56+
URI base = URI.create("https://example.com");
57+
URI path = URI.create("/pets");
58+
59+
URI result = WorkflowUtils.concatURI(base, path.toString());
60+
61+
assertEquals("https://example.com/pets", result.toString());
62+
}
63+
64+
@Test
65+
void absolutePathOverridesBase() {
66+
URI base = URI.create("https://example.com/api");
67+
URI path = URI.create("https://other.example.com/foo");
68+
69+
URI result = WorkflowUtils.concatURI(base, path.toString());
70+
71+
assertEquals("https://other.example.com/foo", result.toString());
72+
}
73+
74+
@Test
75+
void queryAndFragmentAreTakenFromPath() {
76+
URI base = URI.create("https://example.com/api/v1");
77+
URI path = URI.create("/pets?status=available#top");
78+
79+
URI result = WorkflowUtils.concatURI(base, path.toString());
80+
81+
assertEquals("https://example.com/api/v1/pets?status=available#top", result.toString());
82+
}
83+
}

‎impl/http/pom.xml‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
1+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
2+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
23
<modelVersion>4.0.0</modelVersion>
34
<parent>
45
<groupId>io.serverlessworkflow</groupId>
@@ -8,9 +9,9 @@
89
<artifactId>serverlessworkflow-impl-http</artifactId>
910
<name>Serverless Workflow :: Impl :: HTTP</name>
1011
<dependencies>
11-
<dependency>
12-
<groupId>io.serverlessworkflow</groupId>
13-
<artifactId>serverlessworkflow-impl-template-resolver</artifactId>
14-
</dependency>
12+
<dependency>
13+
<groupId>io.serverlessworkflow</groupId>
14+
<artifactId>serverlessworkflow-impl-template-resolver</artifactId>
15+
</dependency>
1516
</dependencies>
1617
</project>

‎impl/http/src/main/java/io/serverlessworkflow/impl/executors/http/HttpExecutor.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,8 @@ private static WorkflowValueResolver<WebTarget> getTargetSupplier(
256256
WorkflowValueResolver<URI> uriSupplier, WorkflowValueResolver<URI> pathSupplier) {
257257
return (w, t, n) ->
258258
HttpClientResolver.client(w, t)
259-
.target(uriSupplier.apply(w, t, n).resolve(pathSupplier.apply(w, t, n)));
259+
.target(
260+
WorkflowUtils.concatURI(
261+
uriSupplier.apply(w, t, n), pathSupplier.apply(w, t, n).toString()));
260262
}
261263
}

0 commit comments

Comments
 (0)