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

Dynamic LDP containers #1151

Merged
merged 43 commits into from
Sep 25, 2023
Merged

Dynamic LDP containers #1151

merged 43 commits into from
Sep 25, 2023

Conversation

srosset81
Copy link
Contributor

@srosset81 srosset81 commented Sep 6, 2023

  • Use "catch-all" routes which points to a new LdpApiService, which itself redirects to LdpResourceService, LdpContainerService or ActivityPubCollectionService depending on the @type.
Method LDP resource LDP container ActivityStreams collection
GET * ldp.resource.get ldp.container.get activitypub.collection.get
POST * ldp.resource.post ldp.container.post (If defined by ControlledCollectionMixin)
PUT * ldp.resource.put - -
PATCH * ldp.resource.patch ldp.container.patch -
DELETE * ldp.resource.delete ldp.container.delete -
  • Allow to create and delete LDP containers via API (with an optional title and description). New tests have been added for that.
  • New resourcesWithContainerPath option for the LdpService. If true, the URI of all new resources will include the container path, otherwise it will use the root container path (default: true)
  • The auto-generated slug of ressource use the uuid/v4 library instead of MongoDB-like ObjectID. The previous method generated unique but easily guessable slugs.
  • Use node-fetch instead of supertest for all API tests.

Breaking changes ⚠️

  • The dereference option has been removed. Blank nodes are now automatically found (on 4 levels of depth).
  • The VoID endpoint doesn't return blank nodes anymore (the data provider will be updated to automatically find blank nodes)
  • The disassembly option has been removed. Use the new DisassemblyMixin, along with the ControlledContainerMixin, if you still need this feature (which will later be moved to the frontend)
  • The undocumented ResourceWatcher bot has been removed. Use ControlledContainerMixin instead.
  • The ldp.resource.get action now always return the semantic representation, even for binaries. The forceSemantic param is thus not necessary anymore. Binaries are handled by the LdpApiService. New tests have been added for that.
  • API routes with catchAll: true are always handled last. This requires a modification of Moleculer's ApiGateway (see this Moleculer issue: Improve routes priorization moleculerjs/moleculer-web#335). If you don't use SemApps CoreService, you must add this method to the service using the ApiGateway mixin or the catch-all route of the LDP service will be handled before other endpoints like VOID, SPARQL, etc.
  methods: {
    optimizeRouteOrder() {
      // Overwrite optimization method to put catchAll routes at the end
      this.routes.sort((a, b) => (a.opts.catchAll ? 1 : -1));
      this.aliases.sort((a, b) => (a.route.opts.catchAll ? 1 : -1));
    }
  }

@srosset81 srosset81 changed the base branch from master to next September 12, 2023 14:41
@srosset81 srosset81 changed the base branch from next to master September 15, 2023 15:04
@srosset81 srosset81 marked this pull request as ready for review September 19, 2023 10:00
@simonLouvet
Copy link
Contributor

hello @srosset81,
considering
The ldp.resource.get action now always return the semantic representation, even for binaries. The forceSemantic param is thus not necessary anymore. Binaries are handled by the LdpApiService. New tests have been added for that.
does the client have to request semantic data to obtain binary url?
does the location header return after post (201) is binary or semantic data url?

@srosset81
Copy link
Contributor Author

srosset81 commented Sep 21, 2023

When the frontend GET the binary through the API, it is returned as a binary. The change is only if you call ldp.resource.get from another Moleculer service.
No changes have been made about the Location header.

@srosset81 srosset81 changed the base branch from master to next September 25, 2023 12:47
@srosset81 srosset81 merged commit 849dc3f into next Sep 25, 2023
@srosset81 srosset81 deleted the ldp-catch-all-routes branch September 25, 2023 12:48
@srosset81 srosset81 mentioned this pull request Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants