Skip to content
This repository has been archived by the owner on Aug 25, 2023. It is now read-only.

Final refactor #31

Closed
5 of 18 tasks
michielbdejong opened this issue Apr 25, 2019 · 5 comments
Closed
5 of 18 tasks

Final refactor #31

michielbdejong opened this issue Apr 25, 2019 · 5 comments

Comments

@michielbdejong
Copy link
Contributor

michielbdejong commented Apr 25, 2019

There have been quite a few requests for refactoring the code of this npm module in various ways. In this ticket I'll try to collect them and merge them into a single 'final refactor' plan, after which we can hopefully close those discussions in a way that everybody's happy and excited about. Here's the list, I'll edit this as we collect more:

Documentation requests:

Refactor requests:

Feature requests (beyond the Solid v0.7 spec):

  • search within pod -> I'll create a separate module that hooks into the change event.
  • search across pods -> I think this one is out of scope
  • caching local ACL/profile docs -> we could do a write-through cache as a performance tweak
  • caching remote ACL/profile docs -> maybe refresh-after-read could work here?
  • Memento support, see Add Memento support for resource versioning #32
  • support for LDN outgoing webhooks, see Feature request to support LDN #33
  • an RDF API for logging in, creating accounts, etc., and a front-end UI (a completely static client-side Solid app) that uses that API. That way, different servers can use different login and management apps. (extracted from Documentation of module context #36 (comment))
  • input validation, see Input validation #43
  • Direct container support?
@RubenVerborgh
Copy link

RubenVerborgh commented Apr 26, 2019

Hey, it's me again, devil's advocate 🙂

Small remark: I don't think they have to be separate npm modules. It might very well make sense to have them all in one npm module. What does matter is their architectural separation and connection.

And their being separate as npm modules does not mean they are separate GitHub projects.

  • use LDN inbox system instead of Node-native events

For what types of events? Internally?
I'd be extremely cautious about this, and we need to specify exactly what we mean here.

  • search within pod -> I'll create a separate module that hooks into the change event.

  • search across pods -> I think this one is out of scope

I don't think we should be looking into solutions for these actually.

The question is: does the architecture support adding these?
Because there is (in my mind) no doubt about that it should.
Whether this is implemented now and/or by us is something else.

  • caching local ACL/profile docs -> we could do a write-through cache as a performance tweak

  • caching remote ACL/profile docs -> maybe refresh-after-read could work here?

Also a question of arch for me.

So here's a very strong plea to keep the discussion on a level of questions and architecture, not (yet) solutions.

@dsebastien
Copy link

Regarding the architecture (I suppose that this refactoring is still related to architecture at this point), I'd like to chime in again with solid/user-stories#22.

I feel like it could be an enabler for a series of uses cases that are already part of the user-stories repository (I've linked those to that issue), but most probably for many others that we haven't thought of just yet.

I've mentioned a bit of how it could fit in the system's layering here: #30 (comment)

@michielbdejong
Copy link
Contributor Author

@dsebastien your point of "keeping track of the whole history" is documented in #32. I like how you link that to the outgoing webhooks idea which is documented in #33 - you and @pmcb55 seem to have a lot of ideas in common! :)

So consider both requests documented, we'll prioritise this list and implement all the winning refactor proposals, probably in about a month from now.

@michielbdejong
Copy link
Contributor Author

We scoped this out in the big V-Next architecture meeting of 13 May. We made the following concessions we made to the architectural committee:

  • I will make diagrams for smaller components
  • I will make the code follow Ruben's UML, or change the UML to follow the code
  • I will write a proxy server for Kjetil
  • I will add an extra unused option to not add resources in slash-determined parent container for Pat
  • I will refactor executeTask to be middle-ware style using an array of operation handlers (see 3. below)

In exchange for that, we got sign-off on the following seven statements:

  1. The pod server should pass the solid test-suite
  2. We should (in an extensible way) implement the current spec. This means a pod server is primarily a read-write web server, not a triple store.
  3. The processing of various http verbs should be organized middle-ware style using an array of operation handlers.
  4. The RDF functionality should be a filtering layer, not a queriable service (done in ACL component should access Storage through RDF component, not directly #78)
  5. The auth functionality should be a queriable service, not a filtering layer (but see above, I will write the proxy server in a separate repo)
  6. The auth and RDF modules should be objects containing config and (through a decorator pattern) caching, not stateless utility functions. Done for the RdfFetcher, will do for auth+caching)
  7. The slashes in resource IRIs refer to a tree structure (this has already helped to make the code a lot more readable! now using Node's URL class a lot, and a function urlToPath to go from URLs to storage paths.)

@RubenVerborgh
Copy link

7\. The slashes in resource IRIs refer to a tree structure

But at what point in the architecture? All the way through, or only at storage level?

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

No branches or pull requests

3 participants