Bidirectional events should come in as on<EventName> (#282)

* feat: Bidirectional events should come in as 'on<Method>'

* fix: Fix unit tests

Author: vladimir-rangelov

languages/javascript/src/shared/Events/index.mjs

@@ -145,8 +145,7 @@ const doListen = function(module, event, callback, context, once, internal=false
       if (Object.values(listeners.get(key)).length === 0) {
         const args = Object.assign({ listen: true }, context)
 
-        // TODO: Is subscriber -> notifer required to be a simple transform (drop 'on'?)
-        const subscriber = module + '.on' + event[0].toUpperCase() + event.substring(1)
+        const subscriber = module + '.' + event
         const notifier = module + '.' + event
 
         Gateway.subscribe(notifier, (params) => {
@@ -252,7 +251,7 @@ export const prioritize = function(...args) {
 const unsubscribe = (key, context) => {
   const [module, event] = key.split('.').slice(0, 2)
   const args = Object.assign({ listen: false }, context)
-  Gateway.request(module + '.on' + event[0].toUpperCase() + event.substr(1), args)
+  Gateway.request(module + '.' + event, args)
   Gateway.unsubscribe(`${module}.${event}`)
 }
 

languages/javascript/src/shared/Prop/index.mjs

@@ -21,7 +21,8 @@ function prop(
     if (immutable) {
       throw new Error('Cannot subscribe to an immutable property');
     }
-    return Events.listen(moduleName, key + 'Changed', ...Object.values(params), callbackOrValue);
+    const subscriber = 'on' + key[0].toUpperCase() + key.substring(1) + 'Changed'
+    return Events.listen(moduleName, subscriber, ...Object.values(params), callbackOrValue);
   } else if (type === 'setter') {
     // setter
     if (immutable) {

src/macrofier/engine.mjs

@@ -346,15 +346,6 @@ const rpcMethodsOrEmptyArray = compose(
 // Pick events out of the methods array
 const eventsOrEmptyArray = compose(
   option([]),
-  map(filter(validEvent)),
-  // Maintain the side effect of process.exit here if someone is violating the rules
-  map(map(e => {
-    if (!e.name.match(/on[A-Z]/)) {
-      console.error(`ERROR: ${e.name} method is tagged as an event, but does not match the pattern "on[A-Z]"`)
-      process.kill(process.pid) // Using process.kill so that other worspaces all exit (and don't bury this error w/ logs)
-    }
-    return e
-  })),
   map(filter(isPublicEventMethod)),
   getMethods
 )
@@ -390,15 +381,6 @@ const providedCapabilitiesOrEmptyArray = compose(
 // Pick providers out of the methods array
 const providersOrEmptyArray = compose(
   option([]),
-  map(filter(validEvent)),
-  // Maintain the side effect of process.exit here if someone is violating the rules
-  map(map(e => {
-    if (!e.name.match(/on[A-Z]/)) {
-      console.error(`ERROR: ${e.name} method is tagged as a provider, but does not match the pattern "on[A-Z]"`)
-      process.exit(1) // Non-zero exit since we don't want to continue. Useful for CI/CD pipelines.
-    }
-    return e
-  })),
   map(filter(isProviderInterfaceMethod)),
   getMethods
 )
@@ -420,7 +402,7 @@ const getModuleName = json => {
   return json ? (json.title || (json.info ? json.info.title : 'Unknown')) : 'Unknown'
 }
 
-const makeEventName = x => methodName(x)[2].toLowerCase() + methodName(x).substr(3) // onFooBar becomes fooBar
+const makeEventName = x => methodName(x) // onFooBar  remains onFooBar
 const makeProviderMethod = x => x.name["onRequest".length].toLowerCase() + x.name.substr("onRequest".length + 1) // onRequestChallenge becomes challenge
 
 const generateAggregateMacros = (platformApi, appApi, additional, templates, library) => {
@@ -1785,7 +1767,7 @@ function insertMethodMacros(template, methodObj, platformApi, appApi, templates,
     .replace(/\$\{method\.context\.count}/g, method.context ? method.context.length : 0)
     .replace(/\$\{method\.deprecation\}/g, deprecation)
     .replace(/\$\{method\.Name\}/g, method.name[0].toUpperCase() + method.name.substr(1))
-    .replace(/\$\{event\.name\}/g, method.name.toLowerCase()[2] + method.name.substr(3))
+    .replace(/\$\{event\.name\}/g, method.name)
     .replace(/\$\{event\.params\}/g, eventParams)
     .replace(/\$\{event\.params\.table\.rows\}/g, eventParamsRows)
     .replace(/\$\{if\.event\.params\}(.*?)\$\{end\.if\.event\.params\}/gms, event && event.params.length ? '$1' : '')

src/shared/modules.mjs

@@ -432,12 +432,12 @@ const eventDefaults = event => {
 const createNotifierFromProperty = (property, type='Changed') => {
 
   const notifier = JSON.parse(JSON.stringify(property))
-  notifier.name = methodRename(notifier, name => name + type)
+  notifier.name = methodRename(notifier, name => 'on' + name.charAt(0).toUpperCase() + name.substring(1) + type)
 
   Object.assign(notifier.tags.find(t => t.name.startsWith('property')), {
       name: 'notifier',
       'x-notifier-for': property.name,
-      'x-event': methodRename(notifier, name => 'on' + name.charAt(0).toUpperCase() + name.substring(1))
+      'x-event': notifier.name
   })
 
   notifier.params.push(notifier.result)
@@ -1110,13 +1110,12 @@ const generateEventSubscribers = json => {
       const tag = notifier.tags.find(tag => tag.name === 'notifier')
       // if there's an x-event extension, this denotes an editorially created subscriber
       if (!tag['x-event']) {
-          tag['x-event'] = methodRename(notifier, name => 'on' + name.charAt(0).toUpperCase() + name.substring(1))
+          tag['x-event'] = notifier.name
       }
-      const subscriber = json.methods.find(method => method.name === tag['x-event'])
+      const subscriber = json.methods.find(method => method.tags.find(t => t['x-notifier'] === notifier.name))
 
       if (!subscriber) {
           const subscriber = JSON.parse(JSON.stringify(notifier))
-          subscriber.name = methodRename(subscriber, name => 'on' + name.charAt(0).toUpperCase() + name.substring(1))
           subscriber.params.pop()
           subscriber.params.push({
               name: 'listen',
@@ -2085,7 +2084,6 @@ export {
     getEnums,
     getTypes,
     getEvents,
-    getPublicEvents,
     getSchemas,
     getParamsFromMethod,
     fireboltize,

test_sdk/openrpc/advanced.json

@@ -10,8 +10,7 @@
             "name": "plainEvent",
             "tags": [
                 {
-                    "name": "notifier",
-                    "x-event": "Advanced.onPlainEvent"
+                    "name": "notifier"
                 },
                 {
                     "name": "capabilities",
@@ -43,7 +42,7 @@
             ]
         },
         {
-            "name": "eventWithContext",
+            "name": "onEventWithContext",
             "tags": [
                 {
                     "name": "notifier",
@@ -96,11 +95,10 @@
             ]
         },
         {
-            "name": "eventWithTwoContext",
+            "name": "onEventWithTwoContext",
             "tags": [
                 {
-                    "name": "notifier",
-                    "x-event": "Advanced.onEventWithTwoContext"
+                    "name": "notifier"
                 },
                 {
                     "name": "capabilities",

test_sdk/suite/properties-context.test.js

@@ -114,13 +114,13 @@ test('Context Property set', () => {
 });
 
 test('Event with single context param', () => {
-    Advanced.listen("eventWithContext", "some-app", (data) => {
+    Advanced.listen("onEventWithContext", "some-app", (data) => {
         expect(contextSentToEvent).toBe(true)
     })
 })
 
 test('Event with two context params', () => {
-    Advanced.listen("eventWithTwoContext", "some-app", "inactive", (data) => {
+    Advanced.listen("onEventWithTwoContext", "some-app", "inactive", (data) => {
         expect(bothContextSentToEvent).toBe(true)
     })
 })

test_sdk/suite/properties.test.js

@@ -69,7 +69,7 @@ test('Basic Property subscribe', () => {
     let p = Simple.plainProperty(value => {
         expect(value.value).toBe("value 123")
     })
-    MockTransport.event("Simple","plainPropertyChanged",  "value 123");
+    MockTransport.event("Simple","onPlainPropertyChanged",  "value 123");
     return p;
 });
 
@@ -90,12 +90,12 @@ test('Basic Property set with null', () => {
     expect(propertySetterWasTriggeredWithValue).toBe(true)
 });
 
-//test listen to "plainPropertyChanged" event
+//test listen to "onPlainPropertyChanged" event
 test('Basic Property subscribe to event', () => {
-    Simple.clear("plainPropertyChanged");
-    let p = Simple.listen("plainPropertyChanged", value => {
+    Simple.clear("onPlainPropertyChanged");
+    let p = Simple.listen("onPlainPropertyChanged", value => {
         expect(value.value).toBe( "value 123")
     })
-    MockTransport.event("Simple","plainPropertyChanged",  "value 123");
+    MockTransport.event("Simple","onPlainPropertyChanged",  "value 123");
     return p;
 });