Skip to content

ResourceMapper transition actions #946

Closed

Description

@RubenVerborgh

Context

We are in the process of changing the URL-to-filename correspondence within node-solid-server, such that it conforms to https://www.w3.org/DesignIssues/HTTPFilenameMapping.html. Importantly, this fixes the current Content-Type breakage we have (#925 and others).
As an example:

  • v4.0.0 will map file:///solid/profile/card to https://pod.example/profile/card with content-type text/turtle
  • in v5.0.0, the file on disk will be file:///solid/profile/card$.ttl instead

This transition is quite complex, since the URL-to-filename correspondence is implemented in multiple places instead of just one. Furthermore, this is a semver.major change.

This issue tracks all steps that need to be taken.

Steps

  1. Encapsulate the current filesystem mapping in one component (Fix .acl lookup for paths with spaces #660)
  2. Replace all ad-hoc mapping code with the single mapping component (Wire up LegacyResourceMapper #661)
  3. Evaluate whether everything still works with the single component, fix where needed (Test LegacyResourceMapper in production #947)
  4. Encapsulate the new filesystem mapping in one component (Add ResourceMapper class for URL/file mapping #643)
  5. Replace all mentions of the old mapping component with the new mapping component (Wire up ResourceMapper #662)
  6. Update the new data pod template filenames to use the new mapping (Update new pod template filenames to follow ResourceMapper mappings #948)
  7. Write a tool to migrate old Solid filesystems to new Solid filesystems (Create migration script for filesystems from LegacyResourceMapper to ResourceMapper #949)
  8. Upgrade the software in all of our services and migrate the filesystems (Upgrade public Solid servers software to v5 and migrate filesystems #950)
  9. Remove LegacyResourceMapper and tests
  10. Document upgrade notes (Write release notes for v5.0.0 #951)

Activity

  1. added this to the 5.0.0 milestone on Nov 19, 2018
  2. melvincarvalho commented on Nov 20, 2018

    @melvincarvalho
    Contributor

    Great work!

    A couple of things.

    • Would it be possible to make this a switch, or behind a flag, or config option. That will enable smoother upgrade, easier testing, and more flexibility for pod owners.

    • It seems like quite a big change, especially in terms of technical debt. ie requires file name changes. Could I request that we put this functionally live in a version after 5.0.0, so that it possible to get the stability release and quota work, before requiring the file system changes.

    Perhaps this is a good topic for a new issue, or the community group. But I have often wondered why the header information (Content-Type) could not be put in the .meta file. That would seem to be a more portable solution (across, different versions, different solid language servers, different free software cloud solutions and even different web servers).

  3. kjetilk commented on Nov 20, 2018

    @kjetilk
    Member

    Would it be possible to make this a switch, or behind a flag, or config option. That will enable smoother upgrade, easier testing, and more flexibility for pod owners.

    I don't think we should do that. We need to move forward, and such a switch would require even more complexity. Such flexibility comes at a significant cost.

    You know, this is exactly the kind of minimal bug fixing and refactoring you've called for, @melvincarvalho . It too has consequences,

    It seems like quite a big change, especially in terms of technical debt. ie requires file name changes. Could I request that we put this functionally live in a version after 5.0.0, so that it possible to get the stability release and quota work, before requiring the file system changes.

    Actually, the stability and quota work was originally intended for 4.4, and I would still like to release it as 4.4. The problem with the quota work is that the current code afforded very little support for it (e.g., there is no way you can persist the current disk usage in memory in a reliable manner), so it is essentially a hack that doesn't scale. Therefore, I'm reluctant to champion the merge myself, and there is very little time left.

    I think we should release it as part of a 4.4 release right now, but we need go forward with 5.0.0 as that code base is holding us back in a very destructive manner.

    Perhaps this is a good topic for a new issue, or the community group. But I have often wondered why the header information (Content-Type) could not be put in the .meta file.

    Yeah, please open an issue for that.

  4. melvincarvalho commented on Nov 20, 2018

    @melvincarvalho
    Contributor

    Actually, the stability and quota work was originally intended for 4.4, and I would still like to release it as 4.4

    @kjetilk thanks for the response. Sounds good!

    With my "solid.community" hat on, I can commit to upgrading to 4.4

    I'll push back on 5.0.0 at this point, if it involves renaming files in every pod, and no switch, which entails being locked in to technical debt that we may not necessarily want.

    Reason being that we'd like to keep compatibility with heterogeneous systems (such as NextCloud, apache, nginx, gold, ldphp, remotestorage etc.). Also what do we do with content addressable files such as git or ni: or did:.

    There seems to be a number of ways to do this. For example, putting headers in the .meta, or even the .acl files, which could extend to things we've wanted in the past, such as to record the user that created a file. This models the UNIX idea of a file and an inode with meta data. Another way, remote storage for example uses redis to solve this in a backwards compatible way. So there's no one solution to this issue.

    I was never 100% keen on the $.ttl idea, and it's not grown on me since (tho in time in may!). Would love to hear @timbl 's view on this one, if it has evolved.

  5. RubenVerborgh commented on Nov 20, 2018

    @RubenVerborgh
    ContributorAuthor

    With my "solid.community" hat on, I can commit to upgrading to 4.4

    I'll push back on 5.0.0 at this point

    Inrupt can (and will) take care of upgrading to v5. It will involve upgrading the software and running a script that does the renaming, and that's it.

    no switch

    As @kjetilk said, we need to move forward.
    There's no point in still allowing a mistake from the past.

    which entails being locked in to technical debt

    How exactly are we locked in and where specifically is the technical debt?

    The technical debt was in the fact that the URL-to-filename mapping as a) broken and b) spread all over the code, resulting in a 1,790 lines change (#952) to fix.

    Reason being that we'd like to keep compatibility with heterogeneous systems (such as NextCloud, apache, nginx, gold, ldphp, remotestorage etc.).

    We're not giving that up. In fact, we are improving it, because files will now have proper extensions.

    Also what do we do with content addressable files such as git or ni: or did:.

    Unaffected; separate issue.

    There seems to be a number of ways to do this. For example, putting headers in the .meta, or even the .acl files, which could extend to things we've wanted such as the user that created a file.

    Exactly. And the good news is that, because of #952, we can switch to such a system much more easily. We won't have to change hundreds of lines again, just the one component that contains the mapping.

    So there's no one solution to this issue.

    Full agreed. And in vnext, this mapping should be customizable.
    But we should not have a broken mapping as one as the options.

    I was never 100% keen on the $.ttl idea, and it's not grown on me since (tho in time in may!). Would love to hear @timbl 's view on this one, if it has evolved.

    @timbl and I have worked a week on this mapping back in March (https://www.w3.org/DesignIssues/HTTPFilenameMapping.html), and it was still his preferred way back then.

    I also see other options besides $.ttl, those are much easier to implement right now, and at least the $.ttl option isn't broken as the current one is. So let's get rid of it.

  6. RubenVerborgh commented on Nov 20, 2018

    @RubenVerborgh
    ContributorAuthor

    As a side remark, I don't think @melvincarvalho and I have the same definition of "technical debt". You wrote:

    It seems like quite a big change, especially in terms of technical debt. ie requires file name changes

    But assuming a definition of

    Technical debt is a concept in software development that reflects the implied cost of additional rework caused by choosing an easy solution now instead of using a better approach that would take longer.

    —https://en.wikipedia.org/w/index.php?title=Technical_debt&oldid=864238884

    We are not choosing the easy solution here. #952 is the proper fix, by moving the mapping rules into a single place. This requires the one-time "pain" of running a migration script.

    Technical debt is choosing an easy solution, which in this case would be a switch that allows buggy behavior to persist, instead of getting rid of that buggy behavior once and for all.

  7. kjetilk commented on Nov 20, 2018

    @kjetilk
    Member

    Yeah, this change is the technical debt payback.

  8. melvincarvalho commented on Nov 20, 2018

    @melvincarvalho
    Contributor

    Inrupt can (and will) take care of upgrading to v5.

    That phrase comes across as slightly heavy handed.

    You may be unaware on the history here, so I can perhaps clarify slightly. I am the owner of solid.community. It was a personal purchase, that predates inrupt. I set it up with the help of Eric, and we pointed it at the MIT server running on a digital ocean droplet. The idea I had was to run a solid server that was not classed as "test" and that could be offered to the general public, to play around with solid.

    The running of the box has been a team effort. Inrupt has played a big role in helping out, and something for which the community is grateful. But deciding what software runs on it, and when, is hopefully a conversation, and not just something that inrupt decides.

    So, my concern is as follows. Getting the other bug fixes into the code base, if they are all bundled into 5.0.0 makes it more difficult to make decisions about the $.ttl change. ie to back it out. It's the lock-in that is the issue. Both of these could be mitigated by the version numbers, or by a switch. Why not just make this change as a self sufficient upgrade, after the 5.0.0 changes. Or create a modular way in which it can be turned on and off. If it cant be turned off, or cant be backed out very easily, there's going to be some push back, Im afraid.

  9. RubenVerborgh commented on Nov 20, 2018

    @RubenVerborgh
    ContributorAuthor

    I am the owner of solid.community.

    I did not know, apologies, I assumed this was an MIT server (and I have managed it like that in the past, for which I also apologize). If it is yours, you're of course free to do as you please.

    makes it more difficult to make decisions about the $.ttl change. ie to back it out.

    Indeed.

    or cant be backed out very easily, there's going to be some push back

    There always is when making progress :-) However, the current mapping is a bug, not a feature. https://xkcd.com/1172/

  10. melvincarvalho commented on Nov 20, 2018

    @melvincarvalho
    Contributor

    @RubenVerborgh not a problem at all! I honestly just got it to help the solid eco system. Many people have been involved with helping, and I consider it a team effort!

    EDIT: Yes, the server server is run by MIT running on Digital Ocean

  11. RubenVerborgh commented on Nov 20, 2018

    @RubenVerborgh
    ContributorAuthor

    @melvincarvalho Can we further clarify this on Gitter? I pinged you there. Thanks for your help.

  12. RubenVerborgh commented on Nov 20, 2018

    @RubenVerborgh
    ContributorAuthor

    For the upgrade aspects, let's follow up in #950.

  13. kjetilk commented on Nov 20, 2018

    @kjetilk
    Member

    At any rate, 4.4.0 is out :-)

  14. RubenVerborgh commented on Nov 28, 2018

    @RubenVerborgh
    ContributorAuthor

    Since there's only one issue left, let's follow up in that issue (#950).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions