Merge pull request #280 from ayeshLK/build_stabilization

Update logs in WebSub Module

.github/workflows/pull-request.yml

@@ -30,7 +30,7 @@ jobs:
         if: failure()
         with:
           name: Ballerina Internal Log
-          path: websubhub-ballerina/ballerina-internal.log
+          path: websub-ballerina/ballerina-internal.log
           if-no-files-found: ignore
       - name: Generate Codecov Report
         if:  github.event_name == 'pull_request'

changelog.md

@@ -5,8 +5,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
-- Multipart content retrieval on event-notification.
-- Subscriber resubscription event.
+
+### Fixed
+
+- [Log error when return from the remote method leads to an error](https://github.com/ballerina-platform/ballerina-standard-library/issues/1450)
+- [WebSubHub Compiler Plugin does not allow additional methods inside service declaration](https://github.com/ballerina-platform/ballerina-standard-library/issues/1417)
 
 ## [1.2.0-beta.1] - 2021-05-06
 

websub-ballerina/errors.bal

@@ -17,7 +17,7 @@
 # Represents a webSub distinct error.
 public type Error distinct error<CommonResponse>;
 
-# Represents a websubhub service execution error.
+# Represents a websub service execution error.
 public type ServiceExecutionError distinct error<CommonResponse>;
 
 # Represents a listener errors.

websub-ballerina/request_processor.bal

@@ -139,7 +139,7 @@ isolated function processEventNotification(http:Caller caller, http:Request requ
             }; 
         }
         _ => {
-            log:printError(string`Unrecognized content-type [${contentType}] found`);
+            log:printError(string `Unrecognized content-type [${contentType}] found`);
         }
     }
 

websub-ballerina/sub_listener.bal

