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

Move EventService into HTTP layer #201

Closed
acoburn opened this issue Sep 17, 2018 · 6 comments
Closed

Move EventService into HTTP layer #201

acoburn opened this issue Sep 17, 2018 · 6 comments

Comments

@acoburn
Copy link
Member

acoburn commented Sep 17, 2018

Notification events are, at present, emitted from within the backend persistence layer, which means that each persistence implementation needs to implement its own logic for emitting notifications. By moving that logic into the HTTP layer, it should be possible to generalize that behavior. This change would also put the code into a good position for supporting WebSocket-based events as in #29.

Before implementing a particular solution here, it seems that the two most obvious paths for implementation would be to either:

  1. Change the mutating ResouceService methods so that they return a Stream<Event> (in this way, the persistence impl keeps track of the resources that were changed -- it gets more complicated w/ direct and indirect resources), and the HTTP layer simply emits events for whatever Events were returned by the ResourceService.
  2. Don't change the ResourceService interface at all, and just use the existing public methods to determine which resources changed.

I have a preference for the second option, provided that such a design is not overly cumbersome to implement (the first method would be pretty trivial to implement, but it would require all persistence layers to implement their own notification logic).

@acoburn acoburn added the area/http HTTP module label Sep 17, 2018
@acoburn acoburn added this to the 0.8.0 Release milestone Sep 17, 2018
@acoburn acoburn added this to To Do in Trellis Linked Data server via automation Sep 17, 2018
@ajs6f
Copy link
Member

ajs6f commented Sep 17, 2018

Huge -1 to the first choice. That doesn't really improve the situation: backends are still duplicating the same functionality over and over. +1 to the second-- it gets the job done, but are there missing cases? I'm not sure what you mean by "it gets more complicated w/ direct and indirect resources"…

@acoburn
Copy link
Member Author

acoburn commented Sep 17, 2018

The complications with direct/indirect containers is really just that it would likely involve an extra resource lookup, but under option 2, that shouldn't really be that complicated.

@acoburn
Copy link
Member Author

acoburn commented Sep 17, 2018

I'll plan to implement the second option.

@ajs6f
Copy link
Member

ajs6f commented Sep 17, 2018

👍

@acoburn acoburn self-assigned this Sep 17, 2018
acoburn added a commit that referenced this issue Sep 18, 2018
acoburn added a commit that referenced this issue Sep 18, 2018
@acoburn
Copy link
Member Author

acoburn commented Sep 18, 2018

With #204, there is no longer any particular need for the Session parameter on the mutating ResourceService methods: create, replace and delete. @ajs6f are you using the Session parameter in your ResourceService implementation methods? Or could we simplify the interface?

@ajs6f
Copy link
Member

ajs6f commented Sep 18, 2018

Nope-- I'm not even sure what I would do with it!

Trellis Linked Data server automation moved this from To Do to Done Sep 18, 2018
acoburn added a commit that referenced this issue Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

2 participants