Skip to content

Commit 6c46ace

Browse files
authored
Minor refactor of missing params check (#997)
Signed-off-by: fjtirado <ftirados@redhat.com>
1 parent dc7de48 commit 6c46ace

File tree

3 files changed

+44
-86
lines changed

3 files changed

+44
-86
lines changed

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

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -223,11 +223,10 @@ public static final String checkSecret(
223223
}
224224

225225
public static URI concatURI(URI base, String pathToAppend) {
226-
if (!isValid(pathToAppend)) {
227-
return base;
228-
}
226+
return !isValid(pathToAppend) ? base : concatURI(base, URI.create(pathToAppend));
227+
}
229228

230-
final URI child = URI.create(pathToAppend);
229+
public static URI concatURI(URI base, URI child) {
231230
if (child.isAbsolute()) {
232231
return child;
233232
}
@@ -242,21 +241,21 @@ public static URI concatURI(URI base, String pathToAppend) {
242241
String relPath = child.getPath();
243242
if (relPath == null) {
244243
relPath = "";
244+
} else {
245+
while (relPath.startsWith("/")) {
246+
relPath = relPath.substring(1);
247+
}
245248
}
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-
255249
try {
256-
return new URI(base.getScheme(), base.getAuthority(), finalPath, query, fragment);
250+
return new URI(
251+
base.getScheme(),
252+
base.getAuthority(),
253+
basePath + relPath,
254+
child.getQuery(),
255+
child.getFragment());
257256
} catch (URISyntaxException e) {
258257
throw new IllegalArgumentException(
259-
"Failed to build combined URI from base=" + base + " and path=" + pathToAppend, e);
258+
"Failed to build combined URI from base=" + base + " and child=" + child, e);
260259
}
261260
}
262261

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,6 @@ private static WorkflowValueResolver<WebTarget> getTargetSupplier(
257257
return (w, t, n) ->
258258
HttpClientResolver.client(w, t)
259259
.target(
260-
WorkflowUtils.concatURI(
261-
uriSupplier.apply(w, t, n), pathSupplier.apply(w, t, n).toString()));
260+
WorkflowUtils.concatURI(uriSupplier.apply(w, t, n), pathSupplier.apply(w, t, n)));
262261
}
263262
}

impl/openapi/src/main/java/io/serverlessworkflow/impl/executors/openapi/OpenAPIExecutor.java

Lines changed: 29 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@
3232
import io.swagger.v3.oas.models.parameters.Parameter;
3333
import java.util.Collection;
3434
import java.util.HashMap;
35+
import java.util.HashSet;
3536
import java.util.Iterator;
3637
import java.util.Map;
38+
import java.util.Set;
3739
import java.util.concurrent.CompletableFuture;
3840

3941
public class OpenAPIExecutor implements CallableTask<CallOpenAPI> {
@@ -105,24 +107,31 @@ private void fillHttpBuilder(WorkflowApplication application, OperationDefinitio
105107
Map<String, Object> headersMap = new HashMap<>();
106108
Map<String, Object> queryMap = new HashMap<>();
107109
Map<String, Object> pathParameters = new HashMap<>();
110+
Set<String> missingParams = new HashSet<>();
108111

109112
Map<String, Object> bodyParameters = new HashMap<>(parameters);
110113
for (Parameter parameter : operation.getParameters()) {
111114
switch (parameter.getIn()) {
112115
case "header":
113-
param(parameter.getName(), bodyParameters, headersMap);
116+
param(parameter, bodyParameters, headersMap, missingParams);
114117
break;
115118
case "path":
116-
param(parameter.getName(), bodyParameters, pathParameters);
119+
param(parameter, bodyParameters, pathParameters, missingParams);
117120
break;
118121
case "query":
119-
param(parameter.getName(), bodyParameters, queryMap);
122+
param(parameter, bodyParameters, queryMap, missingParams);
120123
break;
121124
}
122125
}
123126

124-
validateRequiredParameters(operation, headersMap, queryMap, pathParameters);
125-
127+
if (!missingParams.isEmpty()) {
128+
throw new IllegalArgumentException(
129+
"Missing required OpenAPI parameters for operation '"
130+
+ (operation.getOperation().getOperationId() != null
131+
? operation.getOperation().getOperationId()
132+
: "<unknown>" + "': ")
133+
+ missingParams);
134+
}
126135
builder
127136
.withMethod(operation.getMethod())
128137
.withPath(new OperationPathResolver(operation.getPath(), application, pathParameters))
@@ -131,71 +140,22 @@ private void fillHttpBuilder(WorkflowApplication application, OperationDefinitio
131140
.withHeaders(headersMap);
132141
}
133142

134-
private void param(String name, Map<String, Object> origMap, Map<String, Object> collectorMap) {
135-
Object value = origMap.remove(name);
136-
if (value != null) {
137-
collectorMap.put(name, value);
138-
}
139-
}
140-
141-
private void validateRequiredParameters(
142-
OperationDefinition operation,
143-
Map<String, Object> headersMap,
144-
Map<String, Object> queryMap,
145-
Map<String, Object> pathParameters) {
146-
147-
StringBuilder missing = new StringBuilder();
148-
149-
for (Parameter parameter : operation.getParameters()) {
150-
if (!Boolean.TRUE.equals(parameter.getRequired())) {
151-
continue;
152-
}
153-
154-
String in = parameter.getIn();
155-
String name = parameter.getName();
156-
157-
Map<String, Object> targetMap =
158-
switch (in) {
159-
case "header" -> headersMap;
160-
case "path" -> pathParameters;
161-
case "query" -> queryMap;
162-
default -> null;
163-
};
164-
165-
if (targetMap == null) {
166-
// We don't currently handle other "in" locations here (e.g., cookie).
167-
// Treat as "not validated" instead of failing.
168-
continue;
169-
}
170-
171-
boolean present = targetMap.containsKey(name);
172-
173-
if (!present) {
174-
// Try to satisfy the requirement using the OpenAPI default, if any
175-
Schema<?> schema = parameter.getSchema();
176-
Object defaultValue = schema != null ? schema.getDefault() : null;
177-
178-
if (defaultValue != null) {
179-
targetMap.put(name, defaultValue);
180-
present = true;
181-
}
143+
private void param(
144+
Parameter parameter,
145+
Map<String, Object> origMap,
146+
Map<String, Object> collectorMap,
147+
Set<String> missingParams) {
148+
String name = parameter.getName();
149+
if (origMap.containsKey(name)) {
150+
collectorMap.put(parameter.getName(), origMap.remove(name));
151+
} else if (parameter.getRequired()) {
152+
Schema<?> schema = parameter.getSchema();
153+
Object defaultValue = schema != null ? schema.getDefault() : null;
154+
if (defaultValue != null) {
155+
collectorMap.put(name, defaultValue);
156+
} else {
157+
missingParams.add(name);
182158
}
183-
184-
if (!present) {
185-
if (!missing.isEmpty()) {
186-
missing.append(", ");
187-
}
188-
missing.append(in).append(" parameter '").append(name).append("'");
189-
}
190-
}
191-
192-
if (!missing.isEmpty()) {
193-
String operationId =
194-
operation.getOperation().getOperationId() != null
195-
? operation.getOperation().getOperationId()
196-
: "<unknown>";
197-
throw new IllegalArgumentException(
198-
"Missing required OpenAPI parameters for operation '" + operationId + "': " + missing);
199159
}
200160
}
201161
}

0 commit comments

Comments
 (0)