@@ -134,7 +134,8 @@ public class Listener {
         if serviceConfig is SubscriberServiceConfiguration {
             error? result = initiateSubscription(serviceConfig, <string>callback);
             if result is error {
-                string errorMsg = string`Subscription initiation failed due to [${result.message()}]`;
+                string errorDetails = result.message();
+                string errorMsg = string `Subscription initiation failed due to: ${errorDetails}`;
                 return error SubscriptionInitiationError(errorMsg);
             }
         }
@@ -226,7 +227,7 @@ isolated function retrieveCallbackUrl(string? providedCallback, boolean appendSe
     if providedCallback is string {
         if appendServicePath {
             string completeSevicePath = retrieveCompleteServicePath(servicePath);
-            return string`${providedCallback}${completeSevicePath}`;
+            return string `${providedCallback}${completeSevicePath}`;
         } else {
             return providedCallback;
         }
@@ -249,7 +250,7 @@ isolated function generateCallbackUrl(string[]|string servicePath,
     string host = config.host;
     string protocol = config.secureSocket is () ? "http" : "https";        
     string completeSevicePath = retrieveCompleteServicePath(servicePath);
-    return string`${protocol}://${host}:${port.toString()}${completeSevicePath}`;
+    return string `${protocol}://${host}:${port.toString()}${completeSevicePath}`;
 }
 
 # Retrieves the complete service path.
@@ -321,8 +322,8 @@ isolated function initiateSubscription(SubscriberServiceConfiguration serviceCon
     SubscriptionChangeRequest request = retrieveSubscriptionRequest(topicUrl, callbackUrl, serviceConfig);
     var response = subscriberClientEp->subscribe(request);
     if response is SubscriptionChangeResponse {
-        string subscriptionSuccessMsg = string`Subscription Request successfully sent to Hub[${response.hub}], for Topic[${response.topic}], with Callback [${callbackUrl}]`;
-        log:printInfo(string`${subscriptionSuccessMsg}. Awaiting intent verification.`);
+        string subscriptionSuccessMsg = string `Subscription Request successfully sent to Hub[${response.hub}], for Topic[${response.topic}], with Callback [${callbackUrl}]`;
+        log:printDebug(subscriptionSuccessMsg);
     } else {
         return response;
     }

websub-ballerina/subscriber_client.bal

@@ -16,7 +16,6 @@
 
 import ballerina/url;
 import ballerina/http;
-import ballerina/log;
 import ballerina/mime;
 
 # The HTTP based client for WebSub subscription and unsubscription.
@@ -42,7 +41,7 @@ public client class SubscriptionClient {
 
     # Sends a subscription request to the provided `hub`.
     # ```ballerina
-    # websub:SubscriptionChangeResponse response = check websubHubClientEP->subscribe(subscriptionRequest);
+    # websub:SubscriptionChangeResponse response = check subscriberClientEp->subscribe(subscriptionRequest);
     # ```
     #
     # + subscriptionRequest - The request payload containing the subscription details
@@ -59,7 +58,7 @@ public client class SubscriptionClient {
 
     # Sends an unsubscription request to a WebSub Hub.
     # ```ballerina
-    # websub:SubscriptionChangeResponse response = check websubHubClientEP->unsubscribe(subscriptionRequest);
+    # websub:SubscriptionChangeResponse response = check subscriberClientEp->unsubscribe(subscriptionRequest);
     # ```
     # + unsubscriptionRequest - The request payload containing the unsubscription details
     # + return - The `websub:SubscriptionChangeResponse` indicating that the unsubscription initiation was successful
@@ -151,9 +150,6 @@ isolated function processHubResponse(@untainted string hub, @untainted string mo
             }
             return error SubscriptionInitiationError(errorMessage);
         } else {
-            if responseStatusCode != http:STATUS_ACCEPTED {
-                log:printWarn(string`Subscription request considered successful for non 202 status code: ${responseStatusCode.toString()}`);
-            }
             SubscriptionChangeResponse subscriptionChangeResponse = {hub:hub, topic:topic, response:hubResponse};
             return subscriptionChangeResponse;
         }

websub-ballerina/tests/content_verification_test.bal

@@ -58,7 +58,7 @@ function testOnEventNotificationSuccessForContentVerification() returns @tainted
     http:Request request = new;
     json payload =  {"action":"publish","mode":"remote-hub"};
     byte[] payloadHash = check retrievePayloadSignature(hashKey, payload);
-    request.setHeader("X-Hub-Signature", string`sha256=${payloadHash.toBase16()}`);
+    request.setHeader("X-Hub-Signature", string `sha256=${payloadHash.toBase16()}`);
     request.setPayload(payload);
     http:Response response = check contentVerificationClient->post("/", request);
     test:assertEquals(response.statusCode, 202);
@@ -73,7 +73,7 @@ function testOnEventNotificationSuccessXmlForContentVerification() returns @tain
     http:Request request = new;
     xml payload = xml `<body><action>publish</action></body>`;
     byte[] payloadHash = check retrievePayloadSignature(hashKey, payload);
-    request.setHeader("X-Hub-Signature", string`sha256=${payloadHash.toBase16()}`);
+    request.setHeader("X-Hub-Signature", string `sha256=${payloadHash.toBase16()}`);
     request.setPayload(payload);
     http:Response response = check contentVerificationClient->post("/", request);
     test:assertEquals(response.statusCode, 202);
@@ -88,7 +88,7 @@ function testOnEventNotificationSuccessForUrlEncodedForContentVerification() ret
     string payload = "param1=value1&param2=value2";
     byte[] payloadHash = check retrievePayloadSignature(hashKey, payload);
     request.setTextPayload(payload);
-    request.setHeader("X-Hub-Signature", string`sha256=${payloadHash.toBase16()}`);
+    request.setHeader("X-Hub-Signature", string `sha256=${payloadHash.toBase16()}`);
     check request.setContentType(mime:APPLICATION_FORM_URLENCODED);
     http:Response response = check contentVerificationClient->post("", request);
     test:assertEquals(response.statusCode, 202);

websub-ballerina/tests/subscriber_with_error_return_types.bal

@@ -22,18 +22,16 @@ listener Listener errorReturnsSubscriberListener = new (9099);
 
 var subscriberWithErrorReturns = @SubscriberServiceConfig { target: "http://0.0.0.0:9191/common/discovery", leaseSeconds: 36000 } 
                               service object {
-    isolated remote function onSubscriptionValidationDenied(SubscriptionDeniedError msg) returns Acknowledgement|error? {
-        return error Error("Error occured while processing request");
+    isolated remote function onSubscriptionValidationDenied(SubscriptionDeniedError msg) returns error? {
+        return error ("Error occured while processing request");
     }
 
-    isolated remote function onSubscriptionVerification(SubscriptionVerification msg)
-                        returns SubscriptionVerificationSuccess|SubscriptionVerificationError|error {
-        return error Error("Error occured while processing request");
+    isolated remote function onSubscriptionVerification(SubscriptionVerification msg) returns error {
+        return error ("Error occured while processing request");
     }
 
-    isolated remote function onEventNotification(ContentDistributionMessage event) 
-                        returns Acknowledgement|SubscriptionDeletedError|error? {
-        return error Error("Error occured while processing request");
+    isolated remote function onEventNotification(ContentDistributionMessage event) returns error? {
+        return error ("Error occured while processing request");
     }
 };
 

websub-ballerina/tests/subscription_initiation_test.bal

@@ -16,6 +16,7 @@
 
 import ballerina/test;
 import ballerina/http;
+import ballerina/log;
 
 const string CALLBACK = "https://sample.subscriber.com/subscriber";
 const string DISCOVERY_SUCCESS_URL = "http://127.0.0.1:9192/common/discovery";
@@ -68,6 +69,11 @@ isolated function testSubscriptionInitiationFailureWithDiscoveryUrl() returns @t
     SubscriberServiceConfiguration config = getServiceConfig(DISCOVERY_FAILURE_URL);
     var response = initiateSubscription(config, CALLBACK);
     test:assertTrue(response is ResourceDiscoveryFailedError);
+    if response is error {
+        string errorDetails = response.message();
+        string errorMsg = string `Subscription initiation failed due to: ${errorDetails}`;
+        log:printError(errorMsg);
+    }
 }
 
 @test:Config { 
@@ -77,5 +83,48 @@ isolated function testSubscriptionInitiationFailureWithHubAndTopic() returns @ta
     SubscriberServiceConfiguration config = getServiceConfig([ HUB_FAILURE_URL, COMMON_TOPIC ]);
     var response = initiateSubscription(config, CALLBACK);
     test:assertTrue(response is SubscriptionInitiationError);
+    if response is error {
+        string errorDetails = response.message();
+        string errorMsg = string `Subscription initiation failed due to: ${errorDetails}`;
+        log:printError(errorMsg);
+    }
+}
+
+final var websubServiceObj = service object {
+    isolated remote function onEventNotification(ContentDistributionMessage event) 
+                        returns Acknowledgement {        
+        return ACKNOWLEDGEMENT;
+    }
+};
+
+listener Listener ls = new (9100);
+
+@test:Config { 
+    groups: ["subscriptionInitiation"]
+}
+function testSubInitFailedWithListenerForResourceDiscoveryFailure() returns @tainted error? {
+    var res = ls.attachWithConfig(websubServiceObj, getServiceConfig(DISCOVERY_FAILURE_URL), "sub");
+    test:assertFalse(res is error);
+    var startDetails = ls.'start();
+    test:assertTrue(startDetails is error);
+    if startDetails is error {
+        string expected = "Subscription initiation failed due to: Link header unavailable in discovery response";
+        test:assertEquals(startDetails.message(), expected);
+    }
+    check ls.gracefulStop();
 }
 
+@test:Config { 
+    groups: ["subscriptionInitiation"]
+}
+function testSubInitFailedWithListenerForSubFailure() returns @tainted error? {
+    var res = ls.attachWithConfig(websubServiceObj, getServiceConfig([ HUB_FAILURE_URL, COMMON_TOPIC ]), "sub");
+    test:assertFalse(res is error);
+    var startDetails = ls.'start();
+    test:assertTrue(startDetails is error);
+    if startDetails is error {
+        string expected = "Subscription initiation failed due to: Error in request: Mode[subscribe] at Hub[http://127.0.0.1:9192/common/failed] - no matching resource found for path : /common/failed , method : POST";
+        test:assertEquals(startDetails.message(), expected);
+    }
+    check ls.gracefulStop();
+}

websub-ballerina/utils.bal

@@ -186,7 +186,7 @@ isolated function retrieveContentHash(string method, string key, string payload)
             return crypto:hmacSha512(contentPayload, keyArr);
         }
         _ => {
-            string errorMsg = string`Unrecognized hashning-method [${method}] found`;
+            string errorMsg = string `Unrecognized hashning-method [${method}] found`;
             log:printError(errorMsg);
             return error Error(errorMsg);
         }

websub-compiler-plugin-test/src/test/resources/ballerina_sources/sample_17/service.bal

@@ -26,14 +26,14 @@ service /subscriber on new websub:Listener(9090) {
                 int hookId = check retrievedContent.hook_id;
                 json sender = check retrievedContent.sender;
                 int senderId = check sender.id;
-                io:println(string`PingEvent received for webhook [${hookId}]`);
-                io:println(string`Event sender [${senderId}]`);
+                io:println(string `PingEvent received for webhook [${hookId}]`);
+                io:println(string `Event sender [${senderId}]`);
             } else if (retrievedContent.ref is string) {
                 json repository = check retrievedContent.repository;
                 string repositoryName = check repository.name;
                 string lastUpdatedTime = check repository.updated_at;
-                io:println(string`PushEvent received for [${repositoryName}]`);
-                io:println(string`Last updated at ${lastUpdatedTime}`);
+                io:println(string `PushEvent received for [${repositoryName}]`);
+                io:println(string `Last updated at ${lastUpdatedTime}`);
             }
         } else {
             io:println("Unrecognized content type, hence ignoring");

websub-compiler-plugin-test/src/test/resources/ballerina_sources/sample_18/service.bal

@@ -39,14 +39,14 @@ service /subscriber on new websub:Listener(9090) {
                 int hookId = check retrievedContent.hook_id;
                 json sender = check retrievedContent.sender;
                 int senderId = check sender.id;
-                io:println(string`PingEvent received for webhook [${hookId}]`);
-                io:println(string`Event sender [${senderId}]`);
+                io:println(string `PingEvent received for webhook [${hookId}]`);
+                io:println(string `Event sender [${senderId}]`);
             } else if (retrievedContent.ref is string) {
                 json repository = check retrievedContent.repository;
                 string repositoryName = check repository.name;
                 string lastUpdatedTime = check repository.updated_at;
-                io:println(string`PushEvent received for [${repositoryName}]`);
-                io:println(string`Last updated at ${lastUpdatedTime}`);
+                io:println(string `PushEvent received for [${repositoryName}]`);
+                io:println(string `Last updated at ${lastUpdatedTime}`);
             }
         } else {
             io:println("Unrecognized content type, hence ignoring");

websub-native/src/main/java/io/ballerina/stdlib/websub/Constants.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2021, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
+ *
+ * WSO2 Inc. licenses this file to you under the Apache License,
+ * Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package io.ballerina.stdlib.websub;
+
+/**
+ * {@code Constants} contains the public constants to be used.
+ */
+public interface Constants {
+    String PACKAGE_ORG = "ballerina";
+    String PACKAGE_NAME = "websub";
+
+    String SERVICE_OBJECT = "WEBSUB_SERVICE_OBJECT";
+}

websub-native/src/main/java/io/ballerina/stdlib/websub/NativeHttpToWebsubAdaptor.java

@@ -21,9 +21,7 @@
 import io.ballerina.runtime.api.Environment;
 import io.ballerina.runtime.api.Future;
 import io.ballerina.runtime.api.Module;
-import io.ballerina.runtime.api.async.Callback;
 import io.ballerina.runtime.api.async.StrandMetadata;
-import io.ballerina.runtime.api.creators.ErrorCreator;
 import io.ballerina.runtime.api.creators.ValueCreator;
 import io.ballerina.runtime.api.types.MethodType;
 import io.ballerina.runtime.api.utils.StringUtils;
@@ -35,14 +33,12 @@
 
 import java.util.ArrayList;
 
-import static io.ballerina.runtime.api.utils.StringUtils.fromString;
+import static io.ballerina.stdlib.websub.Constants.SERVICE_OBJECT;
 
 /**
  * {@code NativeHttpToWebsubAdaptor} is a wrapper object used for service method execution.
  */
 public class NativeHttpToWebsubAdaptor {
-    public static final String SERVICE_OBJECT = "WEBSUB_SERVICE_OBJECT";
-
     public static void externInit(BObject adaptor, BObject service) {
         adaptor.addNativeData(SERVICE_OBJECT, service);
     }
@@ -83,20 +79,8 @@ private static Object invokeRemoteFunction(Environment env, BObject bSubscriberS
         StrandMetadata metadata = new StrandMetadata(module.getOrg(), module.getName(), module.getVersion(),
                 parentFunctionName);
         Object[] args = new Object[]{message, true};
-        env.getRuntime().invokeMethodAsync(bSubscriberService, remoteFunctionName, null, metadata, new Callback() {
-            @Override
-            public void notifySuccess(Object result) {
-                balFuture.complete(result);
-            }
-
-            @Override
-            public void notifyFailure(BError bError) {
-                BString errorMessage = fromString("service method invocation failed: " + bError.getErrorMessage());
-                BError invocationError = ErrorCreator.createError(module, "ServiceExecutionError",
-                        errorMessage, bError, null);
-                balFuture.complete(invocationError);
-            }
-        }, args);
+        env.getRuntime().invokeMethodAsync(bSubscriberService, remoteFunctionName, null, metadata,
+                new SubscriberCallback(balFuture, module), args);
         return null;
     }
 }