Skip to content

Commit 4fb03d4

Browse files
dixitdeepakdixitdeepak
committed
Reverted the service definition verb attribute introduced by OFBIZ-11328 for mapping exported services to specific HTTP methods.
- Exported services are currently exposed through the service engine and have historically been available via POST requests. - Introducing HTTP method definitions in service descriptors adds REST-specific concerns to service definitions. - The framework's REST implementation already provides a dedicated mechanism for defining REST endpoints through rest.xml. - Service definitions should remain transport-agnostic and not require HTTP method metadata. - Mixing service contracts and REST endpoint definitions creates duplication and can lead to inconsistent API behavior. - Remove the action attribute from service definitions. - Remove HTTP method mapping logic introduced for exported services. Applications requiring REST-style endpoint definitions should use rest.xml and the REST framework rather than annotating service definitions with HTTP-specific metadata. This keeps service definitions independent of protocol concerns and preserves backward compatibility with existing exported service behavior.
1 parent a595a5c commit 4fb03d4

7 files changed

Lines changed: 11 additions & 89 deletions

File tree

‎framework/rest-api/src/main/java/org/apache/ofbiz/ws/rs/ServiceRequestFilter.java‎

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@
3131
import jakarta.annotation.Priority;
3232
import jakarta.servlet.ServletContext;
3333
import jakarta.servlet.http.HttpServletRequest;
34-
import jakarta.ws.rs.BadRequestException;
35-
import jakarta.ws.rs.HttpMethod;
3634
import jakarta.ws.rs.NotFoundException;
3735
import jakarta.ws.rs.Priorities;
3836
import jakarta.ws.rs.container.ContainerRequestContext;
@@ -73,9 +71,6 @@ public class ServiceRequestFilter implements ContainerRequestFilter {
7371
* <li>The service exists — throws {@link ServiceNotFoundException} if not</li>
7472
* <li>The service is marked as exportable — throws {@link NotFoundException} if not</li>
7573
* <li>The service has an HTTP action defined — throws {@link NotFoundException} if not</li>
76-
* <li>The HTTP method matches the service action — throws {@link MethodNotAllowedException} if not</li>
77-
* <li>GET requests include the {@code inParams} query parameter —
78-
* throws {@link BadRequestException} if absent</li>
7974
* </ul>
8075
*
8176
* <p>On successful validation, the service name is stored in
@@ -86,8 +81,6 @@ public class ServiceRequestFilter implements ContainerRequestFilter {
8681
public void filter(ContainerRequestContext requestContext) throws IOException {
8782
Debug.logInfo("Service request is going to get validated!", MODULE);
8883
String service = (String) RestApiUtil.extractParams(uriInfo.getPathParameters()).get("serviceName");
89-
String method = requestContext.getMethod();
90-
String action = null;
9184
if (UtilValidate.isNotEmpty(service)) {
9285
ModelService mdService = null;
9386
try {
@@ -100,23 +93,10 @@ public void filter(ContainerRequestContext requestContext) throws IOException {
10093
throw new ServiceNotFoundException(service);
10194
}
10295

103-
if (mdService != null && !mdService.isExport()) {
96+
if (!mdService.isExport()) {
10497
throw new NotFoundException("Service '" + service + "' is not exportable.");
10598
}
10699

107-
action = mdService.getAction();
108-
if (mdService != null && UtilValidate.isEmpty(action)) {
109-
throw new NotFoundException("Service '" + service + "' does not have HTTP action defined.");
110-
}
111-
112-
if (!action.equalsIgnoreCase(method)) {
113-
throw new MethodNotAllowedException("HTTP " + method + " is not allowed on service '" + service + "'");
114-
}
115-
116-
if (action.equalsIgnoreCase(HttpMethod.GET) && UtilValidate.isNotEmpty(mdService.getInParamNamesMap())
117-
&& UtilValidate.isEmpty(httpRequest.getParameter(SVC_IN_PARAMS))) {
118-
throw new BadRequestException("Missing Parameter: 'inParams'");
119-
}
120100
// If everything looks good, set the 'requestForService' property in the
121101
// context. Indicates which service this request is for.
122102
ServiceNameContextHolder.set(service);

‎framework/rest-api/src/main/java/org/apache/ofbiz/ws/rs/ServiceRequestProcessor.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
import java.util.Map;
2222

23-
import org.apache.ofbiz.base.util.UtilValidate;
2423
import org.apache.ofbiz.entity.GenericValue;
2524
import org.apache.ofbiz.service.DispatchContext;
2625
import org.apache.ofbiz.service.GenericServiceException;
@@ -75,9 +74,6 @@ public Response process(Map<String, Object> requestContext) throws GenericServic
7574
} catch (GenericServiceException gse) {
7675
throw new NotFoundException(gse.getMessage());
7776
}
78-
if (UtilValidate.isNotEmpty(service.getAction()) && !service.getAction().equalsIgnoreCase(httpVerb)) {
79-
throw new MethodNotAllowedException("HTTP " + httpVerb + " is not allowed on this service.");
80-
}
8177
Map<String, Object> serviceContext = dispatchContext.makeValidContext(serviceName, ModelService.IN_PARAM, requestMap);
8278
serviceContext.put("userLogin", userLogin);
8379
Map<String, Object> result = dispatcher.runSync(serviceName, serviceContext);

‎framework/rest-api/src/main/java/org/apache/ofbiz/ws/rs/openapi/OFBizOpenApiReader.java‎

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import jakarta.ws.rs.core.Response;
3131

3232
import org.apache.ofbiz.base.util.Debug;
33-
import org.apache.ofbiz.base.util.UtilValidate;
3433
import org.apache.ofbiz.service.DispatchContext;
3534
import org.apache.ofbiz.service.GenericServiceException;
3635
import org.apache.ofbiz.service.LocalDispatcher;
@@ -224,40 +223,23 @@ private void addExportableServices() {
224223
} catch (GenericServiceException e) {
225224
e.printStackTrace();
226225
}
227-
if (service != null && service.isExport() && UtilValidate.isNotEmpty(service.getAction())) {
228-
String action = service.getAction().toUpperCase();
226+
if (service != null && service.isExport()) {
229227
SecurityRequirement security = new SecurityRequirement();
230228
security.addList("jwtToken");
231229
final Operation operation = new Operation().summary(service.getDescription())
232230
.description(service.getDescription()).addTagsItem("Exported Services")
233231
.operationId(service.getName()).deprecated(false).addSecurityItem(security);
234232
PathItem pathItemObject = new PathItem();
235-
if (service.getAction().equalsIgnoreCase(HttpMethod.GET)) {
236-
boolean inParamsEmpty = UtilValidate.isEmpty(service.getInParamNamesMap());
237-
if (!inParamsEmpty) {
238-
QueryParameter serviceInParam = new QueryParameter();
239-
serviceInParam.setRequired(true);
240-
serviceInParam.setDescription("Operation Input Parameters in JSON");
241-
serviceInParam.setName("input");
242-
243-
Schema<?> refSchema = new Schema<>();
244-
refSchema.$ref("#/components/schemas/" + "api.request." + service.getName());
245-
serviceInParam.content(new Content().addMediaType(jakarta.ws.rs.core.MediaType.APPLICATION_JSON,
246-
new MediaType().schema(refSchema)));
247-
operation.addParametersItem(serviceInParam);
248-
}
249-
operation.addParametersItem(HEADER_ACCEPT_JSON);
250-
} else if (action.matches(HttpMethod.POST + "|" + HttpMethod.PUT + "|" + HttpMethod.PATCH)) {
251-
RequestBody request = new RequestBody().description("Request Body for service " + service.getName())
252-
.content(new Content().addMediaType(jakarta.ws.rs.core.MediaType.APPLICATION_JSON,
253-
new MediaType().schema(new Schema<>().$ref("#/components/schemas/" + "api.request." + service.getName()))));
254-
operation.setRequestBody(request);
255-
operation.addParametersItem(HEADER_CONTENT_TYPE_JSON);
256-
}
233+
RequestBody request = new RequestBody().description("Request Body for service " + service.getName())
234+
.content(new Content().addMediaType(jakarta.ws.rs.core.MediaType.APPLICATION_JSON,
235+
new MediaType().schema(new Schema<>().$ref("#/components/schemas/" + "api.request." + service.getName()))));
236+
operation.setRequestBody(request);
237+
operation.addParametersItem(HEADER_CONTENT_TYPE_JSON);
238+
257239
addServiceOutSchema(service);
258240
addServiceInSchema(service);
259241
addServiceOperationApiResponses(service, operation);
260-
setPathItemOperation(pathItemObject, service.getAction().toUpperCase(), operation);
242+
setPathItemOperation(pathItemObject, HttpMethod.POST, operation);
261243
paths.addPathItem("/services/" + service.getName(), pathItemObject);
262244
}
263245
}

‎framework/rest-api/src/main/java/org/apache/ofbiz/ws/rs/resources/OFBizServiceResource.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,12 @@ public Response serviceList() throws GenericServiceException {
9797
List<Map<String, Object>> serviceList = new ArrayList<>();
9898
for (String serviceName : serviceNames) {
9999
ModelService service = context.getModelService(serviceName);
100-
if (service != null && service.isExport() && UtilValidate.isNotEmpty(service.getAction())) {
100+
if (service != null && service.isExport()) {
101101
Map<String, Object> serviceMap = new LinkedHashMap<String, Object>();
102102
serviceMap.put("name", service.getName());
103103
serviceMap.put("description", service.getDescription());
104104
Link selfLink = Link.fromUriBuilder(uriInfo.getAbsolutePathBuilder().path(service.getName()))
105-
.type(service.getAction()).rel("self").build();
105+
.type(HttpMethod.POST).rel("self").build();
106106
serviceMap.put("link", selfLink);
107107
serviceList.add(serviceMap);
108108
}

‎framework/service/dtd/services.xsd‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,6 @@ under the License.
6767
<xs:attribute name="export" type="xs:boolean" default="false"/>
6868
<xs:attribute name="validate" type="xs:boolean" default="true"/>
6969
<xs:attribute name="default-entity-name" type="xs:string"/>
70-
<xs:attribute name="action">
71-
<xs:annotation>
72-
<xs:documentation>
73-
Specifies the HTTP method name this service can be called using REST interface. For now only POST and GET are supported.
74-
Services that have export=true and have action attribute defined can be called using REST interface.
75-
</xs:documentation>
76-
</xs:annotation>
77-
<xs:simpleType>
78-
<xs:restriction base="xs:string">
79-
<xs:enumeration value="POST"/>
80-
<xs:enumeration value="GET"/>
81-
</xs:restriction>
82-
</xs:simpleType>
83-
</xs:attribute>
8470
<xs:attribute name="use-transaction" type="xs:boolean" default="true">
8571
<xs:annotation>
8672
<xs:documentation>

‎framework/service/src/main/java/org/apache/ofbiz/service/ModelService.java‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,6 @@ public class ModelService extends AbstractMap<String, Object> implements Seriali
134134
/** The namespace of this service */
135135
private String nameSpace;
136136

137-
/** The corresponding REST verb behaviour for this service */
138-
private String action;
139-
140137
/** The package name or location of this service */
141138
private String location;
142139

@@ -262,14 +259,6 @@ public void setNameSpace(String nameSpace) {
262259
this.nameSpace = nameSpace;
263260
}
264261

265-
/**
266-
* Sets action.
267-
* @param action the action
268-
*/
269-
public void setAction(String action) {
270-
this.action = action;
271-
}
272-
273262
/**
274263
* Sets location.
275264
* @param location the location
@@ -526,14 +515,6 @@ public String getNameSpace() {
526515
return nameSpace;
527516
}
528517

529-
/**
530-
* Gets action.
531-
* @return the action
532-
*/
533-
public String getAction() {
534-
return action;
535-
}
536-
537518
/**
538519
* Gets default entity name.
539520
* @return the default entity name
@@ -801,7 +782,6 @@ public ModelService(ModelService model) {
801782
this.defaultEntityName = model.defaultEntityName;
802783
this.auth = model.auth;
803784
this.export = model.export;
804-
this.action = model.action;
805785
this.validate = model.validate;
806786
this.useTransaction = model.useTransaction;
807787
this.requireNewTransaction = model.requireNewTransaction;
@@ -937,7 +917,6 @@ public String toString() {
937917
buf.append(defaultEntityName).append("::");
938918
buf.append(auth).append("::");
939919
buf.append(export).append("::");
940-
buf.append(action).append("::");
941920
buf.append(validate).append("::");
942921
buf.append(useTransaction).append("::");
943922
buf.append(requireNewTransaction).append("::");

‎framework/service/src/main/java/org/apache/ofbiz/service/ModelServiceReader.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,6 @@ private ModelService createModelService(Element serviceElement, String resourceL
170170
service.setSemaphore(UtilXml.checkEmpty(serviceElement.getAttribute("semaphore")).intern());
171171
service.setDefaultEntityName(UtilXml.checkEmpty(serviceElement.getAttribute("default-entity-name")).intern());
172172
service.setFromLoader(isFromURL ? readerURL.toExternalForm() : handler.getLoaderName());
173-
service.setAction(UtilXml.checkEmpty(serviceElement.getAttribute("action")).intern());
174173

175174
// these default to true; if anything but true, make false
176175
service.setAuth("true".equalsIgnoreCase(serviceElement.getAttribute("auth")));

0 commit comments

Comments
 (0)