Skip to content

feat: import objects referenced using avro namespaces #246

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

Merged
merged 4 commits into from
Mar 1, 2022

Conversation

CameronRushton
Copy link
Member

Problem
When using the namespace field on an avro schema, the class will get put into the proper package, but the java package keyword wouldn't be used at the top of the class nor would it be imported in the generated application class.

Solution
When generating a schema, if there's a namespace, write that in as a package. When generating the application, look through the channels and find schemas that are going to be referenced. If they are in a different namespace, import them.

Design Notes
After adding the package name to the schema, all thats left is importing it in the main class. There are two ways to do this.

  1. use com.example.api.whatever.MyObject literally without importing it.
public Supplier<com.example.api.whatever.MyObject> testJobsOrderSupplier() {
		return () -> {
			// Add business logic here.
			return new com.example.api.whatever.MyObject();
		};
	}
  1. import com.example.api.whatever.MyObject and return MyObject in the code
import com.example.api.whatever.MyObject;
public Supplier<MyObject> testJobsOrderSupplier() {
		return () -> {
			// Add business logic here.
			return new MyObject();
		};
	}

I chose option 2 because the code is cleaner and the variable states are going to make more sense because the class name is MyObject - not com.example.api.whatever.MyObject.

As for how we know what to import, we might be able to collect the package names when we build our function specs, but this would mean that we must build the function specs first, then retrieve the info either from a separate variable or a new parameter in the function spec. The import is too loosely related to a function spec, in my opinion, so I made a completely independent function that looks though the channels for the necessary things to import.
I still moved the function spec initialization earlier in the process though.

Related issue(s)
Resolves #243

Comment on lines -189 to -197
stripPackageName(schemaName) {
// If there is a dot in the schema name, it's probably an Avro schema with a fully qualified name (including the namespace.)
const indexOfDot = schemaName.lastIndexOf('.');
if (indexOfDot > 0) {
return { className: schemaName.substring(indexOfDot + 1), javaPackage: schemaName.substring(0, indexOfDot) };
}
return { className: schemaName, javaPackage: undefined };
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to utility class since it's common for all.js and applicationModel.js. Could also be used in other places in the future.
Not sure whether or not it should live in scsLib.js since the comment in the file says, // This contains functions that are common to both the all.js filter and the post-process.js hook.

Copy link
Contributor

Choose a reason for hiding this comment

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

My intention with scsLib.js was to have a place for code that could be shared between the filters (all.js) and any non-filters such as the pre and post hooks. So it would be fine to put that in scsLib.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I'll move it there, thanks!

@@ -40,7 +40,7 @@
"@semantic-release/npm": "^7.1.3",
"@semantic-release/release-notes-generator": "^9.0.1",
"conventional-changelog-conventionalcommits": "^4.6.0",
"eslint": "^6.8.0",
"eslint": "^7.32.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

Not the most up-to-date version. I just wanted support for optional chaining so it isn't upset when I do ?. which is cleaner than nested conditionals.

Comment on lines 134 to 136
"

import java.util.function.Consumer;
Copy link
Member Author

Choose a reason for hiding this comment

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

Some snapshots are fine, some have an extra space now. I can't be bothered to endlessly fiddle with spacing for every use case. One extra space sometimes is within my tolerance. Let me know if you think we must make the spacing perfect.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see no need to be picky here. It's better to make it easy to work with.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 1, 2022

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
0.0% 0.0% Duplication

@CameronRushton CameronRushton merged commit 357cde4 into asyncapi:master Mar 1, 2022
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Bad references to generated objects in main Application
3 participants