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.

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

Add: Support Custom Sequence as a Backend of an API #12524

Merged
merged 56 commits into from
Sep 27, 2024

Conversation

BLasan
Copy link
Contributor

@BLasan BLasan commented Aug 25, 2024

Purpose

Some API developers want to use connectors within the api manager to implement mediation logic as they do not want to use an additional MI deployment if their use cases are simple. Within the API Manager, currently, we do not have a construct that will allow us to facilitate this requirement.

Approach

  1. Provide a new Endpoint Type "Sequence Backend" for REST APIs under Endpoint Tab
  2. Allow Sequences to be added for Sandbox and Production Key types separately,.
  3. Perform download, delete, upload operations for Sequence Backends.
  4. Add support for APICTL import and export scenarios for REST APIs
  5. Introduced a new table to store Sequence Backend. Following is the schema for the table.
CREATE TABLE IF NOT EXISTS AM_API_SEQUENCE_BACKEND (
  ID VARCHAR(60) NOT NULL,
  API_UUID VARCHAR(256) NOT NULL,
  REVISION_UUID VARCHAR(256) DEFAULT '0',
  SEQUENCE LONGBLOB NOT NULL,
  NAME VARCHAR(256) NOT NULL,
  TYPE VARCHAR(120) NOT NULL,
  PRIMARY KEY (ID),
  FOREIGN KEY (API_UUID) REFERENCES AM_API(API_UUID) ON DELETE CASCADE
)ENGINE INNODB;

Affected Flows

Please refer - wso2/api-manager#3007 (comment)

Issue

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@@ -487,6 +487,218 @@ paths:
-H "Content-Type: application/json" -d @data.json
"https://127.0.0.1:9443/api/am/publisher/v4/apis/7a2298c4-c905-403f-8fac-38c73301631f/topics"'

/apis/{apiId}/custom-backend/{customBackendId}/content:
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to sequence-backend/{sandbox/Prod}/{sequenceID} check dev portal key type implementation

schema:
type: string
content:
application/zip:
Copy link
Contributor

Choose a reason for hiding this comment

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

Change the content type

@@ -487,6 +487,218 @@ paths:
-H "Content-Type: application/json" -d @data.json
"https://127.0.0.1:9443/api/am/publisher/v4/apis/7a2298c4-c905-403f-8fac-38c73301631f/topics"'

/apis/{apiId}/custom-backend/{customBackendId}/content:
Copy link
Contributor

Choose a reason for hiding this comment

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

lets remove the sequenceID

schema:
type: string
content:
application/json:
Copy link
Contributor

Choose a reason for hiding this comment

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

Make this a text type

endpointConfig.put("sequence_id", resultSet.getString("ID"));
}
} catch (SQLException ex) {
handleException("Error when retrieving Custom Backend of API: " + apiUUID, ex);
Copy link
Contributor

Choose a reason for hiding this comment

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

Return the relevant exception codes

resultSet = ps.executeQuery();
while (resultSet.next()) {
map.put("type", resultSet.getString("TYPE"));
map.put("sequence_name", resultSet.getString("NAME"));
Copy link
Contributor

Choose a reason for hiding this comment

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

define and use Static variables

@BLasan
Copy link
Contributor Author

BLasan commented Sep 19, 2024

Only websub tests are failing now
Screenshot 2024-09-19 at 06 40 51

@BLasan BLasan marked this pull request as ready for review September 19, 2024 04:52
@BLasan
Copy link
Contributor Author

BLasan commented Sep 21, 2024

Screenshot 2024-09-21 at 07 34 12

@BLasan
Copy link
Contributor Author

BLasan commented Sep 23, 2024

Changes were not from this PR
Screenshot 2024-09-23 at 07 45 43

Screenshot 2024-09-23 at 07 44 33

@@ -65,6 +66,10 @@ public interface APIProvider extends APIManager {
Comment getComment(ApiTypeWrapper apiTypeWrapper, String commentId, Integer replyLimit, Integer replyOffset) throws
APIManagementException;

void deleteCustomBackendByID(String apiUUID, String type) throws APIManagementException;
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets add Java doc comments. Check other places as well

@@ -0,0 +1,58 @@
package org.wso2.carbon.apimgt.api.model;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add license header

.BACKEND_REQUEST_START_TIME));
messageContext.setProperty(APIMgtGatewayConstants.BACKEND_LATENCY, System.currentTimeMillis() -
executionStartTime);
if (APIUtil.isAnalyticsEnabled()
Copy link
Contributor

Choose a reason for hiding this comment

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

How will this be applied in Sequence as a backend scenario

@@ -21,6 +21,7 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.gson.Gson;

Copy link
Contributor

Choose a reason for hiding this comment

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

remove this new line

addPstmt.setString(6, rs.getString("NAME"));
addPstmt.addBatch();
count++;

Copy link
Contributor

Choose a reason for hiding this comment

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

Why these are commented out

* @return The Sequence of the Custom Backend as an Input Stream
* @throws APIManagementException If an error occurs while reading, throws an error
*/
public static InputStream getCustomBackendSequence(String extractedFolderPath, String customBackendFileName,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we returning an InputStream? where are we closing this?

}
}
} catch (IOException | ParseException ex) {
throw new APIManagementException("Error when updating Endpoint Configuration", ex);
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets add more details to this error, API name etc.

@BLasan
Copy link
Contributor Author

BLasan commented Sep 23, 2024

Screenshot 2024-09-23 at 13 29 29

@BLasan
Copy link
Contributor Author

BLasan commented Sep 26, 2024

Build is passing
Screenshot 2024-09-26 at 07 27 01

addCustomBackend(apiUUID, sequenceName, null, sequence, type, connection, backendUUID);
connection.commit();
} catch (SQLException e) {
handleException("Error while adding Custom Backend for API : " + apiUUID, e);
Copy link
Contributor

Choose a reason for hiding this comment

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

We might need to add a rollback in case of an exception. Check other places as well.

String fileName = extractedFolderPath + File.separator + sequenceName;
if (checkFileExistence(fileName)) {
if (log.isDebugEnabled()) {
log.debug("Found policy definition file " + fileName);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be not policy but custom backend right?

@@ -1858,6 +1858,17 @@ CREATE TABLE IF NOT EXISTS AM_API (
UNIQUE (API_UUID)
);

CREATE TABLE IF NOT EXISTS AM_API_SEQUENCE_BACKEND (
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to add the database changes to multi-dc scripts as well.

@@ -0,0 +1,142 @@
package org.wso2.carbon.apimgt.rest.api.publisher.v1.dto;
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets add the license header. Check other places as well

import io.swagger.annotations.*;
import java.util.Objects;

import javax.xml.bind.annotation.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets remove the * and add all the dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an auto generated one

@RakhithaRR RakhithaRR merged commit c72f127 into wso2:master Sep 27, 2024
2 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants