search

DNNCommunity / Dnn.CommunityForums

Open-source forums module for DNN Platform. This is a fork and continuance of the Active Forums module.
https://dnncommunity.org
Other
13 stars 21 forks source link

ENH: Upgrade should update "resources.zip.manifest" #504

Closed johnhenley closed 11 months ago

johnhenley commented 1 year ago

Is your feature request related to a problem?

When you install Community Forums 8.0 as an upgrade, some files are moved during the installation. If you then uninstall, the moved files are not removed.

Describe the solution you'd like

Moved files should be removed.

Describe alternatives you've considered

Manually delete files after uninstalling.

Additional context

Enhance the upgrade code that moves files to update the Resources.zip.manifest file, which is used to remove files when the module is uninstalled.

Timo-Breumelhof commented 1 year ago

Hmm, good catch..

johnhenley commented 1 year ago

@WillStrohl do you think this should be fixed in 08.00.00 or later? My concern with pushing it later is that 08.00.00 moves a bunch of files, and if someone uninstalls they have to manually clean it up. On the other hand, no one should ever uninstall....? I have some time to work on it while we wait for @Timo-Breumelhof to finish themes/templates.

WillStrohl commented 1 year ago

Well, the resources.zip.manifest file is an artifact created by DNN, not the Forums module itself. It's simply outlining the files that were in the Resources.zip file that was included with the installation package. I'd like to know more about that file before we write code against it. This is something that all module developers should be worrying about, in my opinion. I'll try to look into that file later today.

johnhenley commented 1 year ago

Well, the resources.zip.manifest file is an artifact created by DNN, not the Forums module itself. It's simply outlining the files that were in the Resources.zip file that was included with the installation package. I'd like to know more about that file before we write code against it. This is something that all module developers should be worrying about, in my opinion. I'll try to look into that file later today.

Oh you're funny. You know I've already researched and written the code. Just not completed testing yet. For a moved item, it gets added. But trying to decide if a moved item's old entry should be removed. Not sure there's much value since it doesn't cause any issues. I really with the manifest was more robust and stored in the database rather than an xml file.

johnhenley commented 1 year ago

I also don't know if it's appropriate for us to update the manifest since it's created by the installer. And someone could say theirs was corrupted and we caused it?

WillStrohl commented 1 year ago

Oh you're funny. You know I've already researched and written the code. Just not completed testing yet. For a moved item, it gets added. But trying to decide if a moved item's old entry should be removed. Not sure there's much value since it doesn't cause any issues. I really with the manifest was more robust and stored in the database rather than an xml file.

I was referring to when/how/why DNN does it. I just don't know the underlying details of why.

johnhenley commented 1 year ago

Oh you're funny. You know I've already researched and written the code. Just not completed testing yet. For a moved item, it gets added. But trying to decide if a moved item's old entry should be removed. Not sure there's much value since it doesn't cause any issues. I really with the manifest was more robust and stored in the database rather than an xml file.

I was referring to when/how/why DNN does it. I just don't know the underlying details of why.

I spent a lot of time in the DNN platform code looking at the installer, and it's done in the fileInstaller class as files are placed during the install. The resources.zip file is unpacked and as files are written they are logged to the manifest.

https://github.com/dnnsoftware/Dnn.Platform/blob/25d997ec63597cc3ef8338f7c2de7d2120fa8ec2/DNN%20Platform/Library/Services/Installer/Installers/ResourceFileInstaller.cs#L16

image

Then during the uninstall, the manifest is read back and files in the manifest are removed: image

My only real concern is--since it's a platform-created/maintained file, are we "allowed" to update it?

WillStrohl commented 1 year ago

Oh, got it. We should not be editing that file, in my opinion. If it's not getting updated or something, the update to fix that should go into DNN.

johnhenley commented 1 year ago

No. There's nothing for DNN to fix. "We" are moving files, so I think "we" should update the manifest. Installing 07.00.12 writes file to location X. In 08.00.00 upgrade, we are moving file from location X to location Y (i.e. we are moving the templates). So I'm suggesting that we update the manifest to reflect the change from location X to location Y so that when/if the module is uninstalled, the files are removed. It's not a huge deal, I just think we could do a better job of cleaning up during the install, since technically, we made the mess?

johnhenley commented 1 year ago

What would be nice would be if DNN would expose an API for us to call into to update the manifest without directly changing it...but that's really a stretch I think.

Timo-Breumelhof commented 1 year ago

Hmm, right, DNN does not take moving files into account.

WillStrohl commented 1 year ago

FYI - I'm curious about this.
https://dnncommunity.org/forums/using-dnn/extensions/details-about-resources-zip-manifest/

johnhenley commented 11 months ago

@WillStrohl @Timo-Breumelhof should we close this as "not doing" with understanding that files are left behind ?

Timo-Breumelhof commented 11 months ago

@johnhenley IMO yes

johnhenley commented 11 months ago

Closing