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

feat!: enable usage of generated app as a library without its code modification #220

Merged
merged 17 commits into from
Oct 3, 2023

Conversation

kaushik-rishi
Copy link
Collaborator

Related issue(s)
#219

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

kaushik-rishi pushed a commit to kaushik-rishi/nodejs-template that referenced this pull request Aug 23, 2023
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

you should also modify template/src/api/index.js file and at the end of the file add exports.app = app;

additionally please extend the readme, by adding a new section, with h3 under the CLI section called Adding custom code and:

  • write explanation that it is recommended to not modify the code of generated application but rather use it as an API to start a server and register additional custom code
  • provide this example code
// You require the generated server. Running this code starts the server
// App exposes API to send messages
const { app } = require('./output/src/api/index');
// Requiring generated handler that you should use if you want to react on consumed messages
const receiveLightMeasurementHandler = require('./output/src/api/handlers/smartylighting-streetlights-1-0-event-{streetlightId}-lighting-measured');
// Requiring generated handler that you should use if you want to react on produced messages
const sendDimLightHandler = require('./output/src/api/handlers/smartylighting-streetlights-1-0-action-{streetlightId}-dim');

/**
 *
 * 
 * Example of how to work with generated code as a consumer
 *
 * 
*/

// Writing your custom logic that should be triggered when your app receives as message from a given channel
function triggeredByLightMeasurementHandler(message) {
  console.log('reading message in triggeredByLightMeasurementHandler', message.payload);

  console.log('sending another message in triggeredByLightMeasurementHandler to another channel');
  app.send({percentage: 1}, {}, 'smartylighting/streetlights/1/0/action/1/dim');   
}

// Registering your custom logic in a channel-specific handler
// triggeredByLightMeasurementHandler function is called once the app gets message sent to the channel like:
// "mqtt pub -t 'smartylighting/streetlights/1/0/event/123/lighting/measured' -h 'test.mosquitto.org' -m '{"id": 1, "lumens": 3, "sentAt": "2017-06-07T12:34:32.000Z"}'"
receiveLightMeasurementHandler.registerMiddleware(triggeredByLightMeasurementHandler);

/**
 * 
 * 
 * Example of how to process a message before it is sent to the broker
 * 
 * 
 */

// Writing your custom logic that should be triggered when your app receives as message from a given channel
function triggeredByDimLight(message) {
  console.log('reading message in triggeredByDimLight before it is sent to the broker', message.payload.percentage);
}

// Registering your custom logic in a channel-specific handler
// triggeredByDimLight function is called once the app sends a message to the channel
// For example "triggeredByLightMeasurementHandler" sends a message to some channel using "app.send" and before it is sent, you want to perform some other actions
sendDimLightHandler.registerMiddleware(triggeredByDimLight);

/**
 * 
 * 
 * Example of how to produce a message using API of generated app independently from the handlers
 * 
 * 
*/

(function myLoop (i) {
  setTimeout(() => {
    console.log('producing custom message');
    app.send({percentage: 1}, {}, 'smartylighting/streetlights/1/0/action/1/turn/on');   
    if (--i) myLoop(i);
  }, 1000);
}(3));
  • explain that anyone can run this code and test by sending a custom message mqtt pub -t 'smartylighting/streetlights/1/0/event/123/lighting/measured' -h 'test.mosquitto.org' -m '{"id": 1, "lumens": 3, "sentAt": "2017-06-07T12:34:32.000Z"}'

last but not least change the PR title to feat!: as this is going to be a breaking change and we need to release it as new major

also call it like feat!: make it possible to use generated app without its code modification -> title should say not want you did but what it changed

you could also extend the description of the PR with the example code sample to showcase what is the change

template/src/api/handlers/$$channel$$.js Show resolved Hide resolved
@kaushik-rishi kaushik-rishi changed the title feat: refactoring handlers to separate from generated code feat!: enable usage of generated app as a library without its code modification. Aug 23, 2023
@kaushik-rishi
Copy link
Collaborator Author

@derberg can you please review 🙂 ?

@kaushik-rishi
Copy link
Collaborator Author

@derberg
The end goal is to use it as a library like require("nodejs-my-template").
In such a case, it doesn't make any sense to import the handlers from their respective files like require('./output/src/api/handlers/smartylighting-streetlights-1-0-action-{streetlightId}-dim');

