Skip to content
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

Jump to bottom

IdentifyApi Refactored #2083

Closed
wants to merge 8 commits into from
Closed

IdentifyApi Refactored #2083

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6b4173a
IdentifyApi Refactored
shnrndk Jul 12, 2023
fc5bd93
Identify api method bugs fixed
shnrndk Jul 14, 2023
15f649b
refactored the methods to get resources of apis
chamikasudusinghe Jun 20, 2023
757c5fb
Api invocation bug fixed after enabling api logs
shnrndk Jul 23, 2023
4114329
Api invocation bug fixed after enabling api logs
shnrndk Jul 23, 2023
74dc6e3
Merge remote-tracking branch 'origin/master'
shnrndk Jul 25, 2023
97d23a3
Fixes done to the code after api identification PR review.
shnrndk Aug 21, 2023
3848c18
Merge branch 'wso2:master' into master
shnrndk Nov 6, 2023
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
44 changes: 10 additions & 34 deletions modules/core/src/main/java/org/apache/synapse/api/API.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@
import org.apache.synapse.util.logging.LoggingUtils;

import java.util.Arrays;
import java.util.Collection;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.List;
import java.util.LinkedHashMap;
import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.Collection;
import java.util.Set;
import java.util.HashSet;

Expand Down Expand Up @@ -246,6 +246,10 @@ public Resource[] getResources() {
return resources.values().toArray(new Resource[resources.size()]);
}

public Map<String, Resource> getResourcesMap() {
return resources;
}

