pluginpal / strapi-plugin-config-sync

:recycle: CLI & GUI for continuous migration of config data across environments
https://www.pluginpal.io/plugin/config-sync
MIT License
254 stars 38 forks source link

Use documentId as the unique identifier #65

Open jnachtigall opened 2 years ago

jnachtigall commented 2 years ago

We have a custom type, that only has a uniq headline(in german) and a textfield

image

This gets exported as

      customTypes: [
        {
          configName: "article",
          queryString: "api::article.article",
          uid: ["headline"],
        },
    ]

On the filesystem you then have files like

strapi-app/config/sync$ ls -lh article.*
-rw-r--r-- 1 jnachtigall jnachtigall 517 Aug 30 16:44 'article.Benötige ich eine wasserrechtliche Erlaubnis?.json'
-rw-r--r-- 1 jnachtigall jnachtigall 575 Aug 30 16:44 'article.Gibt es Fälle, in denen ich keine Anzeige stellen kann?.json'
-rw-r--r-- 1 jnachtigall jnachtigall 436 Aug 30 16:44 'article.Wie kann ich mich authentifizieren $ anmelden?.json'

This works fine on Linux or in WSL2. However, on Windows this results in these git errors

$ git -c credential.helper= -c core.quotepath=false -c log.showSignature=false rebase origin/develop
error: invalid path 'strapi-app/config/sync/article.Benötige ich eine wasserrechtliche Erlaubnis?.json'
error: invalid path 'strapi-app/config/sync/article.Gibt es Fälle, in denen ich keine Anzeige stellen kann?.json'
error: invalid path 'strapi-app/config/sync/article.Wie kann ich mich authentifizieren $ anmelden?.json'
error: could not detach HEAD

This seems like a pretty well known error, see https://stackoverflow.com/questions/63727594/github-git-checkout-returns-error-invalid-path-on-windows

FWIW, doing the suggested git config core.protectNTFS false did not work for us (resulted in read/write errors and a bluescreen).

We worked around the issue by adding a field articleId with integer to the above custom type and used this as uid instead.

Not sure you want to fix this in your plugin, but maybe a encoding/decoding of non-ascii characters would be good in the filename?

System

boazpoolman commented 2 years ago

Allright. Thanks for the bug report. You’re kinda hitting the current limit of the uid feature.

The uid will be used in the filename and therefor cannot contain characters which are not supported in filenames on your OS. I’ve allready escaped two characters (semicolon and forward slash) see issue #14 and #56. Though because you’re using a title like field as the uid there are many characters that are likely not supported in a filename.

As you suggested we need to encode the uid value to be a valid filename, and we also need to be able to decode that filename back to it’s original state. Otherwise we can’t match the uid value in the DB.

I’m not planning on fixing this any time soon, but I’ll leave it open for reference. In your case it is best to use another field as the uid. You could create an extra field of type uid which encodes the headline field and saves that in the DB in a seperate field. Than that seperate field can be the uid for your custom config type.

boazpoolman commented 1 year ago

We might have to consider dropping the uid system as it is, and switch to a uuid approach. That would solve this and a lot of other issues.

boazpoolman commented 1 year ago

I'm rebranding this issue from a bug to an enhancement.

I would like to see a new major version of the plugin with a proper uuid system. As of writing this comment the config types are required to have a unique field that is used to match entries across databases. Though that is very prone to errors and is why this issue (and a few others) were created.

I would suggest we inject a uuid field onto the config types in the register function of this plugin, or we use some 3rd party plugin to do that for us. Then that value can be used to write the JSON filename, and be used as a unique identifier.

boazpoolman commented 1 month ago

With Strapi 5 being stable, we might be able to use it's CUID documentId as the unique identifier of config types. I'll do some testing to see if this can be added to v2 of the config sync plugin

boazpoolman commented 6 days ago

I just realised that by using the document ID as the unique identifier we lose the ability to exclude certain configs by default. Those configs are excluded here and are referenced by their names. If we were to use the document ID, that would mean that the names will be different for every instance using this plugin. Losing the ability to exclude these by default.

Soo.. I'll likely stick to the uid system as it is.

Something that we could do is make the uid optional and if it's left empty fall back to the document ID. That way custom types can be registered without a uid, but the default types will still have the custom uid to exclude some configs by default. Though I don't have much incentive to build this as I believe it's quite a niche feature, and I'm not sure how many people use the custom types feature anyways.