🤔 Thinking from a user point of view, I'd rather be happy if I can access all the handlers by just importing the library, The ideal end usage should look something like this

const { client } = require("./server-library")

client.init();
client.subscribeToSmartylightingStreetlights10EventLightingMeasuredHandler.registerMiddleware((messagge) => {
    console.log("hit the subscribe payload", messagge.payload);
})

the code should ideally look more shorter, if not for the long name smartylighting...blablabla 😄

@derberg
Copy link
Member

derberg commented Aug 30, 2023

the code should ideally look more shorter, if not for the long name smartylighting

yeah, in both cases, path or function name, the name is crap 😄 but afaik it is because we do not have explicitly a channel name added to https://bit.ly/asyncapi

Thinking from a user point of view, I'd rather be happy if I can access all the handlers by just importing the library, The ideal end usage should look something like this

yes, eventually this is what we should target:

  • see how to make it easy to just require as library
  • generate client too, so we do not have to do app.send({percentage: 1}, {}, 'smartylighting/streetlights/1/0/action/1/dim'); but client.sendDimLight({percentage: 1}) or similar

thing is that these improvements ideas popped up only because we started this refactoring to enable easier testing. So I suggest you just create an issue so we do not forget:

  1. get followup and name it as ideas for next major
  2. merge this PR as v1
  3. introduce tests
  4. rewrite to react
  5. rewrite to use new Parser API
  6. rewrite to use AsyncAPI v3
  7. look into point 1

derberg
derberg previously approved these changes Aug 30, 2023
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

Lemme know if you agree with suggested plan, and I merge

@kaushik-rishi
Copy link
Collaborator Author

Sure, will create the issue.
There's an edge case i just found out, for which this refactor doesn't hold good.

If there's a route which has both the publish and subscribe operations on a single channel/route, the middlewares and registerMiddleware functions will be the same for both the publish and subscribe methods.

Example AsyncAPI File -> https://github.com/kaushik-rishi/templates-collection-nodejs-templates/blob/master/mqtt-pub-sub-single-route-new-template/asyncapi.yaml

Example generated code -> https://github.com/kaushik-rishi/templates-collection-nodejs-templates/blob/master/mqtt-pub-sub-single-route-new-template/src/api/handlers/smartylighting-streetlights-1-0-action-%7BstreetlightId%7D-turn-on.js

@derberg
Copy link
Member

derberg commented Aug 30, 2023

you also have to update snapshots for tests: npm run test:updateSnapshot

and yeah, good catch about pub/sub
just make sure const middlewares = []; is unique per operation, simply make sure there are 2 separate arrays

@kaushik-rishi
Copy link
Collaborator Author

Sure :), the arrays and the functions should be different as well.
About the tests, I will update them 👍

@kaushik-rishi
Copy link
Collaborator Author

I’ve updated the docs with the new way to use the library/template. Currently i feel using operationId for the function names is fine, in the path forward i’d like to move the operationId into using the channel’s name in pascal / camel case or whatever, but i’d like to do it after we start rewriting the templates to new react renderer template.
Also updated snapshot tests

Path forward

  • get the first regression test up and running integrated with github actions
  • start rewrite to reactjs

@derberg

@derberg derberg changed the title feat!: enable usage of generated app as a library without its code modification. feat!: enable usage of generated app as a library without its code modification Sep 25, 2023
README.md Outdated Show resolved Hide resolved
@kaushik-rishi
Copy link
Collaborator Author

@derberg ready to merge 🙂

derberg
derberg previously approved these changes Oct 2, 2023
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

well done 👏🏼

@derberg
Copy link
Member

derberg commented Oct 2, 2023

@sonarcloud
Copy link

sonarcloud bot commented Oct 2, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@kaushik-rishi
Copy link
Collaborator Author

@derberg oops! fixed it.

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

WELL DONE 👏🏼

@derberg
Copy link
Member

derberg commented Oct 3, 2023

/rtm

@asyncapi-bot asyncapi-bot merged commit c120307 into asyncapi:master Oct 3, 2023
11 checks passed
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@derberg
Copy link
Member

derberg commented Oct 3, 2023

@allcontributors please add @kaushik-rishi for code,docs,test

@allcontributors
Copy link
Contributor

@derberg

I've put up a pull request to add @kaushik-rishi! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants