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

Deprecates the StructuredService framework and provides a replacement using plain Controllers. #855

Merged
merged 14 commits into from
Jun 21, 2021

Conversation

andyHa
Copy link
Member

@andyHa andyHa commented Jun 18, 2021

This contains some internal breaking changes as i.e. the API of Intereptor has changed.

Public API Explorer

Also, we now detect and show all recognized public APIs:

Bildschirmfoto 2021-06-17 um 18 19 56

Bildschirmfoto 2021-06-17 um 22 50 44

Migrating Services

Migrating from StructuredService(s) to the new API is as simple as moving the call(..) method into a Controller and replacing the ServiceCall parameter by WebContext and the StructuredOutput by either JSONStructuredOutput or XMLStructuredOutput (depending on the format). Finally either a @InternalService or @publicservice annotation has to be added to the method.

andyHa added 8 commits June 18, 2021 16:00

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Using the annotations @publicservice or @InternalService, one can define
that a route is considered a service call with a certain response format.

Previously this was done using Routed.jsonCall. We now use extra
annotations to support XML as additional format.

Using @publicservice also provides the benefit of self-documentation
as these services are listed in the Public API explorer.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
If the response is already committed, we do not need to care about
the HTTP code, we simply have to submit an error so that the underlying
channel is closed.

Also, if the response for a service call was already commited as the
internal buffer is full, we can no longer report and error. This
(edge) case is now logged at least to support debugging.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@andyHa andyHa added the 🧬 Enhancement Contains new features label Jun 18, 2021
Copy link
Contributor

@qw3ry qw3ry left a comment

Choose a reason for hiding this comment

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

Build fails

*/
public List<PublicApiInfo> getApis() {
synchronized (apis) {
return new ArrayList<>(apis);
Copy link
Contributor

Choose a reason for hiding this comment

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

unmodifiableList?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to copy because of threading issues - the copy doesn't need to be unmodifyable as it is unused elsewhere..

if (apiInfo == null) {
apiInfo = buildApiInfo(publicService.apiName(), route);
apis.add(apiInfo);
apis.sort(Comparator.comparing(PublicApiInfo::getPriority).thenComparing(PublicApiInfo::getApiName));
Copy link
Contributor

Choose a reason for hiding this comment

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

If you'd make apis a TreeSet it takes care of the order by itself, but it's fine like that as well

Watch w = Watch.start();

CommandParser parser = new CommandParser(webContext.require("command").asString());
if (Strings.isEmpty(parser.parseCommand())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Parse once

Copy link
Member Author

Choose a reason for hiding this comment

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

parse command is a noop the 2nd and 3rd call...

Copy link
Member

@sabieber sabieber left a comment

Choose a reason for hiding this comment

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

Build seems to fail because of the WebServerSpec

@andyHa andyHa added the 💣 BREAKING CHANGE Contains non-backwards compatible changes to public methods or changes the behavior of existing code label Jun 21, 2021
andyHa added 2 commits June 21, 2021 09:34

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Contributor

@mkeckmkeck mkeckmkeck left a comment

Choose a reason for hiding this comment

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

2 typos found

andyHa and others added 4 commits June 21, 2021 11:37

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Co-authored-by: Matthias Keck <60612914+mkeckmkeck@users.noreply.github.com>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Co-authored-by: Matthias Keck <60612914+mkeckmkeck@users.noreply.github.com>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@andyHa andyHa merged commit 30fd127 into develop Jun 21, 2021
@andyHa andyHa deleted the feature/aha/grande-refactoring6 branch June 21, 2021 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💣 BREAKING CHANGE Contains non-backwards compatible changes to public methods or changes the behavior of existing code 🧬 Enhancement Contains new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants