Skip to content

Code Tidy: Clean up further obsoleted code scheduled for removal in Umbraco 18 (LogFiles, legacy permissions tables, LoggerConfigExtensions)#22679

Merged
kjac merged 9 commits into
v18/devfrom
v18/task/further-todos-and-obsoletes-10
May 4, 2026
Merged

Code Tidy: Clean up further obsoleted code scheduled for removal in Umbraco 18 (LogFiles, legacy permissions tables, LoggerConfigExtensions)#22679
kjac merged 9 commits into
v18/devfrom
v18/task/further-todos-and-obsoletes-10

Conversation

@AndyButland

@AndyButland AndyButland commented May 2, 2026

Copy link
Copy Markdown
Contributor

Description

This continues the v18 cleanup series, removing several smaller [Obsolete] items: the Constants.SystemDirectories.LogFiles constant, the legacy UserGroup2Node* database-table constants and their orphaned DTO, and the v13-scheduled overloads on LoggerConfigExtensions. Also defers the Constants.Security.SuperUserId / SuperUserIdAsString removal to Umbraco 19 — both are still load-bearing as default parameter values across the public service surface.

Removed

  • Umbraco.Cms.Core.Constants.SystemDirectories.LogFiles[Obsolete("Use LoggingSettings.GetLoggingDirectory() instead. Scheduled for removal in Umbraco 18.")]. The single internal use (LoggingSettings.StaticDirectory) now inlines the literal ~/umbraco/Logs, identical to what the constant evaluated to.
  • Umbraco.Cms.Core.Constants.DatabaseSchema.Tables.UserGroup2Node and UserGroup2NodePermission[Obsolete] since Umbraco 14, when the underlying tables were dropped.
  • Umbraco.Cms.Infrastructure.Persistence.Dtos.UserGroup2NodePermissionDto — orphan DTO for the dropped table.
  • The remaining v13-scheduled [Obsolete] overloads on LoggerConfigExtensions (the two MinimalConfiguration(LoggerConfiguration, IHostingEnvironment, …) overloads, plus OutputDefaultTextFile(LoggerConfiguration, IHostingEnvironment, …) and OutputDefaultJsonFile(LoggerConfiguration, IHostingEnvironment, ILoggingConfiguration, …)). Non-obsolete replacements taking IHostEnvironment remain on the same class. The two internal call sites in Umbraco.Web.Common.Extensions.ServiceCollectionExtensions already use the replacements.

Other notable changes

Deferred

  • Constants.Security.SuperUserId and SuperUserIdAsString were [Obsolete] and scheduled for removal in v18, but both are still consumed across the public service surface as default parameter values (int userId = Constants.Security.SuperUserId on IContentService, IDataTypeService, IRelationService, IMediaService, etc.). Removing the constants requires reshaping those signatures from int userId to Guid userKey, which is a much larger piece of work than this tidy. Obsoletion is deferred to Umbraco 19.

Breaking changes

  • Constants.SystemDirectories.LogFiles is removed.
  • Constants.DatabaseSchema.Tables.UserGroup2Node / UserGroup2NodePermission are removed. The underlying tables haven't existed since Umbraco 14, so any external code referencing the constants was already non-functional against the live schema.
  • UserGroup2NodePermissionDto is removed.
  • The four v13-scheduled obsolete overloads on LoggerConfigExtensions are removed (see Removed above). Non-obsolete replacements on the same class take their place.

Testing

Solution builds and tests pass.

Copilot AI review requested due to automatic review settings May 2, 2026 17:49
@claude

claude Bot commented May 2, 2026

Copy link
Copy Markdown

Claude finished @AndyButland's task in 4m 6s —— View job


PR Review

Target: origin/v18/dev · Based on commit: 969dffac
Skipped 1 file out of 8 total (deleted UserGroup2NodePermissionDto.cs — no longer reviewable)

Removes a handful of [Obsolete]-since-earlier-versions items now due for collection in v18: the LogFiles directory constant, the legacy UserGroup2Node* table-name constants, the orphaned UserGroup2NodePermissionDto, and the four v13-era LoggerConfigExtensions overloads. Defers SuperUserId/SuperUserIdAsString removal to v19 with clear rationale (still load-bearing as default parameter values across 160+ public service surface call sites).

  • Modified public API: Constants.SystemDirectories (removed LogFiles), Constants.DatabaseSchema.Tables (removed UserGroup2Node, UserGroup2NodePermission), LoggerConfigExtensions (removed four obsolete overloads), Constants.Security (SuperUserId/SuperUserIdAsString [Obsolete] message updated)
  • Breaking changes: All removals are intentional and correct — each removed member carried [Obsolete("…Scheduled for removal in Umbraco 18.")], and the current version is 18.0.0-beta1.
  • Other changes: MigrateCharPermissionsToStrings no longer populates UserGroup2GranularPermission from the now-dropped UserGroup2NodePermission table (the table was already gone since v14, so the old code would have thrown at runtime anyway). The migration is itself [Obsolete("Remove in Umbraco 18.")] and will be deleted in a follow-up.

Suggestions

  • src/Umbraco.Core/Constants-Security.cs:15,26: The updated [Obsolete] messages say "Scheduled for removal in Umbraco 19", but the documented policy in CLAUDE.md states the removal target must be current_major + 2. With the current version being 18, that formula gives v20, not v19. v19 is understandable here — this is a deferral of a prior obsoletion rather than a fresh one, and the constants will genuinely block other work until v18's service-signature refactor lands — but it deviates from the stated formula. Worth confirming this is the deliberate choice rather than an off-by-one.

  • src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/MigrateCharPermissionsToStrings.cs:17: _idKeyMap is now only referenced in the commented-out block (lines 87–118). Since the whole migration is [Obsolete("Remove in Umbraco 18.")] and will be deleted in a follow-up, this is harmless (and CS0169 is suppressed project-wide), but worth noting for the follow-up cleanup pass.


Approved with Suggestions for improvement

Good to go, but please carefully consider the importance of the suggestions.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Continues the Umbraco 18 cleanup by removing a set of long-obsoleted constants/DTOs and obsolete Serilog configuration overloads, while updating remaining code paths to use the current replacements.

Changes:

  • Removed Constants.SystemDirectories.LogFiles and inlined the default ~/umbraco/Logs path where needed.
  • Removed legacy UserGroup2Node* table constants and the orphaned UserGroup2NodePermissionDto.
  • Removed v13-scheduled obsolete overloads from LoggerConfigExtensions; retained the IHostEnvironment-based APIs.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Umbraco.Infrastructure/Persistence/Dtos/UserGroup2NodePermissionDto.cs Deletes orphaned legacy DTO for a dropped table.
src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/MigrateCharPermissionsToStrings.cs Comments out node-permission migration block that referenced the removed DTO/constants.
src/Umbraco.Infrastructure/Migrations/Upgrade/Common/DeleteKeysAndIndexes.cs Removes umbracoUserGroup2NodePermission from the v7.14 table cleanup list.
src/Umbraco.Infrastructure/Logging/Serilog/LoggerConfigExtensions.cs Removes obsolete v13-scheduled overloads using IHostingEnvironment/IConfiguration.
src/Umbraco.Core/Persistence/Constants-DatabaseSchema.cs Removes obsolete legacy table-name constants.
src/Umbraco.Core/Constants-SystemDirectories.cs Removes obsolete LogFiles constant.
src/Umbraco.Core/Constants-Security.cs Defers SuperUserId/SuperUserIdAsString removal to Umbraco 19 via updated [Obsolete] messages.
src/Umbraco.Core/Configuration/Models/LoggingSettings.cs Replaces removed LogFiles constant usage with the literal default logging directory.

AndyButland and others added 2 commits May 2, 2026 19:56
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@kjac kjac merged commit 2d4418b into v18/dev May 4, 2026
28 checks passed
@kjac kjac deleted the v18/task/further-todos-and-obsoletes-10 branch May 4, 2026 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants