Skip to content

Remove v1 ContentRepository#3753

Merged
sumerjabri merged 15 commits into
craftercms:developfrom
jmendeza:feature/7542
Apr 2, 2025
Merged

Remove v1 ContentRepository#3753
sumerjabri merged 15 commits into
craftercms:developfrom
jmendeza:feature/7542

Conversation

@jmendeza
Copy link
Copy Markdown
Member

@jmendeza jmendeza commented Mar 20, 2025 •
edited by coderabbitai Bot
Loading

Remove v1 ContentRepository
craftercms/craftercms#7542

Summary by CodeRabbit

  • New Features

    • Introduced a restore capability to recover previous content versions.
    • Added new methods for managing content, including retrieving content children and reverting content to previous versions.

  • Refactor

    • Streamlined content management by discontinuing direct version history retrieval and remote repository operations.
    • Enhanced security and authentication handling for a more reliable user experience.
    • Updated the handling of publish results to utilize a new data structure for better clarity and error management.

  • Chores

    • Removed legacy integrations and outdated API endpoints to simplify and modernize the underlying infrastructure.
    • Updated import statements to reflect the transition to version 2 of the API across various components.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 20, 2025 •
edited
Loading

Walkthrough

The changes span multiple modules and versions of the API. Several endpoints, interfaces, and methods from the legacy (v1) API have been removed or deprecated—including functionality for version history, file-to-folder conversion, and remote repository operations. In their place, updated method signatures and new exception handling (primarily using ServiceLayerException) have been introduced in the v2 API. Security handling has been refactored to eliminate direct dependencies on SecurityService in favor of a newly introduced utility class, SecurityUtils. Repository implementations and service classes have been streamlined and updated, and corresponding tests and Spring configuration files have been adjusted accordingly.

Changes

File(s) Change Summary
src/main/api/studio-api.yaml Removed the entire endpoint /api/1/services/api/1/content/get-item-versions.json with all its operations and schemas.
src/main/java/org/craftercms/studio/api/v1/ (e.g., *DmContentProcessor.java, ContentRepository.java, GitContentRepository.java, TransactionService.java, DependencyDiffService.java, DependencyService.java, ContentService.java, SiteService.java) Removed several methods and interfaces (e.g., fileToFolder, ContentRepository, GitContentRepository, TransactionService, dependency-related methods, remote repository methods, and version history methods). Exception declarations were simplified or removed in existing methods.
src/main/java/org/craftercms/studio/api/v2/ (e.g., *ContentRepository.java, GitContentRepository.java, RepositoryItem.java, ConfigurationService.java, ContentService.java, ContentTypeServiceInternal.java) Introduced enhancements including a default getContent(String, String) method, added error handling (e.g. throwing ServiceLayerException, UserNotFoundException), changed RepositoryItem to a record, and updated method signatures (e.g. removal of commitId parameters and consolidation of remote management features).
Security-related files (e.g., SecurityService.java, PeerReviewCapableAnnotationHandler.java, PublishPackageAvailableActionResolverImpl.java, various v2/security/) Updated deprecation messages and removed methods such as getCurrentUser() and getAuthentication(). Replaced SecurityService dependency with static calls to SecurityUtils.
Repository & Service Implementations (e.g., *AssetDmContentProcessor.java, FormDmContentProcessor.java, ContentServiceImpl.java, GitContentRepositoryImpl.java (v1 removed), BlobAwareContentRepository.java, StudioBlobStore.java) Refactored method calls (e.g. removal of commitId and fileToFolder references), replaced old GitContentRepositoryImpl with new v2 implementations (adding methods like getContentChildren, createFolder, moveContent), and streamlined repository handling.
Spring Config & Groovy Scripts (e.g., *common.xml, studio-services-context.xml, ContentServices.groovy, SiteServices.groovy, get-item-versions.get.groovy) Removed or commented out remote repository management properties and methods; deleted obsolete groovy scripts for version history retrieval.
Test files (e.g., *ContentServiceImplTest.java, BlobAwareContentRepositoryTest.java, PolicyServiceInternalImplTest.java, RepositoryManagementServiceInternalImplTest.java, SitesServiceInternalImplTest.java) Updated imports and method signatures to align with the new v2 API (e.g. changes from arrays to collections), removed mocks for v1 repositories, and modified exception declarations accordingly.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant SecurityUtils
    participant SecurityContextHolder
    Caller->>SecurityUtils: getCurrentUser()
    SecurityUtils->>SecurityContextHolder: getAuthentication()
    SecurityContextHolder-->>SecurityUtils: Authentication object
    SecurityUtils-->>Caller: Returns current username (or null)
Loading 
sequenceDiagram
    participant Client
    participant GitRepositoryHelper
    participant GitCli
    Client->>GitRepositoryHelper: restoreVersion(repo, siteId, path, version)
    GitRepositoryHelper->>GitCli: restoreVersion(repoPath, path, version)
    GitCli-->>GitRepositoryHelper: File restored (or error thrown)
    GitRepositoryHelper-->>Client: Return success/error result
Loading

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot]
coderabbitai Bot reviewed Mar 20, 2025
View reviewed changes
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🔭 Outside diff range comments (1)
src/main/java/org/craftercms/studio/impl/v1/service/site/SiteServiceImpl.java (1)

464-474: 🛠️ Refactor suggestion

Consider performance implications of enumerating all site paths.
The forAllSitePaths call may be expensive depending on repository size. You may want to offer parallelization or at least logging/progress updates for large sites.

🧹 Nitpick comments (15)
src/main/java/org/craftercms/studio/impl/v2/utils/git/GitCli.java (1)

445-462: Good implementation, minor formatting improvement suggested.

The new restoreVersion method is a valuable addition that follows the established patterns in the class. However, for consistency with other methods in this class (like on lines 254, 284, etc.), consider using String.format for the error message:

-    throw new GitCliException("Git restore failed on directory " + repoPath + " for path " + path, e);
+    throw new GitCliException(format("Git restore failed on directory '%s' for path '%s'", repoPath, path), e);

This minor change would maintain the formatting style used throughout the rest of the class.

src/main/java/org/craftercms/studio/impl/v1/executor/ProcessContentExecutorImpl.java (1)

82-91: Consider removing SecurityService dependency.

The SecurityService field and its getter/setter methods are still present in the class but are no longer used after the change to use the static SecurityUtils.getCurrentUser() method. Consider removing these to clean up the code.

-    protected SecurityService securityService;
-
-    public SecurityService getSecurityService() {
-        return securityService;
-    }
-
-    public void setSecurityService(SecurityService securityService) {
-        this.securityService = securityService;
-    }
src/main/java/org/craftercms/studio/api/v1/service/site/SiteService.java (1)

208-247: Commented-out remote repository methods.
If they are truly unnecessary, consider removing them entirely to reduce clutter. Otherwise, add a rationale comment for future reference.

src/main/java/org/craftercms/studio/impl/v1/service/content/ContentServiceImpl.java (4)

151-151: @deprecated annotation on contentExists method.
Document the replacement or intended removal timeframe.

242-242: Optional shallow parameter in getContent.
Could improve performance by skipping deeper reads.

711-714: Persisting the folder item with current user.
Consider fallback handling when SecurityUtils returns null.

2031-2031: Calling revertContent(...) on repository.
Ensure version validation is done ahead of time.

src/main/java/org/craftercms/studio/impl/v1/service/site/SiteServiceImpl.java (3)

130-130: Favor interface-based injection over concrete class.
Using GitContentRepository as the field type can limit polymorphism and testability. Consider using the more generic ContentRepository interface to keep your design flexible.

630-630: Consider handling the result of remote removal.
If removeRemote returns a boolean or throws specific exceptions, ensure you log or handle failure cases beyond simply calling the method.

969-969: Use the interface in setters for consistency.
Use ContentRepository rather than GitContentRepository to keep the codebase flexible and more easily mockable in tests.

src/main/java/org/craftercms/studio/impl/v2/service/security/internal/UserServiceInternalImpl.java (1)

485-491: Simplified authentication check in getCurrentUser.

The updated implementation simplifies the authentication check and provides a clearer error message when authentication fails. However, the method doesn't log the failure before throwing the exception.

@Override
public AuthenticatedUser getCurrentUser() throws AuthenticationException {
    Authentication authentication = SecurityUtils.getAuthentication();
    if (authentication != null) {
        return (AuthenticatedUser) authentication.getPrincipal();
    }
-   throw new AuthenticationException("User should be authenticated");
+   logger.debug("Authentication check failed - no authenticated user found");
+   throw new AuthenticationException("User should be authenticated");
}
src/main/java/org/craftercms/studio/impl/v2/repository/GitContentRepositoryImpl.java (1)

76-76: Consider avoiding wildcard imports.

Depending on your style guidelines, you might explicitly import only the classes you need rather than using import java.io.*;.

src/test/java/org/craftercms/studio/impl/v2/repository/blob/BlobAwareContentRepositoryTest.java (2)

217-217: Remove vestigial comment referencing localV1.
If no longer needed, deleting the commented line helps reduce clutter and confusion.

- // verify(localV1).writeContent(eq(SITE), eq(POINTER_PATH), any());

231-231: Remove vestigial comment referencing localV1.
Same rationale as above—clean up the commented-out code for clarity.

- // verify(localV1, never()).writeContent(eq(SITE), eq(POINTER_PATH), any());
src/main/java/org/craftercms/studio/impl/v1/service/content/ContentTypeServiceImpl.java (1)

45-45: Wildcard import.
import java.util.*; is acceptable, though some projects discourage it for clarity. If your coding guidelines permit, this is fine.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1a76424 and 76c72d1.

