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

Validators for JSON and Graphql Exporter #1833

Merged
merged 2 commits into from Feb 12, 2021
Merged

Conversation

rishi-aga
Copy link
Collaborator

Validators for JSON and Graphql Exporter

License

I confirm that this contribution is made under an Apache 2.0 license and that I have the authority necessary to make this contribution on behalf of its copyright owner.

@rishi-aga rishi-aga force-pushed the tableExportValidator branch 3 times, most recently from e6743cc to 3ae6b3d Compare February 12, 2021 15:41
Base automatically changed from CARBON-519 to master February 12, 2021 16:03

private static final Validator INSTANCE = new NoRelationshipsProjectionValidator();

public static Validator getInstance() {
Copy link
Member

Choose a reason for hiding this comment

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

Let's not use singletons.

}

@Test
public void testProcessMultipleQuery() {
Copy link
Member

Choose a reason for hiding this comment

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

How is this test different than the one above it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this one has 2 queries with single projection and other one has single query with more than one projection

@@ -41,7 +41,8 @@

public GraphQLTableExportOperation(TableExportFormatter formatter, AsyncExecutorService service,
AsyncAPI export, RequestScope scope, ResultStorageEngine engine) {
super(formatter, service, export, scope, engine);
super(formatter, service, export, scope, engine,
Arrays.asList(NoRelationshipsProjectionValidator.getInstance()));
Copy link
Member

Choose a reason for hiding this comment

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

It seems like both need the same validators. Should we move the NoRelationshipsProjectionValdator to the super class list?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Single SingleRootProjectionValidator is must as I am taking first one from collection in call method, so kept it in superclass. NoRelationshipsProjectionValdator could be optional for any new subclass.

@aklish aklish merged commit 66be9ae into master Feb 12, 2021
@aklish aklish deleted the tableExportValidator branch February 12, 2021 17:49
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.

None yet

2 participants