Delete scopes

[fiz-deliverect]

This PR introduces a new endpoint to delete scope entries.

We needed a way for mifes to use different versions of shared dependencies, so naturally we thought scopes could solve this. Our pipelines are setup to call the import-map-deployer to update the import map whenever our mifes release a new version. We added support for scopes in our pipelines recently; it looks something like this:

pipeline interface

mife-release:
  name: mife-a
  ...
  scopes:
      shared-dep: http://cdn.com/shared-dep@21

This will update the "imports.mife-a" key in standard fashion; but will now also add a scope entry:

{
  ...
  scopes: {
    ...,
    "/mife-a/x.y.z/": {
       "shared-dep": "https://cdn.com/shared-dep@21"
    },
  }
}

Our mifes are hosted with the convention of having the mife name + version in the path (e.g. http://cdn.com/mife-a/1.0.0/mife-a.js). Assuming no other mife/shared-dep has that directory in there url, this ends up working nicely for scoping dependencies to specific releases of microfrontends. Every release adds a scope if its in the config. If no scope is in the ci/cd config, then a new scope entry won't be added and that version would go back to using the top level shared dependency. But now we have old scopes in there that aren't being used. This endpoint gives us a way to delete those, manually or through ci/cd.

Note: Given that our mifes name is in the path of where its hosted, we had originally thought of just using the mife name in the scope key (e.g. "/mife-a/"); but this had a major flaw: If the mife wants to go back to using the top level shared dependency, there was no way to delete the scope key, which would result in the mife being forced to using the scope entry. The addition of this endpoint doesn't actually solve this fully because this endpoint is a separate transaction, which would result in a moment of time where the mife would still use the scoped dependency until its deleted in the next transaction. If we'd want to support the mife name scope key, we'd need to add support for deleting scopes in the PATCH /import-map.json endpoint or the PATCH /services endpoint.

Note: This PR seemingly addresses #138. Not sure what there use case is though.

[internettrans]

I'm not sure that regexString is my preferred approach for this, since regular expressions are language-dependent and so code calling the endpoint might not function as expected if the regular expression string is generated from a language other than javascript. Also because deleting multiple resources simultaneously from a RESTful endpoint is unusual, and regular expressions can easily contain errors that result in matching all or far too many of the scopes.

How many scopes is common in your usage of import-map-deployer? I think I'd prefer deleting just one at a time at the API level and expecting multiple requests when deleting multiple scopes.

[internettrans]

Does the order of the keys matter in import map scopes?

[fiz-deliverect]

No they don't but I wanted to respect the alphabetical order config; which applies ordering to scopes on PATCH /import-map.json and PATCH /services

[internettrans]

Why is the regexString being sent in the request body rather than encoded in the url path? I first expected to see DELETE /scopes/:urlEncodedScope as the API rather than a request body. I've never used request bodies with the HTTP DELETE method before, and some sources (including MDN) suggest that DELETE requests shouldn't have a request body.

[fiz-deliverect]

Agree with you that it's definitely not normal; but then yeah it requires url encoding as you mentioned... Semi-annoying to type if you want to make manual calls to the API (encoding the regex). I'm good with both approaches (assuming we want to do regex).

[internettrans]

In what situation would the request body not be a string? Is this if statement for when there is an empty request body? As far as I can tell, request bodies are always parsed as a string in this project, due to the line of code below.

import-map-deployer/src/web-server.js

Line 38 in b6c0ca7

app.use(bodyParser.text({ type: "*/*" }));

[fiz-deliverect]

Is this if statement for when there is an empty request body?

Yes! In that case, req.body is an empty object. I didn't want to mess around with the middleware; but that oddity is making the code hard to reason about.

[internettrans]

This endpoint should send the same response data regardless of whether the dry run query parameter is provided, but it doesn't. This should send the full import map to match when the query parameter is missing

[internettrans]

scopesToBeDeleted should be calculated in the same place in the code, to avoid the endpoint behaving differently depending on whether the dry run query parameter is present. Creating a function within modify.js to calculate scopesToBeDeleted would be best, so it can be reused within deleteScopes and by the code here.

[internettrans]

Please add a test where the scopes are fully qualified urls

[fiz-deliverect]

@jolyndenning

There wouldn't be many scopes if I have a DELETE /scopes endpoint; because my pipelines (which run per mife) would call this endpoint to delete the previous version after a succesful call to PATCH /import-map.json to update the mife url and the scope (/mife-a/next-version/). Using regex makes that easier since my shared pipeline logic could delete any previous versions of a given mife name (regex: delete scopes with /mife-x/{version} in string with any version). I could still achieve this without regex, but would then require: GET /import-map.json -> parse json -> regex to find the key -> DELETE /scope.

I'm leaning towards your intuition after hearing your views: url encoded exact match of the individual scope you want to delete... but my opinions are fickle with these sortve things

As you mentioned, individual deletions are much safer against silly mistakes

[fiz-deliverect]

@jolyndenning I have another idea for supporting deletion of scopes that I think is interesting:

What if on PATCH /import-map.json we look for a prefix on keys e.g. '~' that marks them for deletion.

{
  "scopes": {
    "~/mife-a/": {....}
  },
}

This "mark for deletion" logic could be applied to all keys in the json, which would then add support for deleting imports (services) and integrity(s), which I believe is also handy. Most importantly, this allows for upserting+deleting in the same transaction. This allows me to have a simple mife key (unversioned) where my ci/cd could delete the scope + update the import in the case the mife is ready to go back to the top-level import.

edit: using a null value (against the key) would be much better instead of a deletion char.