📒 Files selected for processing (82)
  • src/main/api/studio-api.yaml (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/content/pipeline/DmContentProcessor.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/repository/ContentRepository.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/repository/GitContentRepository.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/content/ContentService.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/content/ContentTypeService.java (1 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/dependency/DependencyDiffService.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/dependency/DependencyService.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/security/SecurityService.java (3 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/site/SiteService.java (2 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/transaction/TransactionService.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v2/repository/ContentRepository.java (2 hunks)
  • src/main/java/org/craftercms/studio/api/v2/repository/GitContentRepository.java (5 hunks)
  • src/main/java/org/craftercms/studio/api/v2/repository/RepositoryItem.java (2 hunks)
  • src/main/java/org/craftercms/studio/api/v2/repository/blob/StudioBlobAwareContentRepository.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v2/repository/blob/StudioBlobStore.java (2 hunks)
  • src/main/java/org/craftercms/studio/api/v2/security/publish/PeerReviewCapableAnnotationHandler.java (3 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/config/ConfigurationService.java (1 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/content/ContentService.java (1 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/content/internal/ContentTypeServiceInternal.java (1 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/item/internal/ItemServiceInternal.java (1 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/security/SecurityService.java (3 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/security/internal/GroupServiceInternal.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/security/internal/UserServiceInternal.java (2 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/site/SitesService.java (1 hunks)
  • src/main/java/org/craftercms/studio/api/v2/upgrade/StudioUpgradeManager.java (2 hunks)
  • src/main/java/org/craftercms/studio/api/v2/utils/GitRepositoryHelper.java (3 hunks)
  • src/main/java/org/craftercms/studio/controller/rest/v2/ContentController.java (1 hunks)
  • src/main/java/org/craftercms/studio/controller/rest/v2/SitesController.java (4 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/content/pipeline/AssetDmContentProcessor.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/content/pipeline/FormDmContentProcessor.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/executor/ProcessContentExecutorImpl.java (2 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/repository/git/GitContentRepositoryImpl.java (0 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/service/content/ContentServiceImpl.java (31 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/service/content/ContentTypeServiceImpl.java (3 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/service/dependency/DependencyDiffServiceImpl.java (0 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/service/dependency/DependencyServiceImpl.java (0 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/service/site/SiteServiceImpl.java (12 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/service/transaction/TransactionServiceImpl.java (0 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/repository/GitContentRepositoryImpl.java (5 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/repository/RepositoryStartupCleanup.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/repository/blob/BlobAwareContentRepository.java (25 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/repository/blob/StudioBlobStoreResolverImpl.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/repository/blob/s3/StudioAwsS3BlobStore.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/security/publish/PublishPackageAvailableActionResolverImpl.java (2 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/configuration/ConfigurationServiceImpl.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/configuration/internal/ConfigurationServiceInternalImpl.java (9 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/content/ContentServiceImpl.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/content/internal/ContentServiceInternalImpl.java (10 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/content/internal/ContentTypeServiceInternalImpl.java (2 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/dashboard/DashboardServiceImpl.java (5 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/item/internal/ItemServiceInternalImpl.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/policy/internal/PolicyServiceInternalImpl.java (5 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/publish/internal/PublishServiceInternalImpl.java (5 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/repository/RepositoryManagementServiceImpl.java (4 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/repository/internal/RepositoryManagementServiceInternalImpl.java (4 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/security/SecurityServiceImpl.java (6 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/security/UserServiceImpl.java (9 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/security/internal/GroupServiceInternalImpl.java (0 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/security/internal/UserServiceInternalImpl.java (8 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/site/SitesServiceImpl.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/site/internal/SitesServiceInternalImpl.java (11 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/workflow/internal/WorkflowServiceInternalImpl.java (3 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/upgrade/StudioUpgradeManagerImpl.java (5 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/upgrade/providers/SiteVersionProvider.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/upgrade/providers/XmlFileVersionProvider.java (2 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/upgrade/providers/XmlStreamVersionProvider.java (2 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/utils/git/GitCli.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1 hunks)
  • src/main/resources/crafter/studio/security/common.xml (0 hunks)
  • src/main/resources/crafter/studio/studio-services-context.xml (3 hunks)
  • src/main/webapp/default-site/scripts/api/ContentServices.groovy (0 hunks)
  • src/main/webapp/default-site/scripts/api/SiteServices.groovy (1 hunks)
  • src/main/webapp/default-site/scripts/api/impl/content/SpringContentServices.groovy (0 hunks)
  • src/main/webapp/default-site/scripts/api/impl/site/SpringSiteServices.groovy (1 hunks)
  • src/main/webapp/default-site/scripts/rest/api/1/content/get-item-versions.get.groovy (0 hunks)
  • src/test/java/org/craftercms/studio/impl/v1/service/content/ContentServiceImplTest.java (7 hunks)
  • src/test/java/org/craftercms/studio/impl/v2/repository/blob/BlobAwareContentRepositoryTest.java (8 hunks)
  • src/test/java/org/craftercms/studio/impl/v2/repository/blob/StudioBlobStoreResolverImplTest.java (2 hunks)
  • src/test/java/org/craftercms/studio/impl/v2/service/policy/internal/PolicyServiceInternalImplTest.java (4 hunks)
  • src/test/java/org/craftercms/studio/impl/v2/service/repository/internal/RepositoryManagementServiceInternalImplTest.java (2 hunks)
  • src/test/java/org/craftercms/studio/impl/v2/service/site/internal/SitesServiceInternalImplTest.java (2 hunks)
💤 Files with no reviewable changes (19)
  • src/main/java/org/craftercms/studio/api/v1/service/transaction/TransactionService.java
  • src/main/java/org/craftercms/studio/api/v1/repository/GitContentRepository.java
  • src/main/java/org/craftercms/studio/api/v1/content/pipeline/DmContentProcessor.java
  • src/main/java/org/craftercms/studio/impl/v1/service/dependency/DependencyDiffServiceImpl.java
  • src/main/java/org/craftercms/studio/impl/v2/service/security/internal/GroupServiceInternalImpl.java
  • src/main/webapp/default-site/scripts/api/ContentServices.groovy
  • src/main/java/org/craftercms/studio/api/v1/service/dependency/DependencyService.java
  • src/main/webapp/default-site/scripts/rest/api/1/content/get-item-versions.get.groovy
  • src/main/java/org/craftercms/studio/api/v2/service/security/internal/GroupServiceInternal.java
  • src/main/resources/crafter/studio/security/common.xml
  • src/main/java/org/craftercms/studio/api/v1/repository/ContentRepository.java
  • src/main/java/org/craftercms/studio/api/v2/repository/blob/StudioBlobAwareContentRepository.java
  • src/main/java/org/craftercms/studio/impl/v1/repository/git/GitContentRepositoryImpl.java
  • src/main/java/org/craftercms/studio/impl/v1/service/transaction/TransactionServiceImpl.java
  • src/main/java/org/craftercms/studio/api/v1/service/dependency/DependencyDiffService.java
  • src/main/api/studio-api.yaml
  • src/main/webapp/default-site/scripts/api/impl/content/SpringContentServices.groovy
  • src/main/java/org/craftercms/studio/api/v1/service/content/ContentService.java
  • src/main/java/org/craftercms/studio/impl/v1/service/dependency/DependencyServiceImpl.java
🧰 Additional context used
🧬 Code Definitions (16)
src/main/java/org/craftercms/studio/impl/v1/executor/ProcessContentExecutorImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/workflow/internal/WorkflowServiceInternalImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/site/internal/SitesServiceInternalImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/content/internal/ContentServiceInternalImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/publish/internal/PublishServiceInternalImpl.java (2)
src/main/java/org/craftercms/studio/api/v2/dal/item/LightItem.java (1) (1)
  • LightItem (24-44)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/security/publish/PublishPackageAvailableActionResolverImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/dashboard/DashboardServiceImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/api/v2/utils/GitRepositoryHelper.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/test/java/org/craftercms/studio/impl/v2/service/repository/internal/RepositoryManagementServiceInternalImplTest.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/content/internal/ContentTypeServiceInternalImpl.java (1)
src/main/java/org/craftercms/studio/api/v1/to/ContentTypeConfigTO.java (1) (1)
  • ContentTypeConfigTO (31-375)
src/main/java/org/craftercms/studio/impl/v2/service/configuration/internal/ConfigurationServiceInternalImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/repository/internal/RepositoryManagementServiceInternalImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/api/v2/security/publish/PeerReviewCapableAnnotationHandler.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/security/SecurityServiceImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v1/service/content/ContentTypeServiceImpl.java (1)
src/main/java/org/craftercms/studio/api/v1/constant/StudioConstants.java (1) (1)
  • StudioConstants (28-257)
src/test/java/org/craftercms/studio/impl/v2/repository/blob/BlobAwareContentRepositoryTest.java (1)
src/main/java/org/craftercms/studio/impl/v2/repository/GitContentRepositoryImpl.java (1) (1)
  • GitContentRepositoryImpl (112-1855)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Travis CI - Pull Request
🔇 Additional comments (227)
src/main/java/org/craftercms/studio/api/v2/service/security/internal/UserServiceInternal.java (1)

187-193: New method added to support role-based functionality.

This new getUserGlobalRoles method enhances the API by providing a way to retrieve global roles associated with a user. This aligns with the broader changes in the PR to transition away from v1 ContentRepository in favor of v2 API structures.

src/main/java/org/craftercms/studio/impl/v2/repository/blob/StudioBlobStoreResolverImpl.java (1)

29-29: Updated import to use v2 ContentRepository.

This change correctly updates the import from v1 to v2 ContentRepository as part of the PR objective to remove v1 ContentRepository. The code continues to work without modification because the methods used in this class are still available in the v2 API.

src/main/java/org/craftercms/studio/impl/v1/executor/ProcessContentExecutorImpl.java (1)

39-39: Removed dependency on SecurityService.

The code now uses a static utility method from SecurityUtils instead of the injected SecurityService. This change helps reduce direct dependencies on service instances and follows a more modular approach.

Also applies to: 55-55

src/main/java/org/craftercms/studio/impl/v2/repository/RepositoryStartupCleanup.java (1)

102-105: Added null check to prevent NullPointerException.

Good defensive programming by adding a null check for the repository object. This prevents potential NullPointerExceptions that could occur if the repository doesn't exist for a site.

src/main/java/org/craftercms/studio/impl/v2/service/configuration/ConfigurationServiceImpl.java (1)

181-184: Method implementation aligns with interface contract.

The addition of getSiteGroups method follows the established delegate pattern in this class, properly implementing the interface method by delegating to the internal service.

src/main/java/org/craftercms/studio/api/v2/service/config/ConfigurationService.java (1)

253-261: Well-documented new API method.

The new getSiteGroups method has clear JavaDoc that explains its purpose, parameters, return value, and possible exceptions. This method enhances the interface by providing access to site-specific group information.

src/main/java/org/craftercms/studio/impl/v2/service/content/ContentServiceImpl.java (1)

81-81:

Details

✅ Verification successful

Exception type broadened for consistency.

The method signature has been changed to throw ServiceLayerException instead of the more specific SiteNotFoundException. This change likely aligns with a broader effort to standardize exception handling across the service layer.

🏁 Script executed:

#!/bin/bash
# Check if other methods in ContentService interface have been updated to use ServiceLayerException
rg -A 1 "getQuickCreatableContentTypes" --glob "*.java" --no-filename | grep -v "impl"

Length of output: 990

Verified Exception Consistency

The change in the method signature to throw ServiceLayerException instead of SiteNotFoundException has been confirmed across related implementations in the codebase. The shell script output shows consistent usage of ServiceLayerException for getQuickCreatableContentTypes across various interfaces and implementations. No further changes are necessary.

src/main/java/org/craftercms/studio/api/v2/service/item/internal/ItemServiceInternal.java (1)

148-149:

Details

✅ Verification successful

Parameter removed from method signature.

The commitId parameter has been removed from the persistItemAfterWrite method. This change suggests a simplification of the API and reduced dependency on version control details when persisting item metadata after write operations.

🏁 Script executed:

#!/bin/bash
# Check for implementations of this method to ensure they've been updated
rg -A 4 "persistItemAfterWrite.*\(" --glob "*.java" --no-filename | grep -v "interface"

Length of output: 1868

Verified: commitId parameter successfully removed from persistItemAfterWrite

All call sites and the method signature now consistently use the simplified four-parameter API. The verification confirms that no implementation still expects the removed commitId parameter.

src/test/java/org/craftercms/studio/impl/v2/service/site/internal/SitesServiceInternalImplTest.java (1)

125-126: Dependency injection properly added

The test now correctly sets up the itemServiceInternal dependency in the sitesServiceInternal instance. This change aligns with the removal of direct SecurityService dependencies, part of the broader refactoring to eliminate the v1 ContentRepository pattern.

src/main/java/org/craftercms/studio/impl/v2/upgrade/providers/SiteVersionProvider.java (1)

20-21: Import updated to v2 repository API

The import statement has been properly updated to use the v2 repository API (org.craftercms.studio.api.v2.repository.GitContentRepository) instead of the v1 version, which aligns with the PR objective of removing the v1 ContentRepository.

src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1)

1-68: Well-structured security utility implementation

This new utility class provides a clean and centralized way to handle security operations. The getCurrentUser() and getAuthentication() methods are properly implemented with the necessary null checks to prevent NPEs. This approach eliminates the need for direct SecurityService dependencies across the codebase, making the security handling more modular and easier to maintain.

Both methods use appropriate validation:

  • getCurrentUser() correctly handles both standard authentication and CronJobContext scenarios
  • getAuthentication() safely accesses the SecurityContext
src/main/java/org/craftercms/studio/impl/v2/service/workflow/internal/WorkflowServiceInternalImpl.java (2)

58-58: Properly added static import for SecurityUtils

The static import for getAuthentication from the new SecurityUtils class is correctly added, supporting the change from direct SecurityService dependency to using utility methods.

176-176: Security handling properly migrated to SecurityUtils

The code now uses getAuthentication() from the SecurityUtils utility class instead of relying on a SecurityService instance. This change is consistent with the architectural refactoring to remove direct dependencies on SecurityService.

src/main/java/org/craftercms/studio/api/v2/repository/blob/StudioBlobStore.java (1)

22-22: Interface inheritance updated to use v2 ContentRepository

The StudioBlobStore interface now properly extends the v2 ContentRepository instead of the v1 version. This change aligns with the PR objective of removing the v1 ContentRepository, streamlining the inheritance hierarchy.

Also applies to: 35-35

src/main/java/org/craftercms/studio/api/v2/service/content/internal/ContentTypeServiceInternal.java (1)

38-38: Exception handling standardized

The method signature has been updated to throw ServiceLayerException, aligning with the standardized exception handling approach being implemented across the codebase. This change provides more consistent error propagation.

src/main/java/org/craftercms/studio/controller/rest/v2/ContentController.java (1)

100-101: Exception handling updated in controller

The controller method now throws ServiceLayerException instead of SiteNotFoundException, matching the changes made in the underlying service layer. This ensures consistent exception handling throughout the call stack.

src/main/java/org/craftercms/studio/api/v2/upgrade/StudioUpgradeManager.java (1)

22-22: Exception handling standardized in upgrade manager

The method signature for getExistingEnvironments has been updated to throw ServiceLayerException, adding proper error handling capabilities. This change is part of the broader effort to standardize exception handling throughout the application.

Also applies to: 70-70

src/main/java/org/craftercms/studio/api/v1/service/security/SecurityService.java (3)

2-2: Copyright year updated to 2025

The copyright year has been updated from 2024 to 2025.

37-37: Deprecated method reference updated to point to SecurityUtils utility class

The deprecation message has been updated to direct users to the new utility class SecurityUtils instead of the v2 SecurityService interface. This change reflects the architectural shift from using a service interface to a static utility class for authentication-related methods.

46-46: Deprecated method reference updated to point to SecurityUtils utility class

Similar to the previous change, the deprecation message for getAuthentication() now directs users to the new utility class SecurityUtils instead of the v2 SecurityService interface.

src/main/java/org/craftercms/studio/api/v2/service/content/ContentService.java (1)

67-67:

Details

❓ Verification inconclusive

Exception type broadened from SiteNotFoundException to ServiceLayerException

The method signature has been changed to throw the more general ServiceLayerException instead of the specific SiteNotFoundException. This is a breaking change that may require updates to calling code that handles this exception.

This change is appropriate as it allows the method to signal a wider range of potential error conditions, but ensure that all callers are updated to handle the new exception type.

🏁 Script executed:

#!/bin/bash
# Look for usages of getQuickCreatableContentTypes and their exception handling
echo "Searching for usages of getQuickCreatableContentTypes method..."
rg -A 5 "getQuickCreatableContentTypes" --type java

Length of output: 5399

Action Required: Confirm Handling of Broadened Exception Type

The method signature for getQuickCreatableContentTypes now throws a more general ServiceLayerException instead of the specific SiteNotFoundException. Verification across the codebase shows that all implementations and call sites (e.g., in ContentController.java, ContentServiceImpl.java, and internal service classes) consistently use the updated exception. However, be sure to review any error handling or catch blocks to ensure they appropriately handle the broader ServiceLayerException.

  • ContentController.java: The method is invoked without local try-catch handling, likely relying on a global exception handler.
  • Internal Service Implementations: The change is uniformly applied.
  • Action: Confirm that any caller relying on a specific SiteNotFoundException behavior is updated to handle the new, broader exception type.
src/main/java/org/craftercms/studio/api/v2/repository/RepositoryItem.java (2)

2-2: Copyright year updated to 2025

The copyright year has been updated from 2024 to 2025.

17-19:

Details

❓ Verification inconclusive

Class migrated from v1 to v2 package and converted to a Java record

The RepositoryItem has been:

  1. Moved from v1 to v2 package structure
  2. Converted from a class to a Java record

This is a good modernization that takes advantage of newer Java features, resulting in cleaner, more concise code. The record provides immutability and automatically generates constructors, accessors, equals, hashCode, and toString methods.

However, this is a breaking change that will require updates to all code that imports and uses this class.

🏁 Script executed:

#!/bin/bash
# Find all places that import and use the old RepositoryItem
echo "Searching for imports of the old RepositoryItem..."
rg "import org\.craftercms\.studio\.api\.v1\.repository\.RepositoryItem" --type java

Length of output: 197

Action Required: Verify External Dependencies

The refactoring of RepositoryItem from a v1 class to a v2 Java record is a solid modernization that leverages newer Java features, resulting in cleaner and immutable code. Based on the internal search, we did not find any Java files importing
org.craftercms.studio.api.v1.repository.RepositoryItem. This indicates that the internal codebase appears fully migrated to the new v2 package.

Please note:

  • While internal references seem updated, this change is still breaking for any external consumers still referencing the v1 API.
  • It’s recommended to manually verify external dependencies (or other integration tests) to ensure that no client code remains dependent on the v1 RepositoryItem.
src/main/java/org/craftercms/studio/impl/v1/content/pipeline/AssetDmContentProcessor.java (1)

165-165:

Details

✅ Verification successful

Method parameter removed from persistItemAfterWrite call

The result.getCommitId() parameter has been removed from the call to itemServiceInternal.persistItemAfterWrite(). This change is part of the v1 ContentRepository removal, as commit IDs are likely now handled differently in the underlying implementation.

Ensure that the ItemServiceInternal interface and implementation have been updated accordingly to match this new method signature.

🏁 Script executed:

#!/bin/bash
# Check the signature of the persistItemAfterWrite method in ItemServiceInternal
echo "Looking for the persistItemAfterWrite method signature..."
rg "persistItemAfterWrite\s*\(\s*String\s+\w+\s*,\s*String\s+\w+\s*,\s*String\s+\w+\s*,\s*boolean\s+\w+\s*\)" --type java

Length of output: 439

PersistItemAfterWrite Signature Update Confirmed

The commit ID parameter has been correctly removed from the call in AssetDmContentProcessor.java (line 165). Verification confirms that the ItemServiceInternal signature is now:

persistItemAfterWrite(String siteId, String path, String username, boolean unlock)

No additional changes are necessary, but please ensure that all references to this method in the codebase and any associated tests have been updated accordingly.

src/main/java/org/craftercms/studio/impl/v2/service/content/internal/ContentServiceInternalImpl.java (10)

85-86: Security handling refactored to use static utility methods.

The code now uses SecurityUtils static methods instead of injected SecurityService, improving maintainability by centralizing security operations.

154-154: Current user retrieval refactored.

Changed from securityService.getCurrentUser() to static SecurityUtils.getCurrentUser() for consistency with the security handling refactoring across the codebase.

192-192: Consistent usage of SecurityUtils.

Security access pattern consistently updated to use the static utility method.

242-242: Security access centralized.

Continuing the pattern of replacing SecurityService with SecurityUtils static methods.

262-262: Current user retrieval standardized.

Part of the systematic removal of SecurityService dependency.

336-336: Updated method signature to include exception handling.

The method now correctly declares that it throws ServiceLayerException, which aligns with its implementation that can potentially throw this exception and matches the interface contract.

411-412: Authentication retrieval refactored.

Now using SecurityUtils.getAuthentication() for DeleteContentEvent publishing.

468-469: Security pattern consistently applied.

Replaced securityService.getCurrentUser() with the static utility method.

478-478: Event authentication consistently refactored.

Authentication for the LockContentEvent now uses the static SecurityUtils method.

502-502: Consistent authentication handling for events.

Authentication for the LockContentEvent (unlock operation) now uses the static SecurityUtils method.

src/main/java/org/craftercms/studio/impl/v2/service/content/internal/ContentTypeServiceInternalImpl.java (3)

41-41: Added import for SecurityUtils.

Required for the static getCurrentUser() method call that replaces securityService.getCurrentUser().

127-127: Updated method signature to explicitly throw ServiceLayerException.

Method signature now properly declares the exception it can throw, aligning with interface contract changes and improving error handling transparency.

131-132: Security handling refactored to use static utility method.

Replaced securityService.getCurrentUser() with SecurityUtils.getCurrentUser() for consistent security handling across the codebase, removing direct dependency on SecurityService.

src/main/java/org/craftercms/studio/impl/v2/service/publish/internal/PublishServiceInternalImpl.java (4)

76-77: Added static imports for SecurityUtils methods.

Imports the static getCurrentUser and getAuthentication methods, supporting the security handling refactoring.

233-233: Authentication retrieval refactored.

Using SecurityUtils.getAuthentication() instead of securityService.getAuthentication() for workflow event publishing.

337-337: Current user retrieval standardized.

Audit log creation now uses static SecurityUtils.getCurrentUser() method.

634-635: Event authentication consistently refactored.

WorkflowEvent creation now uses the static SecurityUtils.getAuthentication() method.

src/main/java/org/craftercms/studio/impl/v2/service/repository/internal/RepositoryManagementServiceInternalImpl.java (3)

92-92: Added static import for SecurityUtils.getCurrentUser().

Import supports the refactoring of security handling throughout the class.

788-788: Security access standardized for conflict resolution.

Changed from securityService.getCurrentUser() to the static utility method as part of removing SecurityService dependency.

948-948: Current user retrieval refactored for git commit operations.

Git commit functionality now uses the static SecurityUtils.getCurrentUser() method, consistent with the security handling refactoring.

src/main/java/org/craftercms/studio/api/v2/security/publish/PeerReviewCapableAnnotationHandler.java (1)

40-40: Well-implemented refactoring of security dependencies.

The removal of SecurityService dependency in favor of the SecurityUtils static utility method is a clean approach that reduces coupling while maintaining the same functionality. This change is consistent with the PR's objective of modernizing the codebase.

Also applies to: 53-56, 66-66

src/main/java/org/craftercms/studio/impl/v2/upgrade/providers/XmlFileVersionProvider.java (2)

25-25: API upgrade from v1 to v2 correctly implemented.

The import has been properly updated to use the GitContentRepository from the v2 package, aligning with the PR's objective of removing v1 ContentRepository.

104-118: Improved code structure with simplified control flow.

The conditional logic has been flattened and streamlined, making the method easier to read and maintain while preserving the same functionality. The early exception throw when "/config/studio" doesn't exist eliminates unnecessary nesting.

src/test/java/org/craftercms/studio/impl/v2/repository/blob/StudioBlobStoreResolverImplTest.java (1)

24-28: Import statements correctly updated for v2 API.

The test imports have been properly updated to reference the ContentRepository from the v2 package instead of v1, ensuring test compatibility with the refactored codebase. No functional changes were needed in the test itself.

Also applies to: 46-46

src/main/java/org/craftercms/studio/impl/v2/service/repository/RepositoryManagementServiceImpl.java (2)

2-2: Copyright year correctly updated to 2025.

The file header has been updated with the latest copyright year.

40-40: Security service dependency successfully refactored.

The removal of SecurityService dependency in favor of SecurityUtils.getCurrentUser() is well-implemented and maintains the same functionality while reducing coupling. This change is consistent with the broader refactoring effort across the codebase.

Also applies to: 56-58, 78-78

src/main/webapp/default-site/scripts/api/impl/site/SpringSiteServices.groovy (1)

92-107: Consistent with v1 ContentRepository removal

The commenting out of these remote repository management methods aligns properly with the PR objective to remove v1 ContentRepository. The methods have been commented out rather than deleted, which maintains code history while effectively removing the functionality.

src/main/webapp/default-site/scripts/api/SiteServices.groovy (1)

94-136: Properly commented out remote repository functionality

The complete removal of these remote repository methods (including their documentation) is consistent with removing the v1 ContentRepository functionality. This change correctly aligns with similar modifications made throughout the codebase.

src/main/java/org/craftercms/studio/api/v2/service/security/SecurityService.java (3)

2-2: Updated copyright year

The copyright year has been correctly updated to 2025.

21-21: More flexible type import

Changed from List to Collection import, which allows for more flexibility in the method signature.

35-35: Improved method parameter flexibility

Changed parameter type from List<NormalizedRole> to Collection<NormalizedRole>, which provides more flexibility by allowing any collection implementation to be passed as an argument, not just Lists.

src/main/java/org/craftercms/studio/impl/v2/service/item/internal/ItemServiceInternalImpl.java (1)

238-238:

Details

✅ Verification successful

Simplified method signature

The removal of the commitId parameter from the persistItemAfterWrite method signature simplifies the API and aligns with the PR objective to eliminate v1 ContentRepository dependencies. Ensure all call sites to this method have been updated accordingly.

🏁 Script executed:

#!/bin/bash
# Check if all calls to persistItemAfterWrite have been updated to remove the commitId parameter
rg -A 1 -B 1 "persistItemAfterWrite\(" --type java

Length of output: 3187

Simplified Method Signature Verified

The removal of the commitId parameter from the persistItemAfterWrite method signature has been successfully propagated. All call sites—as seen in the following files—now use the updated 4-parameter signature:

  • src/main/java/org/craftercms/studio/impl/v2/service/item/internal/ItemServiceInternalImpl.java (method definition)
  • src/main/java/org/craftercms/studio/impl/v2/service/configuration/internal/ConfigurationServiceInternalImpl.java
  • src/main/java/org/craftercms/studio/impl/v1/service/content/ContentServiceImpl.java
  • src/main/java/org/craftercms/studio/impl/v1/content/pipeline/FormDmContentProcessor.java
  • src/main/java/org/craftercms/studio/impl/v1/content/pipeline/AssetDmContentProcessor.java
  • src/main/java/org/craftercms/studio/api/v2/service/item/internal/ItemServiceInternal.java

These changes are in line with the PR objective to simplify the API and eliminate v1 ContentRepository dependencies.

src/main/java/org/craftercms/studio/api/v2/utils/GitRepositoryHelper.java (3)

57-58: Import addition aligns with SecurityService dependency removal

The addition of the SecurityUtils import supports the refactoring effort to eliminate direct dependencies on SecurityService in favor of utility methods.

1304-1324: New method for restoring file versions provides valuable functionality

The new restoreVersion method follows good practices:

  • Uses proper locking with generalLockService to prevent concurrent modifications
  • Provides clear error messages when restoration fails
  • Ensures locks are released in the finally block regardless of execution path

This implementation enhances the GitRepositoryHelper with file restoration capabilities and maintains thread safety.

1400-1402: Security implementation refactored to use utility method

Replaced direct dependency on securityService.getCurrentUser() with the static utility method SecurityUtils.getCurrentUser(), which aligns with the broader refactoring effort in the PR to reduce direct dependencies on SecurityService.

src/main/java/org/craftercms/studio/impl/v2/security/publish/PublishPackageAvailableActionResolverImpl.java (3)

36-36: Import statement updated for security refactoring

Static import of SecurityUtils.getCurrentUser() supports the removal of direct SecurityService dependency.

48-55: Constructor updated to remove SecurityService dependency

The constructor signature has been updated to remove the SecurityService parameter, aligning with the broader refactoring effort to eliminate direct dependencies on SecurityService across the codebase.

59-60: Method calls updated to use security utility

Changed direct calls to securityService.getCurrentUser() to use the static utility method getCurrentUser() while maintaining the same functionality. This is part of the consistent refactoring pattern to simplify security-related code.

Also applies to: 67-68

src/main/java/org/craftercms/studio/api/v2/repository/GitContentRepository.java (3)

375-382: New method added for retrieving content children

This new getContentChildren method enhances the repository interface by providing a way to list children of a specific folder path, which is a fundamental operation for content navigation and management.

384-395: Added revert content functionality with proper exception handling

The new revertContent method provides essential version control functionality:

  • Allows reverting content to a specific version
  • Includes documentation for potential exceptions
  • Returns the commit ID of the revert operation for traceability

This addition aligns with the file restoration capabilities added to GitRepositoryHelper and improves content management functionality.

138-139: Formatting improvements to method parameters

The parameter indentation has been standardized across multiple method signatures, improving code readability and consistency.

Also applies to: 248-248, 274-277

src/main/java/org/craftercms/studio/impl/v2/upgrade/providers/XmlStreamVersionProvider.java (2)

20-21: Updated import to use v2 repository API

Changed import from v1 to v2 GitContentRepository, which aligns with the PR objective to remove v1 ContentRepository. This is part of a broader effort to migrate the codebase to the v2 API.

39-40: Improved parameter formatting for constructor

The constructor parameter indentation has been adjusted for better readability while maintaining the same functionality. This is a minor formatting improvement.

src/main/java/org/craftercms/studio/api/v1/service/content/ContentTypeService.java (2)

62-62: Added exception handling to getAllContentTypes method signature

The method signature has been updated to include throws ServiceLayerException, which aligns with the error handling pattern used in other methods of this interface. This is a good improvement for consistency and proper error propagation.

64-64: Added exception handling to getAllowedContentTypesForPath method signature

The method signature has been updated to include throws ServiceLayerException, which aligns with the error handling pattern used in other methods of this interface (like getContentTypeByRelativePath in line 60). This change ensures consistent error handling across the service methods.

src/main/java/org/craftercms/studio/impl/v2/service/security/UserServiceImpl.java (10)

2-2: Updated copyright year

Copyright date has been updated to 2025, reflecting continued maintenance of the codebase.

51-51: Added SecurityUtils import to replace direct SecurityService dependency

Importing the new SecurityUtils class that will be used to replace direct SecurityService calls throughout the code. This is part of the larger refactoring to eliminate the SecurityService dependency.

102-102: Replaced groupServiceInternal with configurationService for getting site groups

The implementation now uses configurationService.getSiteGroups(siteId) instead of the previous implementation that likely used groupServiceInternal.getSiteGroups(siteId). This change aligns with the removal of direct dependencies on SecurityService in favor of other service classes.

305-305: Replaced securityService with securityServiceV2 for admin check

Now using securityServiceV2.isSystemAdmin(username) instead of the previous implementation that likely used securityService.isSystemAdmin(username). This is part of the larger refactoring to eliminate the SecurityService dependency.

309-309: Replaced groupServiceInternal with configurationService for getting site groups

Similar to line 102, this line now uses configurationService.getSiteGroups(siteId) instead of a previous implementation, maintaining consistency throughout the code.

350-350: Replaced securityService with securityServiceV2 for admin check

Similar to line 305, now using securityServiceV2.isSystemAdmin(username) for consistency throughout the code.

375-375: Replaced securityService.getAuthentication with SecurityUtils.getAuthentication

Now using the static utility method SecurityUtils.getAuthentication() instead of the instance method from securityService. This change centralizes authentication retrieval logic in the SecurityUtils class.

390-390: Replaced securityService.getAuthentication with SecurityUtils.getAuthentication

Similar to line 375, using SecurityUtils.getAuthentication() for consistency throughout the code.

576-576: Replaced securityService.getCurrentUser with SecurityUtils.getCurrentUser

Now using the static utility method SecurityUtils.getCurrentUser() instead of the instance method from securityService. This change centralizes user retrieval logic in the SecurityUtils class.

592-593: Replaced security service user role retrieval with direct service calls

The implementation has been updated to use SecurityUtils.getCurrentUser() and userServiceInternal.getUserGlobalRoles(currentUser) instead of previous implementations. This streamlines the code by removing the dependency on the SecurityService class.

src/main/java/org/craftercms/studio/impl/v2/repository/blob/s3/StudioAwsS3BlobStore.java (3)

281-285: Simplified moveContent method signature by removing newName parameter

The method signature has been simplified by removing the newName parameter, making the API more straightforward. The log message has been updated to reflect this change.

286-336: Refactored folder content move implementation

The implementation for moving folder content has been streamlined while maintaining the same error handling and core functionality. The code now directly attempts to copy files from the source path to the target path without additional checks for the now-removed newName parameter.

337-353: Refactored file content move implementation

The implementation for moving individual file content has been simplified while maintaining the same core functionality and error handling. The move operation is now more direct, with less conditional logic due to the removal of the newName parameter.

src/main/java/org/craftercms/studio/controller/rest/v2/SitesController.java (4)

19-19: Reorganized import for jakarta.validation.Valid

The import for jakarta.validation.Valid has been moved to a more appropriate location in the import section, improving code organization.

77-77: Simplified getAvailableBlueprints method implementation

The method implementation has been simplified by directly assigning the result of sitesService.getAvailableBlueprints() to blueprintDescriptors without a try-catch block. This reflects that the method signature for getAvailableBlueprints() in the SitesService interface now includes a throws ServiceLayerException clause, allowing exceptions to propagate up the call stack.

99-99: Improved parameter annotation formatting

The formatting of the @Valid annotation for the request parameter has been adjusted to maintain consistent indentation throughout the controller methods.

129-129: Improved parameter annotation formatting

Similar to line 99, the formatting of the @Valid annotation for the request parameter in the validatePolicy method has been adjusted to maintain consistent indentation.

src/main/java/org/craftercms/studio/impl/v1/content/pipeline/FormDmContentProcessor.java (1)

262-262: Method signature updated to remove commitId parameter

The itemServiceInternal.persistItemAfterWrite method call has been updated to remove the commitId parameter, simplifying the method invocation. This change aligns with the broader effort to remove v1 ContentRepository dependencies throughout the codebase.

src/test/java/org/craftercms/studio/impl/v2/service/policy/internal/PolicyServiceInternalImplTest.java (5)

24-27: Updated imports to use v2 repository components

Imports have been updated to use v2 repository classes and to include ServiceLayerException, aligning with the removal of v1 ContentRepository.

104-105: Updated exception type in setUp method

Changed exception type from ConfigurationException to ServiceLayerException to match the new exception hierarchy in the v2 repository implementation.

114-115: Removed v1 repository dependency from constructor

The PolicyServiceInternalImpl constructor now only receives the v2 contentRepository, removing the dependency on the v1 repository implementation.

146-166: Updated repository item construction and return type

The test method now:

  1. Uses the new RepositoryItem constructor that takes path, name, and folder status parameters
  2. Returns a List instead of an array
  3. Throws ServiceLayerException instead of previous exceptions

These changes properly align with the updated v2 repository API.

178-186: Consolidated repository usage

Updated content size retrieval methods to use the single contentRepository instance rather than the removed contentRepositoryV2, maintaining test functionality while adapting to the v2 API changes.

src/main/java/org/craftercms/studio/impl/v2/upgrade/StudioUpgradeManagerImpl.java (4)

31-34: Updated imports to use v2 repository classes

Imports have been updated to use v2 repository components and include necessary exceptions from the v1 API, supporting the removal of v1 ContentRepository.

88-93: Improved constructor formatting

The constructor has been reformatted for better readability while maintaining the same parameters and functionality.

212-213: Simplified repository existence check

Simplified the method to directly return the repository existence check result, improving code readability and maintainability.

231-263: Enhanced error handling in getExistingEnvironments method

The method has been updated to:

  1. Throw ServiceLayerException instead of the previous exception type
  2. Use Collection instead of RepositoryItem[] for better API consistency
  3. Use item.name() method calls instead of property access
  4. Add a try-catch block to handle ContentNotFoundException gracefully

These changes improve error handling and align with the v2 repository API.

src/main/java/org/craftercms/studio/impl/v2/service/dashboard/DashboardServiceImpl.java (3)

59-59: Added static import for SecurityUtils

Added static import for SecurityUtils.getCurrentUser() to support the removal of the SecurityService dependency.

79-84: Removed SecurityService dependency from constructor

The constructor properties and parameter list have been updated to remove the SecurityService dependency, simplifying the class and reducing its dependencies.

116-117: Replaced SecurityService with SecurityUtils

Replaced securityService.getCurrentUser() calls with the static getCurrentUser() method from SecurityUtils. This change is part of a broader refactoring to eliminate direct dependencies on SecurityService across the codebase.

Also applies to: 126-127

src/test/java/org/craftercms/studio/impl/v1/service/content/ContentServiceImplTest.java (9)

22-23: Updated imports to use v2 repository APIs.

The imports have been correctly updated to use GitContentRepository and RepositoryItem from the v2 package, aligning with the PR objective of removing the v1 ContentRepository.

31-32: Added Collection support for repository items.

The import has been updated to include java.util.Collection, which is required for the updated getRepoItems method that now returns a collection instead of an array. This change improves type flexibility and aligns with modern Java collection practices.

53-57: Simplified repository item creation and collection handling.

The getRepoItems method has been refactored to:

  1. Return a Collection<RepositoryItem> instead of an array
  2. Use a more streamlined approach to create RepositoryItem objects directly
  3. Use the toList() method for creating an immutable list

This change improves readability and maintainability while aligning with v2 API expectations.

61-62: Updated test to use collection instead of array.

The test method has been properly updated to accommodate the new Collection<RepositoryItem> return type from getRepoItems.

77-81: Test updates for collection handling.

The test method has been properly updated to work with the refactored getRepoItems method that now returns a collection.

98-102: Consistent updates to test method for collection handling.

The test method has been correctly updated to use the new Collection<RepositoryItem> return type from getRepoItems.

117-121: Test updates for collection handling in multiple collision scenario.

The test method has been updated appropriately to accommodate the new collection-based approach for repository items.

136-141: Folder cut/paste test updates for collection handling.

The test method has been properly updated to use the new collection-based repository item structure.

158-162: Page/page collision test updates for collection handling.

The test method has been correctly updated to use the new collection-based repository item approach.

src/main/java/org/craftercms/studio/impl/v2/service/policy/internal/PolicyServiceInternalImpl.java (5)

20-24: Updated imports for exception handling and repository item access.

The imports have been updated to include ServiceLayerException and v2 repository classes, aligning with the removal of v1 ContentRepository.

71-83: Simplified constructor to use only v2 GitContentRepository.

The constructor has been streamlined to remove the dependency on v1 ContentRepository. The @ConstructorProperties annotation and implementation correctly reflect this change, eliminating unnecessary complexity and duplication.

193-195: Updated repository item property access using method calls.

The code has been updated to use method calls (child.path(), child.name()) instead of direct field access for repository items, following proper encapsulation principles.

203-204: Updated folder check using method call.

The code now uses child.isFolder() method instead of direct field access, properly respecting the encapsulation of the RepositoryItem class.

206-208: Updated content size retrieval to use v2 repository.

The code has been updated to use the contentRepository instance for retrieving content size, properly aligning with the PR objective of removing v1 ContentRepository.

src/main/java/org/craftercms/studio/impl/v2/service/configuration/internal/ConfigurationServiceInternalImpl.java (9)

58-62: Added Spring annotations for dependency injection.

Added Spring annotations to properly manage dependency injection within the application context:

  • @Autowired for automatic dependency injection
  • @Lazy to support circular dependency resolution

This improves the Spring configuration and helps avoid potential initialization issues.

88-90: Replaced SecurityService dependency with static utility methods.

The code now uses static imports from SecurityUtils instead of depending directly on a SecurityService bean, reducing coupling and making the security-related operations more centralized.

399-402: Updated authentication retrieval to use static utility method.

The code now retrieves authentication using the static getAuthentication() method from SecurityUtils instead of calling the service directly, which aligns with the centralized security approach.

492-496: Updated current user retrieval to use static utility method.

The code now retrieves the current user using the static getCurrentUser() method from SecurityUtils instead of accessing the security service directly, maintaining consistency with the security handling approach.

580-582: Updated user retrieval in environment configuration method.

Consistently updated the method to use the static getCurrentUser() method from SecurityUtils instead of depending on the security service.

644-647: Updated global configuration user retrieval.

Consistently updated the global configuration method to use the static getCurrentUser() utility method instead of relying on security service.

718-725: Added new method to retrieve site groups.

Added a new convenience method getSiteGroups() that:

  1. Retrieves role mappings for a site
  2. Extracts the set of normalized groups from the mappings
  3. Properly converts the result to an ArrayList
  4. Includes appropriate exception handling

This method enhances the service API by providing a simpler way to get site groups.

842-846: Added @lazy and @Autowired annotations to setter method.

Updated the setter method for contentServiceInternal with @Lazy and @Autowired annotations to properly handle circular dependencies during application initialization.

872-876: Added @lazy and @Autowired annotations to itemServiceInternal setter.

Consistently applied the same annotations (@Lazy and @Autowired) to the itemServiceInternal setter method to properly handle circular dependencies.

src/main/java/org/craftercms/studio/api/v2/service/site/SitesService.java (3)

65-65: Updated method signature to include exception declaration.

The getAvailableBlueprints() method signature has been updated to include throws ServiceLayerException, properly indicating that this method may throw an exception during blueprint retrieval. This improves error handling and transparency for callers.

73-73: Enhanced error handling for blueprint descriptor retrieval.

Updated the getBlueprintDescriptor(String id) method signature to declare throws ServiceLayerException, improving error handling capabilities and making it clear to callers that exceptional conditions may occur.

81-81: Updated blueprint location retrieval to properly declare exceptions.

The getBlueprintLocation(String blueprintId) method signature now includes throws ServiceLayerException, which ensures consistent exception handling throughout the blueprint-related operations.

src/test/java/org/craftercms/studio/impl/v2/service/repository/internal/RepositoryManagementServiceInternalImplTest.java (5)

28-28: Good addition of the SecurityUtils import.
No issues found with adding this import for static method mocking.

34-34: Consider @BeforeAll if this is JUnit 5.
If you are still on JUnit 4, then @BeforeClass is correct.

39-39: Use of MockedStatic indicates advanced Mockito usage.
Ensure the project’s Mockito version supports static method mocking.

151-160: Static mocking of SecurityUtils for testCommitResolutionWithUncommittedChanges.
This is a valid approach for isolating user retrieval.

165-174: Static mocking of SecurityUtils for testCommitResolutionExceptionThrown.
Consistent approach to testing by simulating a missing user scenario.

src/main/java/org/craftercms/studio/impl/v2/service/site/SitesServiceImpl.java (3)

56-56: Added throws ServiceLayerException to getAvailableBlueprints.
Explicitly declaring the exception clarifies its occurrence.

61-61: Added throws ServiceLayerException to getBlueprintDescriptor.
Helps with explicit error handling in callers.

66-66: Added throws ServiceLayerException to getBlueprintLocation.
No further concerns; improves exception transparency.

src/main/java/org/craftercms/studio/api/v1/service/site/SiteService.java (1)

68-68: All specific creation exceptions replaced by ServiceLayerException.
This simplifies method signatures but may reduce granularity for error handling.

src/main/java/org/craftercms/studio/impl/v1/service/content/ContentServiceImpl.java (21)

59-59: Importing RepositoryItem from v2.
Aligns the class with new repository APIs.

73-73: Importing SecurityUtils.
Shifts to static utility for current user retrieval.

174-174: Delegating to contentRepository.contentExists.
Straightforward pass-through; no issues found.

187-187: Using contentRepository.shallowContentExists.
Clear naming clarifies the difference from full existence checks.

196-196: Special-case empty site usage.
Check if the empty string might cause confusion in multi-site contexts.

198-198: Direct call to repository’s getContent.
Implementation looks correct.

647-647: contentRepository.writeContent usage in writeContent(...).
Be mindful of concurrency or partial writes.

681-681: Publishing ContentEvent post-write.
Ensures event-based flows receive updates.

703-703: createFolder invocation.
Creating a folder with contentRepository is consistent with new structure.

765-765: String user = SecurityUtils.getCurrentUser().
Ensure consistent usage for auditing.

892-892: Publish ContentEvent when copying content.
Maintains a clear event-driven approach.

1013-1013: MoveContentEvent publication.
Keeps external systems in sync.

1039-1039: Retrieving current user from SecurityUtils.
No issues.

1656-1656: Reading contentPath from item.uri.
Straightforward property usage.

1670-1670: Fetching child repository items.
Implementation consistent with new v2 approach.

1684-1684: Checking for index file in child items.
Prevents double-loading folder index.

1696-1696: Folder check for childRepoItem.
Used for nested folder logic; looks fine.

2023-2026: New signature for revertContentItem.
All fallback exceptions collapsed into ServiceLayerException. Evaluate if clarity is lost.

2045-2045: Fetching username from SecurityUtils.
Standard approach for auditing.

2048-2048: persistItemAfterWrite for revert operation.
Updates item state in DB.

2067-2067: Publishing ContentEvent after revert.
Notifies listeners of the revert action.

src/main/java/org/craftercms/studio/impl/v1/service/site/SiteServiceImpl.java (5)

184-184: Ensure the unified exception signature is comprehensive.
You’ve consolidated the thrown exceptions into ServiceLayerException. Double-check that this covers all possible error states (e.g. BlueprintNotFoundException, entitlement failures, etc.) to avoid inadvertently swallowing important details.

266-266: Looks good: retrieving the last commit ID.
Calling contentRepository.getRepoLastCommitId(siteId) is consistent with the newly refactored repository methods. No immediate concerns.

492-492: Site creation from blueprint usage looks fine.
The call to contentRepository.createSiteFromBlueprint is logically consistent, with success/failure handled immediately afterwards.

599-599: Remote cloning integration is consistent.
The createSiteCloneRemote usage properly assigns success and follows the existing pattern for site creation.

671-671: Get last commit ID on cloned sites.
Retrieving the commit ID after cloning keeps your system updated with the correct revision. Looks aligned with the rest of the workflow.

src/main/java/org/craftercms/studio/impl/v2/service/security/SecurityServiceImpl.java (6)

2-2: Copyright year update.
No issues. The update from 2024 to 2025 is straightforward.

44-44: Static import for getAuthentication.
This change removes the need to fully qualify the method. Usage is clear, and it aligns with the new approach of delegating authentication retrieval to SecurityUtils.

60-60: Expanded flexibility with Collection<NormalizedRole>.
Switching from List<NormalizedRole> to a more generic Collection<NormalizedRole> is a good practice, allowing more flexibility in how roles are provided.

71-71: Same approach for loadUserPermission.
Similarly, changing the signature to accept any Collection<NormalizedRole> enhances reusability.

91-91: Use of Collection<NormalizedRole> for permission extraction.
This maintains consistency across the codebase for role handling and ensures method calls are more permissive in input argument types.

144-144: Validate new site groups retrieval logic.
Replacing groupServiceInternal.getSiteGroups(siteId) with configurationService.getSiteGroups(siteId) can change how groups are fetched. Ensure that the new approach keeps the same group data in sync with the rest of the application.

src/main/java/org/craftercms/studio/api/v2/repository/ContentRepository.java (7)

19-19: New import for ContentNotFoundException.
Integrating this existing exception aligns the code with your method signatures below.

21-21: New import for UserNotFoundException.
This exception is necessary for the updated repository methods. No issues.

23-23: InputStream import.
Also aligns with the new repository methods returning streams.

65-87: New default method for retrieving content.
Providing a default getContent(site, path) that calls the overloaded variant with shallow = false is a neat convenience. Check that your doc comments clearly explain the difference between shallow and non-shallow retrieval.

89-89: Add UserNotFoundException to createFolder.
Broadening the exception types clarifies possible error states for folder creation.

91-91: moveContent signature change.
Maintaining ServiceLayerException ensures uniform handling. Looks good.

93-93: Add UserNotFoundException to writeContent.
Now consistent with other management operations that may depend on valid user contexts.

src/main/java/org/craftercms/studio/impl/v2/service/site/internal/SitesServiceInternalImpl.java (10)

36-36: Import changes reflect the new repository structure.

The import of RepositoryItem from the v2 repository package indicates a shift away from the v1 repository implementation to the v2 API.

49-53: Good use of Spring annotations for dependency management.

Adding the @Autowired and @Lazy annotations for setter injection is appropriate here. The @Lazy annotation helps prevent circular dependency issues that might occur during application startup.

76-76: Using SecurityUtils is a good practice for authentication.

This static import for getCurrentUser() centralizes authentication logic in the SecurityUtils class rather than depending directly on SecurityService, which follows better separation of concerns.

91-91: Field now initialized through setter injection.

The itemServiceInternal field is now set via setter injection rather than constructor injection, which helps to break circular dependencies.

95-102: Constructor parameters updated to remove deprecated dependencies.

Removing securityService and contentRepository from the constructor parameters aligns with the PR objective of removing v1 ContentRepository.

121-125: Setter injection for ItemServiceInternal resolves circular dependencies.

Adding a setter method with @Autowired and @Lazy annotations is a good approach to break circular dependencies between services.

128-130: Method signature updates for better exception handling.

Adding throws ServiceLayerException to method signatures makes error handling more explicit and ensures that callers are aware of potential exceptions.

172-175: Repository method return type change for better collection handling.

Changing the return type from array to Collection<RepositoryItem> provides more flexibility and aligns with Java's collection framework best practices.

198-198: Improved logging statement with placeholder format.

Using the placeholder format in the logger statement improves readability and performance by avoiding string concatenation when logging is disabled.

418-418: Consistent use of SecurityUtils across multiple methods.

Replacing direct calls to securityService.getCurrentUser() with the static getCurrentUser() method from SecurityUtils improves consistency and follows the centralization of security-related functionality.

Also applies to: 465-465, 592-592

src/main/java/org/craftercms/studio/impl/v2/service/security/internal/UserServiceInternalImpl.java (6)

23-23: Updated imports align with architectural changes.

The addition of CollectionUtils, ConfigurationService and SecurityUtils imports reflect the shift in responsibility for group management and security operations.

Also applies to: 34-34, 37-37

66-67: Dependency injection change from GroupServiceInternal to ConfigurationService.

This change moves group management responsibility from GroupServiceInternal to ConfigurationService, centralizing configuration-related functionality.

Also applies to: 74-82

166-166: Updated service call for retrieving site groups.

The method now uses configurationService.getSiteGroups(siteId) instead of the previous groupServiceInternal call, maintaining functionality while aligning with the new architecture.

435-436: Using SecurityUtils for user authentication.

The consistent replacement of direct SecurityContext access with SecurityUtils.getCurrentUser() improves code maintainability and centralizes authentication logic.

Also applies to: 452-453, 471-472

494-516: New isSystemAdmin implementation with proper error handling.

This new implementation correctly checks if a user has the system admin role by retrieving global roles and comparing against the system admin normalized role. The method includes appropriate error handling for cases where the user is not found or there are issues retrieving site membership.

519-536: New getUserGlobalRoles method implementation.

This method retrieves the roles associated with a user based on their groups and the global role mappings. The implementation includes proper null checking and error handling, using the configuration service to access role mappings.

src/main/java/org/craftercms/studio/impl/v2/repository/blob/BlobAwareContentRepository.java (8)

87-87: Updated class declaration to implement StudioBlobAwareContentRepository.

This change reflects the move away from the v1 ContentRepository implementation, which aligns with the PR objective of removing v1 ContentRepository.

96-96: Consolidated repository field.

Replacing the separate localRepositoryV1 and localRepositoryV2 fields with a single localRepository of type GitPublishCapableRepository simplifies the code and removes the dual repository handling.

117-119: Updated setter method for the consolidated repository.

The setter method has been updated to set the single localRepository instead of both V1 and V2 repositories, which reflects the simplified repository structure.

165-165: Consistent repository method calls.

All method calls that previously referenced either localRepositoryV1 or localRepositoryV2 now consistently reference the unified localRepository, maintaining functionality while simplifying the code.

Also applies to: 181-181, 199-199, 212-212, 229-229, 249-249, 264-268, 272-272, 296-297, 322-323

363-368: Updated getContentChildren method for better collection handling.

The method now returns a Collection<RepositoryItem> instead of an array, which provides more flexibility and follows Java collection best practices. The implementation properly maps the items to maintain compatibility.

423-425: Path filtering for blob paths using stream operations.

The method efficiently filters item paths for blob paths using Java streams, which improves code readability and maintainability.

608-615: Updated repository method for file scanning.

The method now uses localRepository.forAllFileSitePaths to scan for blob paths, correctly filtering and processing only blob paths that match configured blob stores.

691-694: Using the consolidated repository for publishing operations.

The publishing methods now use the unified localRepository for publishing operations, which maintains functionality while aligning with the simplified repository structure.

src/main/java/org/craftercms/studio/impl/v2/repository/GitContentRepositoryImpl.java (12)

47-47: No issues with new import for RepositoryItem.

56-56: Straightforward addition of SecurityUtils.

535-542: shallowGetContent method looks correct.

This helper method effectively checks file existence and throws a ContentNotFoundException if missing. It’s a simple and clear approach.

544-550: Check for edge cases in getContent with shallow mode.

Implementation is straightforward. However, verify that calling getContent(site, path, HEAD) is valid in cases where the repository might lack a HEAD commit.

552-563: Ensure the returned TreeWalk is closed by the caller.

The getTreeWalkForPath() method returns a TreeWalk that must be closed to avoid resource leaks. Confirm all call sites properly close it within a try-with-resources or similar pattern.

1564-1599: getContentChildren method is well-structured.

The logic for ignoring internal Git files and returning valid children aligns with expectations. No immediate issues found.

1601-1643: createFolder approach is appropriate for Git-based systems.

Creating an empty file inside the folder ensures it shows up in Git. Error handling and commit logic appear solid.

1645-1666: Validate target path integrity in moveFiles.

You might want to confirm that the target path does not accidentally overwrite another file or folder. Ensure the calling code handles any naming collisions or partial moves.

1667-1706: moveContent effectively handles disk-level move and commit.

The approach (rename on disk, then addFiles, then commit) is consistent. The top-level lock prevents concurrency issues, so this looks good.

1708-1726: revertContent method is straightforward.

This cleanly restores a file to a previous commit, then commits with a user-supplied comment. No concerns here.

1728-1758: getContent by specific commit is clear.

Logic is consistent with the shallow variant. Throws ContentNotFoundException when absent, which is appropriate.

1760-1790: writeContent method handles file creation and commit properly.

The locking and commit flow help avoid concurrency issues. The usage of SecurityUtils.getCurrentUser() for commit messages is also a nice touch.

src/main/resources/crafter/studio/studio-services-context.xml (3)

347-347: Constructor injection of configurationServiceInternal is valid.

This change clarifies dependencies in userServiceInternal by explicitly passing the configuration service. No issues found.

769-769: Switching contentRepository reference to gitContentRepositoryV2.

Replacing the old repository reference with the v2 Git-based repository aligns with the removal of v1. Looks good.

791-791: Setting localRepository to gitContentRepositoryV2.

This ensures that the blob-aware repository delegates to the updated v2 Git repo. The configuration is consistent with the v2 approach.

src/test/java/org/craftercms/studio/impl/v2/repository/blob/BlobAwareContentRepositoryTest.java (11)

26-26: Import statement updates look fine.
No issues: the newly introduced imports (RepositoryItem and GitAPIException) align with the shift to v2 repository usage and JGit error handling.

Also applies to: 33-34

146-150: Mocking contentExists and getContent methods for v2 repository.
These changes appropriately replace any references to v1 with localRepositoryV2. The returned boolean values and content streams look reasonable for the test scenario.

165-166: Check return value from moveContent.
Returning EMPTY might be used to signal success, but confirm if an empty string is universally treated as success or if a dedicated success constant/enumeration would be clearer.

269-269: Use consistent moveContent verification.
Replacing localV1 with store and localRepositoryV2 is correct for the new approach. If the old line is firmly deprecated, consider removing commented references.

Also applies to: 271-272

279-281: Combined store + local repository move operations.
Verifying both store and localRepositoryV2.moveContent is consistent with the behavior for folder moves. No issues appear here.

284-286: Ensure clarity in getContentChildrenWithoutRemoteTest.
This test properly confirms that items without remote pointers are handled solely by localRepositoryV2. The usage of RepositoryItem is correct, and the assertions look good.

Also applies to: 288-288

291-292: Assertions on collection size and path.
The check confirms a single item with the expected path. Straightforward and correct.

296-298: Potential mismatch in pointer name.
The RepositoryItem is initialized with FilenameUtils.getName(POINTER_PATH) but tested against FilenameUtils.getName(ORIGINAL_PATH). Confirm that the final name is correctly translated from pointer to original file name, or adjust the test logic if intended.

Also applies to: 300-300, 303-305

487-490: getContentVersionHistoryTest with v2 repository.
The new mock for localRepositoryV2.getContentItemHistory correctly returns a list of ItemVersion. The test checks for size 2—this aligns with the updated approach for version history.

Also applies to: 492-493, 495-495, 498-498

501-501: fileWithoutExtensionTest logic.
Appending “.blob” for extensionless paths is tested correctly by verifying both store and localRepositoryV2 calls. Implementation appears valid.

Also applies to: 506-507

527-527: File ending changes.
No functional impact. Good to see the file remain syntactically correct.

src/main/java/org/craftercms/studio/impl/v1/service/content/ContentTypeServiceImpl.java (4)

37-38: Adoption of v2 imports.
These imports reflect the shift from v1 to v2 repository interfaces. The usage pattern aligns with the rest of the refactored code.

139-139: getAllContentTypes(site, boolean) ignoring the searchable parameter.
Currently the boolean is discarded. Confirm whether this is intentional or if further filtering should be applied.

146-146: New secure path validation.
Using @ValidateSecurePathParam is consistent with secure path handling. Looks correct.

251-277: reloadContentTypeConfigForChildren recursion

  1. The code repeatedly calls contentRepository.getContentChildren, then recursively processes subfolders. If there's a possibility of nested symbolic links or unusual referencing, guard against infinite recursion.
  2. The logic and exception handling look consistent with v2 usage.

Comment thread ...in/java/org/craftercms/studio/impl/v2/service/policy/internal/PolicyServiceInternalImpl.java
Comment thread src/main/java/org/craftercms/studio/impl/v1/service/content/ContentServiceImpl.java
// historically with MOVE it is not
String commitId = _contentRepository.moveContent(siteId, sourcePath, targetPath);
String commitId = contentRepository.moveContent(siteId, sourcePath, targetPath);

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 20, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Invoking contentRepository.moveContent.
If partial moves occur, consider locking or transaction to avoid conflicts.

- // Potential approach
+ // Consider implementing concurrency locks so that multi-file moves are atomic.

Committable suggestion skipped: line range outside the PR's diff.

Comment thread src/main/java/org/craftercms/studio/impl/v1/service/site/SiteServiceImpl.java Outdated
Comment thread src/main/java/org/craftercms/studio/impl/v2/repository/GitContentRepositoryImpl.java Outdated
Comment thread src/main/java/org/craftercms/studio/impl/v1/service/content/ContentTypeServiceImpl.java
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 20, 2025

Walkthrough

This extensive update removes deprecated API endpoints, interfaces, and methods across multiple modules. Several functionalities have been eliminated from content versioning, file‐to‐folder conversions, remote repository operations, and transaction handling. Service interfaces now favor a unified exception strategy using ServiceLayerException, and many repository contracts have migrated from v1 to v2, including changing data types from arrays to collections. Security handling has been refactored to remove direct SecurityService dependencies in favor of static utility calls via SecurityUtils. Additionally, Spring configuration and Groovy scripts have been simplified by removing legacy bean properties and remote operations.

Changes

Files/Groups Change Summary
API Definition
src/main/api/studio-api.yaml		 Removed the /api/1/services/api/1/content/get-item-versions.json GET endpoint and all its related definitions.
Deprecated Methods & Interfaces (v1)
src/main/java/org/craftercms/studio/api/v1/...		 Removed methods such as fileToFolder in DmContentProcessor, the entire ContentRepository and GitContentRepository interfaces, and several methods in ContentService (e.g. version history, delete candidates, remote push/pull). Also, removed DependencyDiffService and methods in DependencyService.
Service Interface Updates (v1 → v2)
src/main/java/org/craftercms/studio/api/v1/service/...
src/main/java/org/craftercms/studio/api/v2/service/...		 Updated method signatures to throw ServiceLayerException instead of specific exceptions; removed remote management and blueprint descriptor methods from SiteService; deprecation messages in SecurityService were updated to point to new utility classes.
Repository & Model Changes
src/main/java/org/craftercms/studio/api/v2/repository/...		 Introduced new default and overloaded getContent methods; updated signatures of methods such as createFolder and writeContent to include additional exceptions; removed getRepoFirstCommitId and converted RepositoryItem to a Java record.
Implementation Refactoring
src/main/java/org/craftercms/studio/impl/...		 Replaced v1 repository implementations with v2 versions; removed TransactionServiceImpl and legacy GitContentRepositoryImpl; updated calls to use the new repository methods and refactored security calls, replacing SecurityService with static calls from SecurityUtils; simplified method calls in content processing and file management.
Security & Utility Updates
src/main/java/org/craftercms/studio/impl/v2/utils/...		 Added a new restoreVersion method to GitCli; introduced a new SecurityUtils class with static methods getCurrentUser() and getAuthentication().
Spring XML Configuration
src/main/resources/crafter/studio/...		 Removed bean properties referencing securityService, dependencyDiffService, and obsolete repository beans; updated repository property from gitContentRepository to gitContentRepositoryV2.
Groovy Script Changes
src/main/webapp/default-site/scripts/api/...		 Removed or commented out methods for locking, retrieving version history, and remote push/pull operations in both content and site service scripts, and deleted the get-item-versions.get.groovy script.
Test Updates
src/test/...		 Updated test files to import repository v2 classes; method signatures and return types changed from arrays to collections; replaced SecurityService mocks with static mocks for SecurityUtils and removed unnecessary dependencies.

Sequence Diagram(s)

sequenceDiagram
    participant Client as API Client
    participant CTL as ContentController
    participant CS as ContentService
    participant CR as ContentRepository (v2)
    participant Sec as SecurityUtils

    Client->>CTL: listQuickCreateContent(siteId)
    CTL->>CS: getQuickCreatableContentTypes(siteId)
    CS->>CR: getContent(site, path, shallow=false)
    CR-->>CS: InputStream / Repository Items
    CS->>Sec: getCurrentUser()
    Sec-->>CS: current user information
    CS-->>CTL: Response with content data and user details
    CTL-->>Client: Return result
Loading

Another diagram illustrating dependency removal in security:

sequenceDiagram
    participant Impl as Service Implementation
    participant OldSec as SecurityService (removed)
    participant NewSec as SecurityUtils

    %% Old flow (now removed)
    %% Impl->>OldSec: getCurrentUser()
    %% OldSec-->>Impl: current user

    Impl->>NewSec: getCurrentUser()
    NewSec-->>Impl: current user
Loading

These diagrams capture the high-level interactions introduced by the refactoring changes.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot]
coderabbitai Bot reviewed Mar 20, 2025
View reviewed changes
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (10)
src/main/java/org/craftercms/studio/impl/v2/service/security/internal/UserServiceInternalImpl.java (3)

452-452: Security consistency
Replacing usage of direct security calls with SecurityUtils is consistent. Confirm that tests are updated to ensure coverage of changes in the current user retrieval logic.

492-515: Add method-level JavaDoc
The isSystemAdmin(String username) method is a valuable addition for centralized role checks. Consider adding explicit JavaDoc or in-code comments explaining the logic for easier maintainability.

519-536: Global roles retrieval
getUserGlobalRoles(String username) centralizes role resolution. Verify that results are appropriately cached if repeatedly invoked for the same user, especially for large-scale usage.

src/main/java/org/craftercms/studio/impl/v2/service/site/internal/SitesServiceInternalImpl.java (2)

128-132: Method signature & usage
getAvailableBlueprints() now throws ServiceLayerException and leverages Collection<RepositoryItem>. This is an improvement over arrays. Confirm that all callers handle the new exception properly.

172-173: Refactored getBlueprintsFolders
This new method provides better modularity and clarity. Check that any relevant caching or concurrency concerns are addressed if the blueprint folder list is accessed frequently.

src/main/java/org/craftercms/studio/impl/v1/service/content/ContentTypeServiceImpl.java (2)

136-136: Exception specification
Adding throws ServiceLayerException to getAllContentTypes and getAllowedContentTypesForPath clarifies the error surface. Remember to add negative test cases or mocks in unit tests for these exceptions.

Also applies to: 146-146

223-248: Refactor getAllContentTypes

  1. Replacing arrays with Collection<RepositoryItem> simplifies iteration and improves clarity.
  2. Checking for folders == null before iteration is good defensive coding.
  3. The path manipulations (line 233~) that combine folder.path() and folder.name() must consistently apply a file separator to avoid potential path issues on different platforms.
  4. Consider potential concurrency concerns if multiple threads retrieve content types simultaneously.
src/main/java/org/craftercms/studio/impl/v2/repository/GitContentRepositoryImpl.java (2)

205-209: Remove or justify commented-out code.
The lines referencing fromEmptyRepo are fully commented. If this logic is no longer needed, consider removing it rather than leaving it commented out.

-// boolean fromEmptyRepo = isEmpty(commitIdFrom);
-// String firstCommitId = getRepoFirstCommitId(site);
// ...

219-248: Consider removing legacy v1 references.
These commented blocks appear to reference an older workflow for computing diff entries from the “first commit.” If no longer relevant, removing them can reduce clutter.

-// if (fromEmptyRepo) {
-//   ...
-//   // operations.addAll(processDiffEntry(...))
-// }
src/test/java/org/craftercms/studio/impl/v2/repository/blob/BlobAwareContentRepositoryTest.java (1)

218-218: Remove outdated references to localV1.
These lines comment out localV1 usage. If the v1 repository is removed, consider deleting these lines.

-// verify(localV1).writeContent( ... )

Also applies to: 232-232, 272-272

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1a76424 and 76c72d1.

📒 Files selected for processing (82)
  • src/main/api/studio-api.yaml (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/content/pipeline/DmContentProcessor.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/repository/ContentRepository.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/repository/GitContentRepository.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/content/ContentService.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/content/ContentTypeService.java (1 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/dependency/DependencyDiffService.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/dependency/DependencyService.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/security/SecurityService.java (3 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/site/SiteService.java (2 hunks)
  • src/main/java/org/craftercms/studio/api/v1/service/transaction/TransactionService.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v2/repository/ContentRepository.java (2 hunks)
  • src/main/java/org/craftercms/studio/api/v2/repository/GitContentRepository.java (5 hunks)
  • src/main/java/org/craftercms/studio/api/v2/repository/RepositoryItem.java (2 hunks)
  • src/main/java/org/craftercms/studio/api/v2/repository/blob/StudioBlobAwareContentRepository.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v2/repository/blob/StudioBlobStore.java (2 hunks)
  • src/main/java/org/craftercms/studio/api/v2/security/publish/PeerReviewCapableAnnotationHandler.java (3 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/config/ConfigurationService.java (1 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/content/ContentService.java (1 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/content/internal/ContentTypeServiceInternal.java (1 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/item/internal/ItemServiceInternal.java (1 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/security/SecurityService.java (3 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/security/internal/GroupServiceInternal.java (0 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/security/internal/UserServiceInternal.java (2 hunks)
  • src/main/java/org/craftercms/studio/api/v2/service/site/SitesService.java (1 hunks)
  • src/main/java/org/craftercms/studio/api/v2/upgrade/StudioUpgradeManager.java (2 hunks)
  • src/main/java/org/craftercms/studio/api/v2/utils/GitRepositoryHelper.java (3 hunks)
  • src/main/java/org/craftercms/studio/controller/rest/v2/ContentController.java (1 hunks)
  • src/main/java/org/craftercms/studio/controller/rest/v2/SitesController.java (4 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/content/pipeline/AssetDmContentProcessor.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/content/pipeline/FormDmContentProcessor.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/executor/ProcessContentExecutorImpl.java (2 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/repository/git/GitContentRepositoryImpl.java (0 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/service/content/ContentServiceImpl.java (31 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/service/content/ContentTypeServiceImpl.java (3 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/service/dependency/DependencyDiffServiceImpl.java (0 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/service/dependency/DependencyServiceImpl.java (0 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/service/site/SiteServiceImpl.java (12 hunks)
  • src/main/java/org/craftercms/studio/impl/v1/service/transaction/TransactionServiceImpl.java (0 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/repository/GitContentRepositoryImpl.java (5 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/repository/RepositoryStartupCleanup.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/repository/blob/BlobAwareContentRepository.java (25 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/repository/blob/StudioBlobStoreResolverImpl.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/repository/blob/s3/StudioAwsS3BlobStore.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/security/publish/PublishPackageAvailableActionResolverImpl.java (2 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/configuration/ConfigurationServiceImpl.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/configuration/internal/ConfigurationServiceInternalImpl.java (9 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/content/ContentServiceImpl.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/content/internal/ContentServiceInternalImpl.java (10 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/content/internal/ContentTypeServiceInternalImpl.java (2 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/dashboard/DashboardServiceImpl.java (5 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/item/internal/ItemServiceInternalImpl.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/policy/internal/PolicyServiceInternalImpl.java (5 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/publish/internal/PublishServiceInternalImpl.java (5 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/repository/RepositoryManagementServiceImpl.java (4 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/repository/internal/RepositoryManagementServiceInternalImpl.java (4 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/security/SecurityServiceImpl.java (6 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/security/UserServiceImpl.java (9 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/security/internal/GroupServiceInternalImpl.java (0 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/security/internal/UserServiceInternalImpl.java (8 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/site/SitesServiceImpl.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/site/internal/SitesServiceInternalImpl.java (11 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/service/workflow/internal/WorkflowServiceInternalImpl.java (3 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/upgrade/StudioUpgradeManagerImpl.java (5 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/upgrade/providers/SiteVersionProvider.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/upgrade/providers/XmlFileVersionProvider.java (2 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/upgrade/providers/XmlStreamVersionProvider.java (2 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/utils/git/GitCli.java (1 hunks)
  • src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1 hunks)
  • src/main/resources/crafter/studio/security/common.xml (0 hunks)
  • src/main/resources/crafter/studio/studio-services-context.xml (3 hunks)
  • src/main/webapp/default-site/scripts/api/ContentServices.groovy (0 hunks)
  • src/main/webapp/default-site/scripts/api/SiteServices.groovy (1 hunks)
  • src/main/webapp/default-site/scripts/api/impl/content/SpringContentServices.groovy (0 hunks)
  • src/main/webapp/default-site/scripts/api/impl/site/SpringSiteServices.groovy (1 hunks)
  • src/main/webapp/default-site/scripts/rest/api/1/content/get-item-versions.get.groovy (0 hunks)
  • src/test/java/org/craftercms/studio/impl/v1/service/content/ContentServiceImplTest.java (7 hunks)
  • src/test/java/org/craftercms/studio/impl/v2/repository/blob/BlobAwareContentRepositoryTest.java (8 hunks)
  • src/test/java/org/craftercms/studio/impl/v2/repository/blob/StudioBlobStoreResolverImplTest.java (2 hunks)
  • src/test/java/org/craftercms/studio/impl/v2/service/policy/internal/PolicyServiceInternalImplTest.java (4 hunks)
  • src/test/java/org/craftercms/studio/impl/v2/service/repository/internal/RepositoryManagementServiceInternalImplTest.java (2 hunks)
  • src/test/java/org/craftercms/studio/impl/v2/service/site/internal/SitesServiceInternalImplTest.java (2 hunks)
💤 Files with no reviewable changes (19)
  • src/main/java/org/craftercms/studio/api/v2/repository/blob/StudioBlobAwareContentRepository.java
  • src/main/java/org/craftercms/studio/api/v1/content/pipeline/DmContentProcessor.java
  • src/main/java/org/craftercms/studio/api/v2/service/security/internal/GroupServiceInternal.java
  • src/main/java/org/craftercms/studio/api/v1/repository/GitContentRepository.java
  • src/main/java/org/craftercms/studio/impl/v2/service/security/internal/GroupServiceInternalImpl.java
  • src/main/java/org/craftercms/studio/api/v1/service/dependency/DependencyService.java
  • src/main/java/org/craftercms/studio/impl/v1/service/dependency/DependencyDiffServiceImpl.java
  • src/main/java/org/craftercms/studio/api/v1/service/transaction/TransactionService.java
  • src/main/resources/crafter/studio/security/common.xml
  • src/main/webapp/default-site/scripts/rest/api/1/content/get-item-versions.get.groovy
  • src/main/java/org/craftercms/studio/impl/v1/repository/git/GitContentRepositoryImpl.java
  • src/main/java/org/craftercms/studio/api/v1/service/dependency/DependencyDiffService.java
  • src/main/java/org/craftercms/studio/impl/v1/service/transaction/TransactionServiceImpl.java
  • src/main/java/org/craftercms/studio/api/v1/service/content/ContentService.java
  • src/main/webapp/default-site/scripts/api/ContentServices.groovy
  • src/main/java/org/craftercms/studio/api/v1/repository/ContentRepository.java
  • src/main/webapp/default-site/scripts/api/impl/content/SpringContentServices.groovy
  • src/main/java/org/craftercms/studio/impl/v1/service/dependency/DependencyServiceImpl.java
  • src/main/api/studio-api.yaml
🧰 Additional context used
🧬 Code Definitions (15)
src/main/java/org/craftercms/studio/api/v2/security/publish/PeerReviewCapableAnnotationHandler.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/workflow/internal/WorkflowServiceInternalImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v1/executor/ProcessContentExecutorImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/content/internal/ContentServiceInternalImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/repository/internal/RepositoryManagementServiceInternalImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/dashboard/DashboardServiceImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/test/java/org/craftercms/studio/impl/v2/service/repository/internal/RepositoryManagementServiceInternalImplTest.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/configuration/internal/ConfigurationServiceInternalImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/api/v2/utils/GitRepositoryHelper.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/service/security/SecurityServiceImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v2/security/publish/PublishPackageAvailableActionResolverImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/test/java/org/craftercms/studio/impl/v2/repository/blob/BlobAwareContentRepositoryTest.java (1)
src/main/java/org/craftercms/studio/impl/v2/repository/GitContentRepositoryImpl.java (1) (1)
  • GitContentRepositoryImpl (112-1855)
src/main/java/org/craftercms/studio/impl/v1/service/content/ContentTypeServiceImpl.java (1)
src/main/java/org/craftercms/studio/api/v1/constant/StudioConstants.java (1) (1)
  • StudioConstants (28-257)
src/main/java/org/craftercms/studio/impl/v2/service/site/internal/SitesServiceInternalImpl.java (1)
src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1) (1)
  • SecurityUtils (27-67)
src/main/java/org/craftercms/studio/impl/v1/service/content/ContentServiceImpl.java (2)
src/main/java/org/craftercms/studio/api/v2/utils/StudioUtils.java (1) (1)
  • StudioUtils (35-127)
src/main/java/org/craftercms/studio/impl/v2/utils/TimeUtils.java (1) (1)
  • TimeUtils (25-142)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (227)
src/main/java/org/craftercms/studio/impl/v2/utils/git/GitCli.java (1)

445-462: Well-designed implementation of file version restoration.

The new restoreVersion method follows the established patterns in the class with proper error handling and detailed documentation. The method correctly implements Git's restore functionality to checkout a file from a specific commit to the working directory.

src/main/java/org/craftercms/studio/api/v2/service/config/ConfigurationService.java (1)

252-261: Good enhancement to the ConfigurationService interface.

The addition of the getSiteGroups method provides a clear and well-documented way to access site-specific groups mapped in the role-mappings-config.xml. This change aligns with the overall objective of removing the v1 ContentRepository by migrating functionality to appropriate services.

src/main/java/org/craftercms/studio/impl/v1/executor/ProcessContentExecutorImpl.java (2)

21-24: Good refactoring of imports to support static utility usage.

The import changes properly support the transition from direct SecurityService dependency to using the SecurityUtils static methods, which is consistent with the PR's objective of code modernization.

Also applies to: 29-30, 33-34, 39-39

55-56: Improved security access implementation.

The replacement of securityService.getCurrentUser() with the static getCurrentUser() method from SecurityUtils aligns with the goal of removing direct SecurityService dependencies in favor of utility-based access.

Note that the securityService field is still declared and has accessor methods, though it's no longer used in this method. This should be fine as the field might be used elsewhere or by other components through dependency injection.

src/main/java/org/craftercms/studio/impl/v2/repository/blob/StudioBlobStoreResolverImpl.java (1)

29-30: Updated import to use v2 ContentRepository.

This change correctly updates the import from v1 to v2 ContentRepository, which is essential for the removal of the v1 ContentRepository as outlined in the PR objectives.

src/main/java/org/craftercms/studio/impl/v2/service/configuration/ConfigurationServiceImpl.java (1)

181-184: Properly implemented the getSiteGroups method.

The implementation follows the established pattern in this class by delegating to the configurationServiceInternal instance. This maintains consistency with other methods in the class.

Unlike other methods in this class, this implementation doesn't include permission annotations like @HasPermission. Consider whether this method should have permission restrictions applied to maintain security consistency.

src/main/java/org/craftercms/studio/api/v2/service/content/ContentService.java (1)

67-67: Standardized exception handling approach.

The method signature now throws ServiceLayerException instead of the more specific SiteNotFoundException. This aligns with the PR objective of removing v1 ContentRepository and standardizing exception handling across the codebase.

src/main/java/org/craftercms/studio/api/v2/upgrade/StudioUpgradeManager.java (2)

22-22: Added required import for ServiceLayerException.

The import was correctly added to support the method signature change below.

70-70: Updated method signature to include ServiceLayerException.

Method signature has been updated to declare that it can throw ServiceLayerException, aligning with the overall refactoring effort to standardize exception handling across the API.

src/main/java/org/craftercms/studio/impl/v2/repository/RepositoryStartupCleanup.java (1)

102-105: Improved error handling with null check.

This defensive programming addition prevents a potential NullPointerException by checking if the repository is null before performing operations on it. The appropriate warning message is logged to aid in troubleshooting.

src/main/java/org/craftercms/studio/controller/rest/v2/SitesController.java (4)

19-19: Repositioned import.

The import for jakarta.validation.Valid has been moved to a different position in the import section. This change doesn't affect functionality but improves code organization.

77-77: Simplified exception handling.

The try-catch block that previously wrapped the sitesService.getAvailableBlueprints() call has been removed, streamlining the code. This simplification is possible because the method now properly declares that it throws ServiceLayerException, which is then propagated to the caller.

99-99: Improved code formatting.

The annotation positioning has been standardized for better readability, placing @Valid and @RequestBody on the same line.

129-129: Consistent annotation formatting.

Formatting has been standardized to match other similar parameter declarations in the controller, improving overall code consistency.

src/main/java/org/craftercms/studio/api/v2/service/content/internal/ContentTypeServiceInternal.java (1)

38-38: Method signature updated to handle exceptions uniformly.

The getQuickCreatableContentTypes method now properly declares that it throws ServiceLayerException, which aligns with the exception handling pattern used by other methods in this interface. This change improves consistency and makes error handling expectations clearer for implementers.

src/main/java/org/craftercms/studio/api/v2/service/security/internal/UserServiceInternal.java (3)

24-25: Import added for new functionality.

The addition of the NormalizedRole import supports the new getUserGlobalRoles method added to this interface.

28-28: Import updated for Collection interface.

The import for Collection has been added to support the return type of the new method.

187-193: New method added to retrieve user global roles.

The addition of the getUserGlobalRoles method provides a standardized way to retrieve global roles for a specific user, which enhances the security management capabilities of the service. The method follows consistent exception handling patterns by declaring both ServiceLayerException and UserNotFoundException.

src/main/java/org/craftercms/studio/impl/v2/upgrade/providers/SiteVersionProvider.java (1)

20-21: Updated import to use v2 repository.

The import has been updated to use GitContentRepository from the v2 package instead of v1, aligning with the PR objective to remove v1 ContentRepository dependencies.

src/main/java/org/craftercms/studio/api/v2/repository/blob/StudioBlobStore.java (2)

22-23: Updated import to use v2 ContentRepository.

The import has been updated to use ContentRepository from the v2 package, which is consistent with the PR's objective to remove v1 ContentRepository dependencies.

35-35: Simplified interface extension.

The StudioBlobStore interface now extends only BlobStore and the v2 ContentRepository, removing redundant inheritance and simplifying the class hierarchy.

src/main/webapp/default-site/scripts/api/impl/site/SpringSiteServices.groovy (1)

92-107: Appropriate removal of deprecated remote repository functionality

The commenting out of remote repository methods (addRemote, removeRemote, and listRemote) aligns with the PR's objective to remove the v1 ContentRepository. This change ensures these deprecated functions are no longer accessible.

src/test/java/org/craftercms/studio/impl/v2/service/site/internal/SitesServiceInternalImplTest.java (2)

2-2: Copyright year updated correctly

The copyright year has been updated to 2025, maintaining consistency with other files in the codebase.

126-126: Properly initialized ItemServiceInternal dependency

The addition of sitesServiceInternal.setItemServiceInternal(itemServiceInternal) ensures that the test has all required dependencies properly initialized, which is necessary after the removal of the SecurityService dependency.

src/main/webapp/default-site/scripts/api/SiteServices.groovy (1)

94-136: Consistent removal of remote repository API methods

The commented-out methods (addRemote, removeRemote, and listRemote) along with their documentation align with the overall objective of removing v1 ContentRepository functionality. This change is consistent with similar modifications in other files like SpringSiteServices.groovy.

src/main/java/org/craftercms/studio/impl/v2/service/workflow/internal/WorkflowServiceInternalImpl.java (3)

2-2: Copyright year updated correctly

The copyright year has been updated to 2025, maintaining consistency with other files in the codebase.

58-58: Appropriate static import for authentication utility

The addition of the static import for getAuthentication from SecurityUtils supports the architectural shift from instance-based security service to static utility methods.

176-176: Refactored to use static SecurityUtils method

Replaced securityService.getAuthentication() with the static getAuthentication() method, which aligns with the broader effort to remove direct SecurityService dependencies in favor of static utility calls. This enhances consistency across the codebase.

src/main/java/org/craftercms/studio/impl/v2/service/content/internal/ContentTypeServiceInternalImpl.java (3)

41-41: Added import for SecurityUtils utility class.

This import supports the migration from direct SecurityService dependency to using the static utility class for security operations, which is part of the broader refactoring to remove v1 ContentRepository dependencies.

127-127: Method signature updated to include ServiceLayerException.

The method signature now includes throws ServiceLayerException, aligning with the updated interface and providing consistent exception handling across the service layer.

131-132:

Details

✅ Verification successful

SecurityService direct use replaced with SecurityUtils utility.

The code now uses SecurityUtils.getCurrentUser() instead of directly accessing the user through the security service. This change is part of the refactoring to remove direct dependencies on SecurityService in favor of static utility calls.

🏁 Script executed:

#!/bin/bash
# Verify consistent usage of SecurityUtils across the codebase
rg -A 1 "SecurityUtils.getCurrentUser" --type java

Length of output: 7687

Refactoring Verified: Consistent Usage of SecurityUtils

The updated code now correctly uses SecurityUtils.getCurrentUser() to retrieve the current user, in line with the repository-wide refactoring. Although the call to retrieve permissions remains on the securityService, the user retrieval no longer relies on the service directly, which is consistent with the desired pattern.

  • Confirmed changes in src/main/java/org/craftercms/studio/impl/v2/service/content/internal/ContentTypeServiceInternalImpl.java (lines 131-132) reflect the new approach.
  • Verification across multiple files shows consistent use of SecurityUtils.getCurrentUser().
src/main/java/org/craftercms/studio/api/v2/security/publish/PeerReviewCapableAnnotationHandler.java (3)

40-41: Added static import for SecurityUtils methods.

Adding a static import for the getAuthentication() method from SecurityUtils supports the refactoring to remove the direct SecurityService dependency.

53-55: Simplified constructor by removing SecurityService dependency.

The constructor no longer requires the SecurityService parameter, streamlining dependency management within the class. This change aligns with the broader refactoring to remove direct SecurityService dependencies from various components.

66-66: Updated user authentication retrieval method.

The code now uses the static getAuthentication().getPrincipal() method instead of accessing it through the SecurityService instance. This change maintains the same functionality while adhering to the new utility-based approach for security operations.

src/main/java/org/craftercms/studio/impl/v1/content/pipeline/AssetDmContentProcessor.java (1)

165-165:

Details

✅ Verification successful

Removed commitId parameter from persistItemAfterWrite method call.

The method call to persistItemAfterWrite has been simplified by removing the commitId parameter, aligning with changes to the ItemServiceInternal interface. This simplification likely corresponds to the removal of the v1 ContentRepository which handled commit IDs differently.

🏁 Script executed:

#!/bin/bash
# Look for the ItemServiceInternal interface definition to confirm parameter changes
rg -A 3 "persistItemAfterWrite.*\(" --type java

Length of output: 4188

Verified: CommitId Parameter Removal Confirmed

The removal of the commitId parameter from the persistItemAfterWrite call has been verified against the updated ItemServiceInternal interface. All occurrences now correctly reflect the four-parameter signature (String siteId, String path, String username, boolean unlock), including the one in AssetDmContentProcessor.

  • Confirmed in src/main/java/org/craftercms/studio/impl/v2/service/item/internal/ItemServiceInternalImpl.java and other related files.
  • The change aligns with the updated API and the removal of the deprecated v1 ContentRepository handling.
src/main/java/org/craftercms/studio/controller/rest/v2/ContentController.java (1)

101-101: Updated exception handling to use ServiceLayerException.

The method now throws a broader ServiceLayerException instead of the more specific SiteNotFoundException. This change aligns with updates to the service layer implementations and enables more comprehensive error handling.

src/main/java/org/craftercms/studio/impl/v2/utils/security/SecurityUtils.java (1)

1-67: Well-structured utility class for centralized security operations

This newly introduced utility class effectively centralizes security-related functionality, making it easier to retrieve authentication information throughout the application. The implementation properly handles edge cases like null security contexts and anonymous users, with a fallback mechanism for cron jobs.

The static methods align with the refactoring goal of removing direct SecurityService dependencies in favor of utility calls.

src/main/java/org/craftercms/studio/api/v1/service/security/SecurityService.java (3)

2-2: Copyright year updated

Updated the copyright year to 2025.

37-37: Updated deprecation message to reference new utility class

The deprecation message now correctly points to the new SecurityUtils utility class instead of the v2 service implementation, aligning with the architectural changes being made.

46-46: Updated deprecation message to reference new utility class

The deprecation message now correctly points to the new SecurityUtils utility class instead of the v2 service implementation, aligning with the architectural changes being made.

src/main/java/org/craftercms/studio/api/v2/service/security/SecurityService.java (3)

2-2: Copyright year updated

Updated the copyright year to 2025.

21-21: Imported Collection interface

Added import for java.util.Collection to support the updated method parameter type.

35-35: Improved method signature with more flexible parameter type

Changed the parameter type from List<NormalizedRole> to Collection<NormalizedRole>, making the API more flexible and allowing callers to pass any Collection implementation (Set, Queue, etc.) without conversion.

src/main/java/org/craftercms/studio/api/v2/service/item/internal/ItemServiceInternal.java (1)

148-149:

Details

✅ Verification successful

Simplified method signature by removing commitId parameter

The persistItemAfterWrite method signature has been simplified by removing the commitId parameter. This change aligns with the broader refactoring effort to remove the v1 ContentRepository and simplify API interfaces.

Make sure to verify that all callers of this method have been updated to use the new signature.

🏁 Script executed:

#!/bin/bash
# Find all calls to persistItemAfterWrite to ensure they've been updated
rg -p "persistItemAfterWrite\s*\(" --type java

Length of output: 1209

Action: persistItemAfterWrite signature update verified

The commit removing the commitId parameter has been confirmed. All callers now correctly use the new 4-parameter signature. Notable call sites include:

  • ContentServiceImpl.java (line 2047)
  • ItemServiceInternalImpl.java (line 238)
  • ConfigurationServiceInternalImpl.java (lines 495 & 581)
  • AssetDmContentProcessor.java (line 165)
  • FormDmContentProcessor.java (line 262)

No further changes are required.

src/main/java/org/craftercms/studio/api/v2/repository/RepositoryItem.java (2)

2-2: Updated copyright year to 2025.

The copyright notice has been updated to include 2025, ensuring proper legal protection for the latest code changes.

17-19: Modernized RepositoryItem to use Java record.

The class has been moved from v1 to v2 package and converted from a traditional class to a Java record. This is an excellent improvement that:

  1. Reduces boilerplate code by auto-generating getters, equals, hashCode, and toString
  2. Makes the class immutable by default
  3. Improves code readability and maintainability
src/main/java/org/craftercms/studio/impl/v2/service/content/ContentServiceImpl.java (1)

81-81: Exception type broadened in method signature.

The method now throws ServiceLayerException instead of the more specific SiteNotFoundException. This change aligns with the consolidation to a unified exception strategy and allows for more comprehensive error handling.

src/main/java/org/craftercms/studio/impl/v2/service/publish/internal/PublishServiceInternalImpl.java (3)

76-77: Dependency removed in favor of static utility methods.

Added static imports from SecurityUtils to replace direct SecurityService dependency, following the refactoring pattern evident throughout this PR. This change reduces coupling between components and simplifies dependency management.

233-233: Replaced SecurityService reference with static utility call.

Changed from using securityService.getAuthentication() to the static utility method getAuthentication(). This is consistent with the security refactoring to favor utility methods over direct dependency.

337-337: Replaced SecurityService reference with static utility call.

Changed from using securityService.getCurrentUser() to the static utility method getCurrentUser(). This matches the pattern of using security utilities consistently throughout the codebase.

src/main/java/org/craftercms/studio/impl/v2/upgrade/providers/XmlFileVersionProvider.java (2)

25-25: Updated import to use v2 repository API.

Import statement has been updated to reference the GitContentRepository from the v2 API package, consistent with the overall migration from v1 to v2.

105-118: Simplified control flow in the doGetVersion method.

The code has been refactored to:

  1. Throw an UpgradeNotSupportedException immediately if the /config/studio path doesn't exist
  2. Remove the check for the repository's first commit ID
  3. Improve the logical flow of the method

This simplification makes the code more maintainable and easier to understand while still preserving the original behavior.

src/test/java/org/craftercms/studio/impl/v2/repository/blob/StudioBlobStoreResolverImplTest.java (2)

28-28: API version upgrade from v1 to v2

The import has been changed from org.craftercms.studio.api.v1.repository.ContentRepository to org.craftercms.studio.api.v2.repository.ContentRepository, correctly aligning with the PR objective to remove v1 ContentRepository.

46-46: Import statement simplification

Changed from importing individual assertion methods to a wildcard static import, which reduces verbosity and improves maintainability.

src/main/java/org/craftercms/studio/impl/v2/service/repository/internal/RepositoryManagementServiceInternalImpl.java (3)

92-92: Migrated to static utility method for security operations

Importing the static method from SecurityUtils aligns with the codebase's move toward more centralized security handling.

788-788: Security service refactoring

Replaced dependency on securityService.getCurrentUser() with static utility method getCurrentUser(). This reduces direct dependencies and improves maintainability.

948-948: Security service refactoring

Replaced dependency on securityService.getCurrentUser() with static utility method getCurrentUser(). This reduces direct dependencies and improves maintainability.

src/main/java/org/craftercms/studio/impl/v2/service/item/internal/ItemServiceInternalImpl.java (1)

238-238: Simplified method signature

Removed the unused commitId parameter from the persistItemAfterWrite method. This is a good cleanup that simplifies the API without affecting functionality, as the parameter wasn't being used in the method implementation.

src/main/java/org/craftercms/studio/api/v2/utils/GitRepositoryHelper.java (3)

58-58: Added import for security utilities

Added import for SecurityUtils to support the refactoring from direct service dependency to static utility methods.

1304-1324: Added file version restoration functionality

New method to restore a file from a specific version in the repository. Implementation follows proper locking patterns to ensure thread safety during git operations.

1401-1401: Security service refactoring

Replaced direct call to securityService.getCurrentUser() with SecurityUtils.getCurrentUser(), consistent with the broader refactoring of security handling across the codebase.

src/main/java/org/craftercms/studio/impl/v2/service/content/internal/ContentServiceInternalImpl.java (4)

85-86: Good refactoring to use static utility methods

The change to import and use static utility methods from SecurityUtils is a positive refactoring that centralizes security-related functionality and reduces direct dependencies.

154-154: Clean replacement of SecurityService usage

Replacing direct calls to securityService.getCurrentUser() with the static utility method getCurrentUser() improves code maintainability by centralizing the security logic in a single place.

Also applies to: 192-192, 242-242, 262-262, 468-468

411-412: Good authentication handling refactoring

Using getAuthentication() from SecurityUtils for event publishing is a good practice that maintains consistent authentication handling throughout the codebase.

Also applies to: 478-478, 502-502

335-336: Method signature updated to handle exceptions

Adding throws ServiceLayerException to the method signature aligns with the interface changes and provides better error handling.

src/main/java/org/craftercms/studio/impl/v2/security/publish/PublishPackageAvailableActionResolverImpl.java (3)

36-36: Good use of static import

Adding the static import for SecurityUtils.getCurrentUser() method aligns with the overall refactoring approach.

48-55: Constructor simplified by removing SecurityService dependency

The constructor has been updated to remove the SecurityService dependency, which reduces coupling and aligns with the refactoring goals.

59-60: Refactored security operations

Security-related operations now use the static utility methods instead of the direct service dependency, which is a good architectural improvement.

Also applies to: 67-67

src/main/java/org/craftercms/studio/impl/v1/content/pipeline/FormDmContentProcessor.java (1)

262-262: Simplified method call by removing commitId parameter

The persistItemAfterWrite method call has been updated to remove the commitId parameter, which aligns with the larger refactoring effort to simplify content versioning and repository operations.

src/main/java/org/craftercms/studio/api/v1/service/content/ContentTypeService.java (1)

62-62: Improved exception handling in method signatures

Adding throws ServiceLayerException to these method signatures makes the contract more explicit and ensures consistent exception handling across the codebase. This is a good practice that improves the robustness of the API.

Also applies to: 64-64

src/main/java/org/craftercms/studio/impl/v2/upgrade/providers/XmlStreamVersionProvider.java (2)

20-20: Updated import to v2 GitContentRepository

The import statement has been updated to use the v2 version of GitContentRepository, which aligns with the PR objective to remove v1 ContentRepository.

39-39: Formatting adjustment

The parameter indentation has been adjusted for better readability, with no functional changes.

src/main/java/org/craftercms/studio/impl/v2/upgrade/StudioUpgradeManagerImpl.java (4)

31-35: Updated imports to v2 classes

Import statements have been updated to include necessary exceptions and use v2 repository classes, which aligns with the PR objective of removing v1 ContentRepository.

88-93: Improved constructor parameter formatting

The constructor parameter indentation has been adjusted for better readability, with no functional changes.

212-212: Simplified repository existence check

The method has been simplified to directly return the result of contentRepository.repositoryExists(site), removing unnecessary complexity.

231-263: Enhanced getExistingEnvironments method

The method has been updated to:

  1. Return a Collection instead of an array
  2. Add proper error handling with try-catch for ContentNotFoundException
  3. Update logging statements to use the new name() method on RepositoryItem

These changes improve error handling and adapt to the v2 repository API.

src/main/java/org/craftercms/studio/impl/v2/repository/blob/s3/StudioAwsS3BlobStore.java (2)

281-281: Simplified moveContent method signature

The method signature has been updated to remove the redundant newName parameter, simplifying the API.

284-353: Refactored moveContent implementation

The implementation has been adjusted to work with just the fromPath and toPath parameters, maintaining the same functionality while simplifying the code. The error handling and logging have been preserved.

src/main/java/org/craftercms/studio/api/v2/repository/GitContentRepository.java (5)

21-22: Added necessary exception imports

Added imports for ContentNotFoundException and UserNotFoundException to support the new method implementations.

Also applies to: 26-27

138-139: Improved method parameter formatting

Method parameter indentation has been adjusted for better readability, with no functional changes.

Also applies to: 248-248, 274-277

362-366: Enhanced method documentation

Added detailed comments to existing methods, improving code documentation and maintainability.

Also applies to: 370-373

375-382: Added getContentChildren method

New method to retrieve children of a repository folder, enhancing the content management capabilities of the interface. This method returns a Collection of RepositoryItem objects, consistent with the v2 API design.

384-395: Added revertContent method

New method to revert content to a previous version, with proper exception handling for UserNotFoundException and ServiceLayerException. This enhances the content versioning capabilities of the interface.

src/main/java/org/craftercms/studio/impl/v2/service/repository/RepositoryManagementServiceImpl.java (4)

2-2: Updated copyright year

Copyright year has been updated to reflect the current year.

40-40: Switched to SecurityUtils import

Import for SecurityUtils class has been added, which aligns with the strategy of centralizing security-related functionality.

56-62: Removed SecurityService dependency

The SecurityService dependency has been correctly removed from constructor properties and initialization, streamlining the dependency structure of the class.

78-78: Updated user retrieval to use SecurityUtils

The direct call to securityService.getCurrentUser() has been replaced with SecurityUtils.getCurrentUser(), which follows the new pattern of using static utility methods for security operations.

src/main/java/org/craftercms/studio/impl/v2/service/dashboard/DashboardServiceImpl.java (4)

2-2: Updated copyright year

Copyright year has been updated to reflect the current year.

59-59: Added static import for getCurrentUser

Added static import for SecurityUtils.getCurrentUser() to simplify code calls to the method.

79-92: Removed SecurityService dependency

SecurityService has been removed from the constructor properties and initialization, simplifying the class's dependency structure.

116-116: Updated user retrieval method

Changed from using injected securityService to the static utility method getCurrentUser(), which supports the centralization of security-related functionality.

Also applies to: 126-126

src/main/java/org/craftercms/studio/impl/v2/service/site/SitesServiceImpl.java (3)

56-58: Added ServiceLayerException to method signature

The getAvailableBlueprints() method now correctly declares that it throws ServiceLayerException, improving exception handling transparency.

61-63: Added ServiceLayerException to method signature

The getBlueprintDescriptor() method now correctly declares that it throws ServiceLayerException, improving exception handling transparency.

66-68: Added ServiceLayerException to method signature

The getBlueprintLocation() method now correctly declares that it throws ServiceLayerException, improving exception handling transparency.

src/test/java/org/craftercms/studio/impl/v1/service/content/ContentServiceImplTest.java (4)

22-23: Updated imports to v2 repository classes

Imports have been updated from v1 to v2 repository classes, aligning with the objective of removing v1 ContentRepository.

31-31: Changed import from array to collection

Updated import to use Collection instead of arrays, following modern Java best practices.

53-57: Updated repository items handling

Modified getRepoItems method to use Collection return type and .toList() for collection creation instead of arrays, which aligns with the updated repository API.

61-62: Updated test methods to use Collection instead of arrays

All test methods now correctly use Collection instead of RepositoryItem[] for repository items, maintaining consistency with the updated repository API.

Also applies to: 77-80, 98-101, 117-120, 136-139, 158-161

src/main/java/org/craftercms/studio/impl/v2/service/security/UserServiceImpl.java (8)

51-51: Import added for SecurityUtils - part of security refactoring

This import is part of the overall refactoring to remove direct SecurityService dependencies in favor of static utility calls, which aligns with the PR objectives.

102-103: Method call updated to use ConfigurationService instead of GroupServiceInternal

The method now retrieves site groups from configurationService instead of groupServiceInternal. This change is part of the service refactoring and moves the group retrieval responsibility to a more appropriate service.

305-305: Updated security checks in getUserSites method

The method now:

  1. Uses securityServiceV2 instead of securityService for system admin check
  2. Gets site groups from configurationService instead of groupServiceInternal

These changes align with the removal of v1 services in favor of v2 implementations.

Also applies to: 309-309

350-350: System admin check updated to use securityServiceV2

Replaced securityService.isSystemAdmin with securityServiceV2.isSystemAdmin as part of the v1 services removal.

375-376: Authentication retrieval replaced with static SecurityUtils call

Changed from using securityService.getAuthentication() to SecurityUtils.getAuthentication(), removing the dependency on the injected SecurityService.

390-391: Authentication retrieval replaced with static SecurityUtils call

Changed from using securityService.getAuthentication() to SecurityUtils.getAuthentication() in getCurrentUserSiteRoles method.

576-577: Current user retrieval replaced with static SecurityUtils call

Changed from securityService.getCurrentUser() to SecurityUtils.getCurrentUser() in getCurrentUserSitePermissions method.

592-594: Updated user roles retrieval in getCurrentUserGlobalPermissions

  1. Changed from securityService.getCurrentUser() to SecurityUtils.getCurrentUser()
  2. Now uses userServiceInternal.getUserGlobalRoles() instead of a method from securityService

This change maintains the functionality while removing the dependency on the v1 SecurityService.

src/main/java/org/craftercms/studio/impl/v2/service/configuration/internal/ConfigurationServiceInternalImpl.java (8)

58-62: Added Spring annotations for proper dependency management

Added @Autowired and @Lazy annotations for better Spring dependency management. This helps manage circular dependencies and makes the autowiring explicit.

88-90: Static imports for SecurityUtils methods

Added static imports for SecurityUtils.getAuthentication and SecurityUtils.getCurrentUser to maintain code readability while removing the SecurityService dependency.

400-402: Updated authentication retrieval in writeConfiguration

Changed from using securityService.getAuthentication() to the statically imported getAuthentication() method, consistent with the security refactoring strategy.

493-496: Updated user retrieval and item persistence in writeDefaultConfiguration

  1. Now gets current user via static SecurityUtils method
  2. Directly calls itemServiceInternal.persistItemAfterWrite with the retrieved user
  3. Removed intermediate SecurityService dependency

This simplifies the security operations while maintaining the same functionality.

580-582: Updated user retrieval in writeEnvironmentConfiguration

Similar to other changes, replaced securityService.getCurrentUser() with the static getCurrentUser() method to eliminate the SecurityService dependency.

644-646: Updated user retrieval in writeGlobalConfiguration

Replaced securityService.getCurrentUser() with static getCurrentUser() method in audit log generation.

718-725: Added new getSiteGroups method

This new method implementation gets site groups by extracting keys from the role mappings. It provides a standardized way to retrieve site groups that was previously handled by different services.

842-844: Added @lazy and @Autowired annotations to setter methods

These annotations help prevent circular dependencies by lazily initializing contentServiceInternal and itemServiceInternal. The explicit @Autowired annotation also makes the dependency injection clearer.

Also applies to: 872-874

src/test/java/org/craftercms/studio/impl/v2/service/repository/internal/RepositoryManagementServiceInternalImplTest.java (3)

28-29: Added imports for SecurityUtils and MockedStatic

Added necessary imports to support static mocking of SecurityUtils, which is required after replacing SecurityService with static utility methods.

Also applies to: 39-40

151-161: Replaced direct SecurityService mocking with static SecurityUtils mocking

Updated the test to use MockedStatic for SecurityUtils instead of mocking the securityService dependency. This change aligns with the refactoring to use static utility methods instead of the injected service.

The try-with-resources pattern ensures the MockedStatic is properly closed after use.

165-175: Updated exception test to use static SecurityUtils mocking

Similar to the previous test method, updated to use MockedStatic for SecurityUtils.getCurrentUser() to align with the main code changes.

src/test/java/org/craftercms/studio/impl/v2/service/policy/internal/PolicyServiceInternalImplTest.java (6)

24-27: Updated imports for repository refactoring

Changed imports to reflect the repository layer refactoring:

  1. Added ServiceLayerException instead of ConfigurationException
  2. Added GitContentRepository and RepositoryItem imports

These changes align with the removal of v1 ContentRepository.

104-104: Updated exception type in setUp method signature

Changed from ConfigurationException to ServiceLayerException to align with the updated error handling approach.

114-115: Simplified PolicyServiceInternalImpl constructor call

Removed contentRepositoryV2 parameter from the constructor, as part of consolidating repository interfaces.

146-147: Updated setUpRepository method signature

Changed to throw ServiceLayerException, simplifying the exception handling approach.

147-156: Modernized repository mock setup

Updated the repository setup to:

  1. Use List.of() instead of arrays for returned items
  2. Simplify RepositoryItem creation
  3. Use contentRepository instead of contentRepositoryV2

These changes make the tests more maintainable and align with the repository refactoring.

Also applies to: 158-172

178-186: Updated content size mock setup

Changed from using contentRepositoryV2 to contentRepository, consistent with the repository interface consolidation.

src/main/java/org/craftercms/studio/impl/v2/service/policy/internal/PolicyServiceInternalImpl.java (4)

20-24: Updated imports for error handling and repository interfaces

The imports have been updated to:

  1. Add ServiceLayerException for error handling
  2. Update repository interfaces to use GitContentRepository and RepositoryItem
  3. Use Collection instead of arrays

This aligns with the PR objective of removing v1 ContentRepository.

Also applies to: 35-35

71-83: Constructor simplified by removing contentRepositoryV2

The constructor no longer takes contentRepositoryV2 as a parameter, and the ConstructorProperties annotation has been updated to match. This simplifies the class's dependencies and state management.

184-191: Improved error handling for getContentChildren

Added robust error handling when retrieving content children with a try-catch block for ServiceLayerException. The method now logs a warning and returns early if an exception occurs, preventing further processing.

This change improves the robustness of the code and aligns with the standardized exception handling approach.

192-210: Updated repository API access pattern

Changed from direct field access to method calls (e.g., child.path() instead of child.path). This suggests that the RepositoryItem interface has been updated to use getter methods instead of public fields, which is a better encapsulation practice.

src/main/java/org/craftercms/studio/api/v2/service/site/SitesService.java (3)

65-65: Added ServiceLayerException to getAvailableBlueprints method

Modified the method signature to explicitly throw ServiceLayerException, standardizing error handling across the service layer.

73-73: Added ServiceLayerException to getBlueprintDescriptor method

Modified the method signature to explicitly throw ServiceLayerException, standardizing error handling across the service layer.

81-81: Added ServiceLayerException to getBlueprintLocation method

Modified the method signature to explicitly throw ServiceLayerException, standardizing error handling across the service layer.

src/main/java/org/craftercms/studio/api/v1/service/site/SiteService.java (2)

66-68: Simplified exception handling in createSiteFromBlueprint

Changed from throwing multiple specific exceptions to a single ServiceLayerException. This simplifies error handling by consolidating different types of exceptions, making the API easier to use.

208-247: Removed remote repository management methods

The methods addRemote, removeRemote, and listRemote have been removed from the interface. This aligns with the PR objective of removing v1 ContentRepository and its associated functionalities for managing remote repositories.

src/main/java/org/craftercms/studio/api/v2/repository/ContentRepository.java (3)

19-22: Added imports for exception handling

Added imports for ContentNotFoundException and UserNotFoundException to support the new method signatures and improved error handling.

65-87: Added content retrieval methods

Added two methods for retrieving content:

  1. A default method that provides a simpler API by setting shallow=false
  2. An overloaded method that allows specifying whether to retrieve from disk directly (shallow=true) or from the git repository

This improves the API's flexibility while maintaining backward compatibility.

89-93: Added content management methods

Added method signatures for:

  1. createFolder - Creates a new folder in the repository
  2. moveContent - Moves content from one path to another
  3. writeContent - Writes content to the repository

These methods standardize content management operations and explicitly declare which exceptions they throw, improving API clarity.

src/main/java/org/craftercms/studio/impl/v2/service/security/SecurityServiceImpl.java (6)

2-2: Copyright year update looks fine.
No issues with updating the year from 2024 to 2025.

44-44: Static import usage looks good.
This direct static import helps keep calls to getAuthentication() concise, though be mindful of potential naming conflicts.

60-60: Parameter changed to Collection<NormalizedRole>.
Using a more generic Collection instead of List increases flexibility for client code. All calls to this method must be updated accordingly.

71-71: Consistent change to support Collection<NormalizedRole>.
This helper method properly aligns with the signature in getUserPermission(). Looks good.

91-91: Consistent refactor to support Collection<NormalizedRole> for permissions lookup.
This aligns with the other changes. Ensure existing invocations pass a valid Collection.

143-143: Ensure getSiteGroups() via configurationService returns correct group data.
Verify that switching from groupServiceInternal.getSiteGroups() to configurationService.getSiteGroups() has no side effects in membership checks.

src/main/resources/crafter/studio/studio-services-context.xml (3)

347-347: Constructor-arg injection for configurationService.
Replacing or adding constructor-arg injection is fine. Confirm other properties remain properly set if they were previously injected differently.

769-769: Bean references gitContentRepositoryV2.
Confirm that this updated reference is the intended V2 repository. If so, this is consistent with the PR’s objective of removing the v1 interface.

791-791: localRepository now set to gitContentRepositoryV2.
This change aligns with the shift to the v2 Git repository. Looks consistent with the other updates.

src/main/java/org/craftercms/studio/impl/v1/service/site/SiteServiceImpl.java (5)

130-130: Field uses GitContentRepository v2.
Switching the repository layer to v2 is consistent with the goal of removing v1. No issues here.

266-266: Retrieving last commit ID via v2 repository method.
Implementation aligns with v2 usage. Ensure upstream logic handles any differences in method behavior.

465-465: Iterating over site paths using forAllSitePaths().
This is an effective way to process directories and files. Double-check performance on large sites.

493-493: Creating site from blueprint with the v2 repo.
The new call to createSiteFromBlueprint on v2 aligns with the PR objective. Implementation looks coherent.

600-600: Cloning a remote repository using the new v2 API.
Switching from legacy code to createSiteCloneRemote is appropriate. Verify all parameters (like remotePrivateKey) are set correctly.

src/main/java/org/craftercms/studio/impl/v2/service/security/internal/UserServiceInternalImpl.java (12)

22-22: Add utility import usage justification
The usage of org.apache.commons.collections4.CollectionUtils is sensible for advanced collection operations. Ensure that the overhead of including this library is justified by verifying that other functionalities from it are leveraged throughout the class.

33-33: Proper usage of configuration
Injecting ConfigurationService is in line with best practices for retrieving site configuration or group data. Confirm that usage of configuration keys is consistent with the existing naming conventions to prevent potential key collisions.

36-36: SecurityUtils import
Replacing direct references to securityService with a static utility approach can simplify code and unify security checks. Ensure the SecurityUtils methods handle all edge cases (e.g., threads without a security context) to prevent NullPointerExceptions.

50-55: Refined collections usage
These new static imports from java.util.Collections and MapUtils help to handle empty or non-empty checks elegantly. Make sure to keep consistent usage across the class to avoid confusion with manual checks (e.g., != null && !something.isEmpty()).

66-66: ConfigurationService field
Declaring configurationService as final is recommended for better immutability. This is already done here, which is a good practice.

74-79: Constructor refactoring
Switching to a constructor-based injection for ConfigurationService is consistent with best practices for testability and immutability. Ensure any prior method-based injection or direct field injection is fully removed.

81-81: Field assignment
Assigning this.configurationService = configurationService; is correct. Validate that no duplicative assignment or setter usage remains in older code.

164-164: Check list usage
Retrieving site groups via configurationService.getSiteGroups(siteId) centralizes site group retrieval. Make sure that the returned list is non-null (the code handles an empty list gracefully but might break if null is returned).

435-435: Use of SecurityUtils in property retrieval
Switching to var username = SecurityUtils.getCurrentUser(); is consistent with the rest of the refactor. Ensure no corner cases exist where an anonymous user might proceed into user-specific logic.

471-471: Username retrieval
Again, referencing SecurityUtils.getCurrentUser(). This straightforward approach fosters clarity. No additional changes required.

486-486: Authentication retrieval
Using SecurityUtils.getAuthentication() helps unify authentication logic. Good usage that aligns with a single entry point for retrieving authentication data.

490-490: AuthenticationException handling
Throwing an AuthenticationException with a clear message is correct. Verify that the calling code in the upper layers properly handles or logs this exception for better debugging and transparency.

src/main/java/org/craftercms/studio/impl/v2/service/site/internal/SitesServiceInternalImpl.java (8)

36-36: Import updates
These import additions (e.g., for RepositoryItem, @Autowired, @Lazy, and general collection utilities) align with the move toward newer repository and injection mechanisms. Ensure that all references to removed v1 classes are fully dropped to maintain a clean codebase.

Also applies to: 49-49, 52-52, 66-66

91-91: Constructor & field changes
Introducing ItemServiceInternal as a lazily autowired dependency is consistent with the new injection strategy. This improves testability and avoids circular dependencies. Confirm that the usage site is tested, especially for concurrency or lazy-loading race conditions around itemServiceInternal.

Also applies to: 95-125

143-146: Descriptor retrieval logic
getBlueprintDescriptor(...) uses the newly introduced getBlueprintsFolders(). Ensure no hidden exceptions pass silently if folder is unexpectedly null or if there's an underlying IO issue.

157-160: Blueprint location retrieval
getBlueprintLocation(...) again standardizes on Collection<RepositoryItem>. Good move away from older array-based code. Make sure to handle empty or missing blueprint folders gracefully.

179-179: Path construction
Combining folder.path() and folder.name() is straightforward, but carefully validate file separator usage on different OSes if cross-platform compatibility is a requirement.

198-198: Error message placeholders
logger.error("Failed to load descriptor from blueprint '{}'", folder.name(), e); is correct. Double-check that the placeholder count matches the argument count. (It does here: one placeholder plus the throwable).

416-416: Set actor ID from user
Extracting user from the calling method and setting it in auditLog is consistent, but confirm that the caller always has a valid user context (i.e., this might fail for service accounts if not carefully set up).

466-466: Actor ID consistency
Switching to getCurrentUser() for the actor ID ensures consistency across different methods. Confirm that the getCurrentUser() usage is well tested for API calls that run under system context.

src/main/java/org/craftercms/studio/impl/v1/service/content/ContentTypeServiceImpl.java (1)

37-38: Repository imports
The addition of GitContentRepository and RepositoryItem imports, along with java.util.*, reflect the transition from v1 repos to v2. Ensure that all replaced references to older repos are fully removed.

Also applies to: 45-45

src/main/java/org/craftercms/studio/impl/v2/repository/GitContentRepositoryImpl.java (11)

47-47: No issues with added imports.
All newly added imports appear standard and correctly reference required classes without duplications or unused references.

Also applies to: 56-56, 76-76

1535-1542: Ensure the returned InputStream is closed by the caller.
This method returns a raw FileInputStream without an internal close. Verify that the caller properly closes it to avoid resource leaks.

1544-1550: Conditional shallow retrieval looks correct.
The logic checks the shallow parameter and delegates properly to either shallowGetContent or the HEAD-based method.

1552-1562: Confirm TreeWalk closure downstream.
This helper method returns a TreeWalk; ensure any subsequent usage properly closes the TreeWalk to release resources.

1564-1599: Child listing implementation is consistent and thorough.
The code correctly handles tree vs. blob objects. Off-by-one or null checks appear valid. The try-with-resources pattern covers TreeWalk closure.

1601-1643: Folder creation flow appears sound.
Creating a zero-byte file is a valid Git workaround for empty directories. Proper commits and exception handling are included.

1654-1665: File or directory move helper is clear.
This private moveFiles method relies on FileUtils and has appropriate checks for file vs. directory. No immediate concerns identified.

1667-1706: Good usage of addFiles and commit for move operation.
The logic differentiates folders vs. pointer paths and updates the Git index accordingly. This is consistent with the rest of the repository’s design.

1708-1726: Revert procedure is straightforward.
By restoring a version and committing changes, the approach is aligned with standard Git usage. No major issues found.

1728-1758: Versioned read method is well-structured.
Handles retrieving a file from a specific commit. The exception handling is appropriate for missing content.

1760-1790: Write method properly commits user changes.
All steps (write, add, commit, record) follow standard patterns. The usage of SecurityUtils for user retrieval is consistent with the repository’s design.

src/test/java/org/craftercms/studio/impl/v2/repository/blob/BlobAwareContentRepositoryTest.java (8)

26-26: New imports appear correct.
These additional imports (RepositoryItem, ItemVersion, GitAPIException) match usage within the file.

Also applies to: 33-33, 34-34

146-150: Mocking localRepositoryV2 for content resolution.
Stubs for contentExists and getContent are logically consistent with the Blob logic. No issues found.

165-165: Mocking store movement return value.
Returning EMPTY for moveContent is a valid test scenario. No issues.

280-281: Verifying move operations.
Ensures that both the remote store and local repository methods are invoked. Looks correct for test coverage.

284-292: Child listing test without remote store.
Ensures that if an item is not recognized as a blob pointer, it’s returned directly from the local repository. The test logic is fine.

296-306: Child listing test with remote.
Confirms that pointer-based items are mapped to their original path. This validates the transformation logic in the repository.

488-499: Version history test for blob pointer.
Tests whether the repository properly retrieves version history by referencing the pointer path. No issues found.

506-506: Extension check for files without an extension.
Ensures the pointer extension “.blob” is appended. This test coverage looks appropriate.

src/main/java/org/craftercms/studio/impl/v2/repository/blob/BlobAwareContentRepository.java (17)

44-44: Additional import for RepositoryItem.
This lines up with usage in getContentChildren and other references below.

96-96: Unified local repository approach.
Replacing separate v1/v2 repositories with a single GitPublishCapableRepository simplifies maintenance.

171-181: Blob resolution vs. local fallback logic.
This block checks for folder vs. pointer existence and conditionally delegates to the blob store or local repository. Implementation is correct for a layered approach.

199-201: Content existence check with local fallback.
Throws ContentNotFoundException if no local repository record is found and no blob pointer applies. Matches desired behavior.

241-249: Returns -1 for blob sizes to avoid overhead.
This is a practical decision. The fallback to local repository size is consistent if no pointer is found.

257-265: Write content flow merges blob store references and pointer.
Serializing the Blob reference into the pointer path ensures consistency. Good use of an XML mapper.

282-297: Create folder in both blob store and local repo.
This synergy ensures that empty directories are tracked in local Git, while also existing in the blob store. No concerns found.

340-356: Move logic combines blob store and local repository.
The code properly updates the pointer path for non-folders. Error-handling approach is aligned with typical multi-store logic.

364-368: Child listing remaps pointer names to original.
This approach ensures users see the real names without the .blob extension. Implementation is consistent.

371-381: Version history fallback to pointer path.
This ensures that when an asset is blob-backed, the pointer’s history is used from Git. No immediate issues.

463-463: Consistent getContentByCommitId bridging.
Delegates entirely to local repository. Good for centralized Git logic.

471-472: Direct pass-through of site repository calls.
All these lines are straightforward delegations (lock, unlock, last commit, repository existence, etc.).

Also applies to: 475-475, 481-481, 489-489, 494-494, 499-499, 504-504

531-545: Mapping subtree paths to original asset names.
Converts pointer paths to their original form, ensuring consistent user-facing references.

559-563: Filtering out pointer suffix in forAllSitePaths.
Truncating the “.blob” extension before passing to the file processor ensures that higher-level logic sees original paths.

582-594: Initial publish merges blob uploads and local commit.
The repository ignores any failed blob items during final commit. This flow avoids partial commits for failed blobs.

631-646: Blob store “initialPublishBlobs” logic.
Groups items by store, then publishes in batches, closely tracking failures. The approach is coherent and clearly separated from final Git commit.

691-698: Selective vs. full publish logic.
Falls back on publishAll if no failures exist, else partial publish is used. Combining successful and failed items is handled properly.

src/main/java/org/craftercms/studio/impl/v1/service/content/ContentServiceImpl.java (21)

59-59: Import added for v2 repository types.

New import for RepositoryItem from the v2 API to support the transition from v1 ContentRepository to v2 repository implementation.

73-73: Security handling changed to use SecurityUtils.

The code has been refactored to use static utility calls via SecurityUtils instead of a dependency-injected security service. This is part of a security handling refactoring pattern applied across the codebase.

135-135: Repository type changed to GitContentRepository from v2 API.

The v1 ContentRepository has been replaced with the v2 GitContentRepository implementation. This is a core change that enables removal of the v1 repository.

174-174: Repository method calls updated to use v2 implementation.

All calls to repository methods have been updated to use the v2 implementation. The change is consistent across all method invocations.

Also applies to: 186-186, 196-198

241-241: Updated getContent method to utilize the v2 repository API.

The method now uses the v2 repository implementation's getContent method, which includes the 'shallow' parameter.

647-647: Repository writeContent call updated to v2 implementation.

The writeContent method now uses the v2 repository implementation, maintaining the same functionality but through the new API.

680-680: Security handling refactored to use SecurityUtils.

Security-related operations have been refactored to use static utility methods from SecurityUtils instead of a dependency-injected security service. This is consistent with the security refactoring pattern applied throughout the codebase.

Also applies to: 711-713, 726-726

702-702: Repository createFolder call updated to v2 implementation.

The repository method for folder creation has been updated to use the v2 repository API.

891-891: Event publishing updated to use SecurityUtils for authentication.

All event publishing code now retrieves the current user's authentication through SecurityUtils, maintaining consistent security handling throughout the application.

Also applies to: 1013-1013, 2066-2066, 2502-2502

1214-1215: Repository existence check updated to v2 implementation.

Content existence checks have been updated to use the v2 repository API methods.

1256-1256: Updated to use getContentChildren from v2 repository.

The method now uses the v2 repository's getContentChildren method to retrieve content children.

1524-1524: Updated isFolder check to use v2 repository implementation.

Folder type checking now uses the v2 repository API, maintaining consistent repository access patterns.

1655-1655: Repository children retrieval updated to use v2 collection-based API.

The method for retrieving children has been updated to work with Collection instead of arrays, reflecting the v2 repository's preference for collections over arrays.

Also applies to: 1669-1673

1684-1684: Updated repository item field access to match v2 API.

Repository item property access has been updated to use the v2 API's methods like name() and path() instead of direct field access.

Also applies to: 1693-1696

1707-1707: Updated isFolder check to use v2 repository implementation.

Folder type checking has been standardized to use the v2 repository API method throughout the codebase.

2023-2026: Simplified revertContentItem signature.

The method signature has been simplified, removing parameters related to the v1 implementation that are no longer needed, while maintaining the core functionality.

2031-2031: Repository revert content call updated to v2 implementation.

The revert content operation now uses the v2 repository API, maintaining the same functionality through the new interface.

2045-2047: Updated user retrieval to use SecurityUtils.

User retrieval for persistence operations now uses SecurityUtils, consistent with the security refactoring pattern.

2488-2488: Repository moveContent call updated to v2 implementation.

The move content operation now uses the v2 repository API, maintaining consistent repository access patterns throughout the codebase.

2564-2566: Repository field type updated to GitContentRepository.

The content repository field type has been updated to use the v2 implementation GitContentRepository, with corresponding setter method signature update.

2591-2597: Protected static inner class updated for consistency.

The PastedPathMap inner class is now explicitly protected static for better encapsulation and clarity.

Comment thread src/main/java/org/craftercms/studio/impl/v1/service/site/SiteServiceImpl.java Outdated
Comment thread src/main/java/org/craftercms/studio/impl/v1/service/site/SiteServiceImpl.java Outdated
Comment thread src/main/java/org/craftercms/studio/impl/v1/service/site/SiteServiceImpl.java Outdated
Comment thread src/main/java/org/craftercms/studio/impl/v1/service/site/SiteServiceImpl.java Outdated
Comment thread ...in/java/org/craftercms/studio/impl/v2/service/security/internal/UserServiceInternalImpl.java
import org.craftercms.studio.api.v1.service.site.SiteService;
import org.craftercms.studio.api.v2.dal.*;
import org.craftercms.studio.api.v2.dal.security.NormalizedGroup;
import org.craftercms.studio.api.v2.dal.security.NormalizedRole;
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 20, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Reuse security data structures
Importing NormalizedRole suggests a more standardized approach to handling user roles. Verify that references to older role representations (if any) are fully removed to avoid confusion or duplication.

Run a quick text search to confirm no references to the removed v1 roles remain:

🏁 Script executed:

#!/bin/bash
rg "v1.*Role"

Length of output: 150

Align Role Handling Across the Codebase
While the new implementation imports and uses NormalizedRole, our search reveals a legacy reference to org.craftercms.studio.api.v1.dal.Role in the file src/main/resources/crafter/studio/database/mybatis-config.xml. Please verify whether this v1 role alias is still required. If it’s no longer used anywhere in the system, remove or update the alias registration to ensure that our role-handling strategy remains consistent and free of duplication.

Comment thread src/main/java/org/craftercms/studio/impl/v1/service/content/ContentTypeServiceImpl.java
@jmendeza jmendeza marked this pull request as ready for review March 21, 2025 23:17
@jmendeza jmendeza requested a review from sumerjabri as a code owner March 21, 2025 23:17
@sumerjabri sumerjabri merged commit a176eb0 into craftercms:develop Apr 2, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Apr 23, 2025
Merged
@coderabbitai coderabbitai Bot mentioned this pull request May 1, 2025
Closed
@coderabbitai coderabbitai Bot mentioned this pull request Jun 3, 2025
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Aug 7, 2025
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Oct 1, 2025
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Dec 23, 2025
Merged
This was referenced Jan 14, 2026
Closed
Merged
Merged
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Jan 23, 2026
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Mar 18, 2026
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Mar 31, 2026
Merged
@coderabbitai coderabbitai Bot mentioned this pull request May 21, 2026
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@sumerjabri sumerjabri sumerjabri approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jmendeza @sumerjabri