-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Simplify JSON schema, generation, copying and updating #13427
Conversation
src/Umbraco.Cms.Targets/buildTransitive/Umbraco.Cms.Targets.targets
Outdated
Show resolved
Hide resolved
public string UmbracoPath | ||
{ | ||
get => Constants.System.DefaultUmbracoPath; | ||
[Obsolete($"{nameof(UmbracoPath)} is no longer configurable, property setter is scheduled for removal in V12")] | ||
// NOTE: when removing this, also clean up the hardcoded removal of UmbracoPath in UmbracoJsonSchemaGenerator | ||
[Obsolete($"{nameof(UmbracoPath)} is no longer configurable, this property setter is scheduled for removal in V12.")] | ||
// NOTE: When removing this, also clean up the hardcoded removal of UmbracoPath in Umbraco.JsonSchema | ||
set { } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is only the setter marked as obsolete? There's no reason for only keeping the getter in future versions and obsoleting the whole property would ensure we update all references to the constant value, possibly removing the need to inject the IOptions[Monitor|Snapshot]<GlobalSettings>
into implementations that only require this value.
Obsoleted properties are also ignored when generating the JSON schema, so that would already allow cleaning up the hardcoded removal of this property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks to work as described. Have tested with the following:
- Clean clone of the CMS solution using this branch
- Opened in VS.NET and built solution
- Confirmed that the following files exist and appear to be correctly populated:
src\Umbraco.Cms.Targets\appsettings-schema.Umbraco.Cms.json
src\Umbraco.Web.UI\appsettings-schema.Umbraco.Cms.json
tests\Umbraco.Tests.Integration\appsettings-schema.Umbraco.Cms.json
src\Umbraco.Web.UI\appsettings-schema.json
tests\Umbraco.Tests.Integration\appsettings-schema.json
- Confirmed that the noted build log lines exist
- Cleaned the solution
- Confirmed that the following files are removed:
src\Umbraco.Cms.Targets\appsettings-schema.Umbraco.Cms.json
src\Umbraco.Web.UI\appsettings-schema.Umbraco.Cms.json
tests\Umbraco.Tests.Integration\appsettings-schema.Umbraco.Cms.json
- Confirmed that the following files still exist and are correctly populated:
src\Umbraco.Web.UI\appsettings-schema.json
tests\Umbraco.Tests.Integration\appsettings-schema.json
- Confirmed that the noted build log lines exist
This is intended, and the grouping works after I've re-opened the project, so this looks all good to me :D Just had a test, and I'm running into some issues, the clean tasks doesn't seem to clean the files you specified, it leaves the I've also tried creating a new site from this build, and there the file isn't removed either. Additionally there seems to be some trouble with grouping the files in Rider. I've made a recording of cleaning where you can see the issues in a new site: Json.schema.test.site.mp4And the source code: Json.schema.source.code.mp4 |
This has indeed changed since the initial PR was created, see #13427 (comment). Only the |
Prerequisites
Description
The single (and at 133 kB quite big)
appsetting-schema.json
file that we've shipped since Umbraco 10.0.0 included a point-in-time version of the official appsettings.json schema and wasn't extendable by other packages (only the schemas of the current CMS version and specific versions of Forms and Deploy were included).PR #13123 already improved this in the v11 RCs by moving the individual product schemas into separate files (CMS, Forms, Deploy and later also Workflow). This still merged and shipped the official schema and wasn't easily extendable (you'd have to create a PR to add a reference to a new schema file for your package and wait for a new CMS release). Besides that, the JSON schema isn't valid if any of the referenced files don't exist.
This PR fixes these issues by simplifying the
appsettings-schema.json
file to only use references (that are dynamically added):More information on how this works can be found in the JSON Schema documentation: Schema Composition and Structuring a complex schema.
The
appsettings-schema.json
file isn't shipped anymore though, but automatically created/updated using a new MSBuild task fromUmbraco.JsonSchema.Extensions
. This task is redistributed inUmbraco.Cms.Targets
, that uses<UmbracoJsonSchemaReferences Include="" />
MSBuild items to automatically add these references on build. You can also use the<UmbracoJsonSchemaFiles Include="" />
MSBuild items to automatically copy JSON schema files into the Umbraco project directory and have a reference added to this file (you can opt-out of adding the reference by settingReference="false"
on the item). This is also used for theappsettings-schema.Umbraco.Cms.json
file:To test this, ensure all
appsettings-schema*.json
files are deleted and build the solution; you should now have the following files:src\Umbraco.Cms.Targets\appsettings-schema.Umbraco.Cms.json
- automatically created using the Umbraco.JsonSchema console application;src\Umbraco.Web.UI\appsettings-schema.Umbraco.Cms.json
andtests\Umbraco.Tests.Integration\appsettings-schema.Umbraco.Cms.json
- copied from Umbraco.Cms.Targets into the project directory;src\Umbraco.Web.UI\appsettings-schema.json
andtests\Umbraco.Tests.Integration\appsettings-schema.json
- created/updated with references to the original appsettings.json JSON schema and the Umbraco CMS file.You should also see the following in the build log (for both the Umbraco.Web.UI and Umbraco.Tests.Integration projects):
Now clean the solution: all
appsettings-schema.Umbraco.Cms.json
files will be deleted, so it will be re-generated again from source (using the console application, this is skipped when the file already exists) and copied to the project directories on the next build. Theappsettings-schema.json
file isn't touched, as this might be manually edited (and the order of references is important). The build log should contain the following (again for both projects):Breaking changes/upgrade instructions:
Although the structure of the additional
appsettings-schema.*.json
files have changed, these files are overwritten on build.However, if you're upgrading an existing project, you have to manually remove the
appsettings-schema.json
file, as this isn't overwritten and adding the references to the existing file of previous Umbraco versions will result in an invalid schema.