Serialize stub for dynamic profiles

[carlos-zamora]

#9962 was caused by a serialization bug. Technically, ToJson works
as intended: if the current layer has any values set, write them out to
the json. However, on first load, the dynamic profile Profile objects
are actually empty (because they inherit from base layer, then the
dynamic profile generator). This means that ToJson writes the dynamic
profiles as empty objects {}. Then, on reload, we see that the dynamic
profiles aren't in the JSON, and we write them again.

To get around this issue, we added a simple check to Profile::ToJson:
if we have a source, make sure we write out the name, guid, hidden, and
source. This is intended to align with Profile::GenerateStub.

Closes #9962

[DHowett]

Great! Thanks so much!

qq: why force it during serialization instead of making sure the values are populated when the layer is created? On deserialization, that is.

or: should we just always write out name, guid, hidden, source?

[carlos-zamora]

Had an offline discussion:

• Technically the only important ones to carry over are guid and source
• name is just nice to have because it helps the user orient themselves
• Idea: generate guid, source, and (as comment) name (make sure that GenerateStub does the same though)

qq: why force it during serialization instead of making sure the values are populated when the layer is created? On deserialization, that is.

No preference between the two. Figured altering ToJson would be safer but the other would work too. I guess it would mess with finding the source profile object for those 4 properties?

[lhecker]

I can confirm that this fixes the bug of empty ({}) profiles being created, as well as the SUI save button not working during the first launch.

I personally believe that we should put this logic into Profile::ToJson, as this model class controls how it's serialized/deserialized and "should" probably also control which fields are required in a serialization.
The alternative would be to put this change into CascadiaSettings::_ApplyDefaultsFromUserSettings which I believe might be a less general solution.

[zadjii-msft]

I'm gonna approve for now because this is such a high hitter, but we really should have a test for this

[DHowett]

Just a nit. This is a fix that will work, but it is a bit adhoc. To @lhecker's point about things that should be invariant during serialization: these things should be invariant during construction! The object should not be able to be in a bad state, at least for GUID. Name is neither here nor there. 😄

[lhecker]

these things should be invariant during construction

@DHowett As you might've already heard before the underlying issue is that during the first start we generate dynamic profiles. Due to that you might have two OriginTag::InBox profiles (cmd, powershell) and two OriginTag::Generated ones (pwsh7, wsl). During this initial load all of them get child profiles in CascadiaSettings::_ApplyDefaultsFromUserSettings. At this point we got a problem already: All 4 child profiles have no GUID and no name! The invariant is already violated. So question No. 1:
👉 Would you suggest that CreateChild() copies over the GUID, name, etc. to uphold this "invariant"?

This "issue" of having no GUID (and so on) is fixed for the two inbox profiles right here. Since we load the builtin default-settings.json during load the 2 inbox profiles are stored in _userSettings and that's the two profiles which _GetProfilesJsonObject returns. As such those two inbox profiles are layered in _LayerOrCreateProfile, where those child profiles are assigned the 4 essential properties (GUID, ...) from the default-settings.json.

💣 The dynamic profiles don't get this treatment since they're not part of the _userSettings during the first start of our app. Only on the second start they'll be stored in _userSettings.

[ghost]

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[ghost]

🎉Windows Terminal v1.8.1444.0 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download

[ghost]

🎉Windows Terminal Preview v1.9.1445.0 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download