public void addHandler(Handler handler) {
handlers.add(handler);
}
Expand Down Expand Up @@ -276,20 +280,14 @@ public boolean canProcess(MessageContext synCtx) {
return false;
}
} else {
String path = ApiUtils.getFullRequestPath(synCtx);
if (null == synCtx.getProperty(RESTConstants.IS_PROMETHEUS_ENGAGED) &&
(!ApiUtils.matchApiPath(path, context))) {
auditDebug("API context: " + context + " does not match request URI: " + path);
return false;
}

if(!versionStrategy.isMatchingVersion(synCtx)){
if (!ApiUtils.identifyApi(this, synCtx)) {
return false;
}else{
ApiUtils.getFullRequestPath(synCtx);
shnrndk marked this conversation as resolved.
Show resolved Hide resolved
}

org.apache.axis2.context.MessageContext msgCtx =
((Axis2MessageContext) synCtx).getAxis2MessageContext();

if (host != null || port != -1) {
String hostHeader = getHostHeader(msgCtx);
if (hostHeader != null) {
Expand Down Expand Up @@ -443,12 +441,7 @@ public void process(MessageContext synCtx) {
msgCtx.getIncomingTransportName() + "://" + hostHeader);
}

Set<Resource> acceptableResources = new LinkedHashSet<Resource>();
for (Resource r : resources.values()) {
if (isBound(r, synCtx) && r.canProcess(synCtx)) {
acceptableResources.add(r);
}
}
Set<Resource> acceptableResources = ApiUtils.getAcceptableResources(resources, synCtx);

boolean processed = false;
if (!acceptableResources.isEmpty()) {
Expand Down Expand Up @@ -520,23 +513,6 @@ private void handleMethodNotAllowed(MessageContext synCtx) {

}

/**
* Checks whether the provided resource is capable of processing the message from the provided message context.
* The resource becomes capable to do this when the it contains either the name of the api caller,
* or {@value ApiConstants#DEFAULT_BINDING_ENDPOINT_NAME}, in its binds-to.
*
* @param resource Resource object
* @param synCtx MessageContext object
* @return Whether the provided resource is bound to the provided message context
*/
private boolean isBound(Resource resource, MessageContext synCtx) {
Collection<String> bindings = resource.getBindsTo();
Object apiCaller = synCtx.getProperty(ApiConstants.API_CALLER);
if (apiCaller != null) {
return bindings.contains(apiCaller.toString());
}
return bindings.contains(ApiConstants.DEFAULT_BINDING_ENDPOINT_NAME);
}

/**
* Helper method to use when no matching resource found
Expand Down
21 changes: 16 additions & 5 deletions modules/core/src/main/java/org/apache/synapse/api/AbstractApiHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,30 @@ protected boolean dispatchToAPI(Collection<API> apiSet, MessageContext synCtx) {

Object apiObject = synCtx.getProperty(RESTConstants.PROCESSED_API);
if (apiObject != null) {
if (identifyAPI((API) apiObject, synCtx, defaultStrategyApiSet)) {
if (startProcess((API) apiObject, synCtx, defaultStrategyApiSet)) {
return true;
}
} else {
//To avoid apiSet being modified concurrently
List<API> duplicateApiSet = new ArrayList<API>(apiSet);
for (API api : duplicateApiSet) {
if (identifyAPI(api, synCtx, defaultStrategyApiSet)) {
API identifiedApi = (API) synCtx.getProperty(RESTConstants.IDENTIFIED_API);
//If normal path this goes to else, if logging enabled then goes inside the if method.
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic is not clear to me, can you please explain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the api logging flow in the first iteration it needs to identify the api. For that it calls the identifyApi() method. If the api is identified then that api gets stored in the IDENTIFIED_API property. Then in the second iteration, we do not need to identify the api again. Therefore we get the identified api from the from the IDENTIFIED_API property. So we do not need to iterate all the apis in the duplicateApiSet to identify the api.

But if the api logging is disabled then api is not identified previously, therefore IDENTIFIED_API property is null, So the api needs to be identified by iterating all the apis in the defaultStrategyApiSet.

if (identifiedApi != null
&& duplicateApiSet.contains(identifiedApi)) {
if (startProcess(identifiedApi, synCtx, defaultStrategyApiSet)) {
return true;
}
} else {
for (API api : duplicateApiSet) {
if (startProcess(api, synCtx, defaultStrategyApiSet)) {
return true;
}
}
}
}

if(synCtx.getProperty(RESTConstants.IDENTIFIED_API) != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for removing the property?

synCtx.getPropertyKeySet().remove(RESTConstants.IDENTIFIED_API);
}
for (API api : defaultStrategyApiSet) {
api.setLogSetterValue();
if (api.canProcess(synCtx)) {
Expand Down Expand Up @@ -110,7 +121,7 @@ protected void apiProcessNonDefaultStrategy(MessageContext synCtx, API api) {
}
}

protected boolean identifyAPI(API api, MessageContext synCtx, List defaultStrategyApiSet) {
protected boolean startProcess(API api, MessageContext synCtx, List defaultStrategyApiSet) {
API defaultAPI = null;
api.setLogSetterValue();
if ("/".equals(api.getContext())) {
Expand Down
59 changes: 54 additions & 5 deletions modules/core/src/main/java/org/apache/synapse/api/ApiUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
import org.apache.synapse.MessageContext;
import org.apache.synapse.SynapseConstants;
import org.apache.synapse.SynapseException;
import org.apache.synapse.api.version.ContextVersionStrategy;
import org.apache.synapse.api.version.DefaultStrategy;
import org.apache.synapse.api.version.URLBasedVersionStrategy;
import org.apache.synapse.api.version.VersionStrategy;
import org.apache.synapse.core.axis2.Axis2MessageContext;
import org.apache.synapse.rest.RESTConstants;
import org.apache.synapse.api.dispatch.DefaultDispatcher;
Expand All @@ -39,8 +43,7 @@
import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLDecoder;
import java.util.ArrayList;
import java.util.List;
import java.util.*;

public class ApiUtils {

Expand Down Expand Up @@ -158,6 +161,52 @@ public static String getSubRequestPath(MessageContext synCtx) {
return (String) synCtx.getProperty(RESTConstants.REST_SUB_REQUEST_PATH);
}

public static boolean identifyApi(API api, MessageContext synCtx) {
if (synCtx.getProperty(RESTConstants.IDENTIFIED_API) == null) {
String path = ApiUtils.getFullRequestPath(synCtx);
if (null == synCtx.getProperty(RESTConstants.IS_PROMETHEUS_ENGAGED) &&
shnrndk marked this conversation as resolved.
Show resolved Hide resolved
(!ApiUtils.matchApiPath(path, api.getContext()))) {
log.debug("API context: " + api.getContext() + " does not match request URI: " + path);
return false;
}
if (!api.getVersionStrategy().isMatchingVersion(synCtx)) {
return false;
}
synCtx.setProperty(RESTConstants.IDENTIFIED_API, api);
return true;
} else {
return true;
}
}

/**
* Checks whether the provided resource is capable of processing the message from the provided message context.
* The resource becomes capable to do this when the it contains either the name of the api caller,
* or {@value ApiConstants#DEFAULT_BINDING_ENDPOINT_NAME}, in its binds-to.
*
* @param resource Resource object
* @param synCtx MessageContext object
* @return Whether the provided resource is bound to the provided message context
*/
public static boolean isBound(Resource resource, MessageContext synCtx) {
Copy link
Contributor

Choose a reason for hiding this comment

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

make it private

Collection<String> bindings = resource.getBindsTo();
Object apiCaller = synCtx.getProperty(ApiConstants.API_CALLER);
if (apiCaller != null) {
return bindings.contains(apiCaller.toString());
}
return bindings.contains(ApiConstants.DEFAULT_BINDING_ENDPOINT_NAME);
}

public static Set<Resource> getAcceptableResources(Map<String, Resource> resources, MessageContext synCtx) {
Set<Resource> acceptableResources = new LinkedHashSet<Resource>();
for (Resource r : resources.values()) {
if (isBound(r, synCtx) && r.canProcess(synCtx)) {
acceptableResources.add(r);
}
}
return acceptableResources;
}

public static List<RESTDispatcher> getDispatchers() {
return dispatchers;
}
Expand All @@ -177,10 +226,10 @@ private static void handleException(String msg, Throwable t) {
* and false if the two values don't match
*/
public static boolean matchApiPath(String path, String context) {
if (!path.startsWith(context + "/") && !path.startsWith(context + "?") &&
!context.equals(path) && !"/".equals(context)) {
if (!path.startsWith(context + "/") && !path.startsWith(context + "?")
&& !context.equals(path) && !"/".equals(context)) {
return false;
}
return true;
}
}
}
2 changes: 2 additions & 0 deletions modules/core/src/main/java/org/apache/synapse/api/dispatch/DefaultDispatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import org.apache.synapse.MessageContext;
import org.apache.synapse.api.Resource;
import org.apache.synapse.rest.RESTConstants;

import java.util.Collection;

Expand All @@ -28,6 +29,7 @@ public class DefaultDispatcher implements RESTDispatcher {
public Resource findResource(MessageContext synCtx, Collection<Resource> resources) {
for (Resource resource : resources) {
if (resource.getDispatcherHelper() == null) {
synCtx.setProperty(RESTConstants.SELECTED_RESOURCE, resource);
return resource;
}
}
Expand Down
5 changes: 3 additions & 2 deletions modules/core/src/main/java/org/apache/synapse/api/dispatch/URITemplateBasedDispatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,13 @@ public Resource findResource(MessageContext synCtx, Collection<Resource> resourc
DispatcherHelper helper = r.getDispatcherHelper();
if (helper instanceof URITemplateHelper) {
URITemplateHelper templateHelper = (URITemplateHelper) helper;
Map<String,String> variables = new HashMap<String,String>();
Map<String, String> variables = new HashMap<String, String>();
if (templateHelper.getUriTemplate().matches(url, variables)) {
for (Map.Entry<String,String> entry : variables.entrySet()) {
for (Map.Entry<String, String> entry : variables.entrySet()) {
synCtx.setProperty(RESTConstants.REST_URI_VARIABLE_PREFIX + entry.getKey(),
entry.getValue());
}
synCtx.setProperty(RESTConstants.SELECTED_RESOURCE, r);
return r;
}
}
Expand Down
4 changes: 4 additions & 0 deletions modules/core/src/main/java/org/apache/synapse/api/dispatch/URLMappingBasedDispatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.synapse.MessageContext;
import org.apache.synapse.api.ApiUtils;
import org.apache.synapse.api.Resource;
import org.apache.synapse.rest.RESTConstants;

import java.util.ArrayList;
import java.util.Collection;
Expand Down Expand Up @@ -55,6 +56,7 @@ public Resource findResource(MessageContext synCtx, Collection<Resource> resourc
if (log.isDebugEnabled()) {
log.debug("Found exact URL match for: " + url);
}
synCtx.setProperty(RESTConstants.SELECTED_RESOURCE, filteredResources.get(i));
return filteredResources.get(i);
}
}
Expand All @@ -72,6 +74,7 @@ public Resource findResource(MessageContext synCtx, Collection<Resource> resourc
if (log.isDebugEnabled()) {
log.debug("Found path match for: " + url + " with matching length: " + maxLength);
}
synCtx.setProperty(RESTConstants.SELECTED_RESOURCE, matchedResource);
return matchedResource;
}

Expand All @@ -80,6 +83,7 @@ public Resource findResource(MessageContext synCtx, Collection<Resource> resourc
if (log.isDebugEnabled()) {
log.debug("Found extension match for: " + url);
}
synCtx.setProperty(RESTConstants.SELECTED_RESOURCE, filteredResources.get(i));
return filteredResources.get(i);
}
}
Expand Down
2 changes: 2 additions & 0 deletions modules/core/src/main/java/org/apache/synapse/rest/RESTConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,5 +91,7 @@ public static enum METHODS {
*/
public static final String IS_PROMETHEUS_ENGAGED = "IS_PROMETHEUS_ENGAGED";
public static final String PROCESSED_API = "PROCESSED_API";
public static final String IDENTIFIED_API = "IDENTIFIED_API";
public static final String SELECTED_RESOURCE = "SELECTED_RESOURCE";

}
Loading