Skip to content

Java: Docs for improved event handler signatures #1944

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 8 commits into from
Jul 7, 2025

Conversation

beckermarc
Copy link
Contributor

@beckermarc beckermarc commented Jun 30, 2025

  • Arbitrary return types
  • Service arguments
  • Entity reference arguments

github-actions[bot]

This comment was marked as outdated.

beckermarc and others added 2 commits June 30, 2025 15:54
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@vmikhailenko
Copy link
Contributor

Whole statements are left for generic handlers?


The CAP Java SDK Maven Plugin can generate query builder interfaces for entities defined in the CDS model. These interfaces allow for a [type-safe query building](../working-with-cql/query-api#concepts) and can be used in arguments as well:

```java
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd omit this option as the entity argument is redundant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This matches the documentation structure that we also when describing the same concept for events based on contexts and entities based on data accessor interfaces. I'd keep it symmetrical for now.

@vmikhailenko
Copy link
Contributor

Maybe we should also add some kind of "when-to-use-what" tips or guidelines: e.g. prefer tight and typed signatures for the handlers first, switch to the more generic versions if you really want to write a generic handler.

E.g. Books_ ref, CatalogService service is better because [reason], the Books book, Books_ ref, CatalogService service also possible if I want to have payload and the ref together.

@beckermarc
Copy link
Contributor Author

Whole statements are left for generic handlers?

There is actually not really a great use case for these so far, so I'd keep that a hidden feature for now. The reference has proven much more useful.

@beckermarc
Copy link
Contributor Author

Maybe we should also add some kind of "when-to-use-what" tips or guidelines: e.g. prefer tight and typed signatures for the handlers first, switch to the more generic versions if you really want to write a generic handler.

That's actually not so easy and highly depends on the logic that is within the event handler method I think. I guess we still need to learn from some good examples for these new possibilities to be able to document something here.

@vmikhailenko
Copy link
Contributor

Maybe we should also add some kind of "when-to-use-what" tips or guidelines: e.g. prefer tight and typed signatures for the handlers first, switch to the more generic versions if you really want to write a generic handler.

That's actually not so easy and highly depends on the logic that is within the event handler method I think. I guess we still need to learn from some good examples for these new possibilities to be able to document something here.

That is exactly why "when-to-use-what" is beneficial. Hard choice.

@vmikhailenko
Copy link
Contributor

Maybe also mention that event handler returning a collection of entities will lead to broken result in case of queries with $count. I remember more than one issue about this. This is very hard to figure that out without documentation. Or we should fix it for good.

@beckermarc
Copy link
Contributor Author

Maybe also mention that event handler returning a collection of entities will lead to broken result in case of queries with $count. I remember more than one issue about this. This is very hard to figure that out without documentation. Or we should fix it for good.

That's something that is mentioned here: https://pages.github.tools.sap/cap/docs/java/cqn-services/application-services#read-result. Which is also somehow linked from this page. I will improve the link however to link to the upper section instead.

@vmikhailenko
Copy link
Contributor

Whole statements are left for generic handlers?

There is actually not really a great use case for these so far, so I'd keep that a hidden feature for now. The reference has proven much more useful.

Extend the statement to fetch something in addition?

Why hide it? It will be very strange later if we decide to use it somewhere in the samples or as a recommendation.

@beckermarc
Copy link
Contributor Author

Extend the statement to fetch something in addition?

Why hide it? It will be very strange later if we decide to use it somewhere in the samples or as a recommendation.

You can always get it from the context as well. There are rare scenarios where you would benefit from not using the context, because you are registering on multiple events. The main reason I don't want to add it now, is that I can't give good reasons when to use it. We can always add it later on, if we find great use cases for that in our samples.

@vmikhailenko
Copy link
Contributor

Extend the statement to fetch something in addition?

Why hide it? It will be very strange later if we decide to use it somewhere in the samples or as a recommendation.

You can always get it from the context as well. There are rare scenarios where you would benefit from not using the context, because you are registering on multiple events. The main reason I don't want to add it now, is that I can't give good reasons when to use it. We can always add it later on, if we find great use cases for that in our samples.

Why not mention this as a technical possibility then? Not blocking anything, I just remember me being confused that I have no reference point to give to stakeholder.

@beckermarc
Copy link
Contributor Author

Why not mention this as a technical possibility then?

To me it doesn't feel right to promote this feature at the moment, because it doesn't really fill a gap, as it:
a) Doesn't provide the possibility to get a typed object (like for data, reference and service arguments)
b) Somehow is going into the direction of getting event context arguments, which we however don't support for anything else.

Obtaining statements from the context is as good as getting them into the event handler directly (even better, as from the context we can derive event names). That's why I am hesitant to document and promote it.

@vmikhailenko
Copy link
Contributor

Maybe also mention that event handler returning a collection of entities will lead to broken result in case of queries with $count. I remember more than one issue about this. This is very hard to figure that out without documentation. Or we should fix it for good.

That's something that is mentioned here: https://pages.github.tools.sap/cap/docs/java/cqn-services/application-services#read-result. Which is also somehow linked from this page. I will improve the link however to link to the upper section instead.

Maybe a warning then? It is very unexpected behaviour, especially in @After handler.
Non-blocker, again, just an ask.

@beckermarc
Copy link
Contributor Author

Maybe a warning then? It is very unexpected behaviour, especially in @after handler.
Non-blocker, again, just an ask.

We can make the text in the application service section more explicit and add a warning to the count docs there I think.

@beckermarc beckermarc enabled auto-merge July 7, 2025 06:31
@beckermarc beckermarc disabled auto-merge July 7, 2025 06:31
@renejeglinsky renejeglinsky merged commit f7a04e6 into main Jul 7, 2025
4 checks passed
@renejeglinsky renejeglinsky deleted the improved-event-handler-signatures branch July 7, 2025 13:40
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.

5 participants