Skip to content

Conversation

@Flix-29
Copy link
Member

@Flix-29 Flix-29 commented Nov 6, 2025

No description provided.

@Flix-29 Flix-29 linked an issue Nov 6, 2025 that may be closed by this pull request
@Flix-29 Flix-29 requested a review from Copilot November 6, 2025 19:09
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces fine-grained endpoint generation control for REST controllers by adding include and exclude options to the @SproutResource annotation. This allows developers to selectively generate specific CRUD endpoints instead of the previous all-or-nothing readOnly flag.

Key Changes

  • Added new Endpoint enum to represent the five controller methods (GET_ALL, GET_BY_ID, CREATE, UPDATE, DELETE)
  • Extended @SproutResource annotation with include() and exclude() arrays for selective endpoint generation
  • Refactored controller generation logic to conditionally generate each endpoint based on the new configuration options

Reviewed Changes

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

File Description
Endpoint.java New enum defining the five endpoint types with human-readable names
SproutResource.java Added include/exclude arrays and updated documentation for readOnly flag interaction
SproutControllerProcessor.java Refactored method generation to use new methodGenerationAllowed helper for conditional endpoint creation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 359 to 360
(endpoint == Endpoint.GET_ALL && !excluded.contains(Endpoint.GET_ALL)
|| endpoint == Endpoint.GET_BY_ID && !excluded.contains(Endpoint.GET_BY_ID))
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

The logical condition has a precedence issue. The || operator binds more loosely than &&, causing the condition to evaluate incorrectly. When endpoint == Endpoint.GET_BY_ID && !excluded.contains(Endpoint.GET_BY_ID) is true, the entire condition becomes true regardless of the readOnly() check. Add parentheses around the OR expression: (endpoint == Endpoint.GET_ALL && !excluded.contains(Endpoint.GET_ALL) || endpoint == Endpoint.GET_BY_ID && !excluded.contains(Endpoint.GET_BY_ID))

Suggested change
(endpoint == Endpoint.GET_ALL && !excluded.contains(Endpoint.GET_ALL)
|| endpoint == Endpoint.GET_BY_ID && !excluded.contains(Endpoint.GET_BY_ID))
((endpoint == Endpoint.GET_ALL && !excluded.contains(Endpoint.GET_ALL))
|| (endpoint == Endpoint.GET_BY_ID && !excluded.contains(Endpoint.GET_BY_ID)))

Copilot uses AI. Check for mistakes.
Comment on lines 359 to 360
(endpoint == Endpoint.GET_ALL && !excluded.contains(Endpoint.GET_ALL)
|| endpoint == Endpoint.GET_BY_ID && !excluded.contains(Endpoint.GET_BY_ID))
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

The exclusion check is redundant since line 352-353 already returns false if the endpoint is excluded. The !excluded.contains(...) checks on lines 359-360 will always be true at this point in the method. Simplify to: endpoint == Endpoint.GET_ALL || endpoint == Endpoint.GET_BY_ID

Suggested change
(endpoint == Endpoint.GET_ALL && !excluded.contains(Endpoint.GET_ALL)
|| endpoint == Endpoint.GET_BY_ID && !excluded.contains(Endpoint.GET_BY_ID))
(endpoint == Endpoint.GET_ALL || endpoint == Endpoint.GET_BY_ID)

Copilot uses AI. Check for mistakes.
Endpoint[] include() default {};

/**
* Endpoints to exclude for this resource. Bounds stronger than {@link SproutResource#include()} option.
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

The word 'Bounds' should be 'Binds' to correctly convey that exclusion takes precedence over inclusion.

Suggested change
* Endpoints to exclude for this resource. Bounds stronger than {@link SproutResource#include()} option.
* Endpoints to exclude for this resource. Binds stronger than {@link SproutResource#include()} option.

Copilot uses AI. Check for mistakes.
@Flix-29 Flix-29 requested a review from Copilot November 6, 2025 19:20
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

sprout-processor/src/main/java/de/flix29/sprout/processor/SproutControllerProcessor.java:34

  • Corrected grammar from 'Successful created' to 'Successfully created'.
            generateSwaggerApiResponseString(201, "Successful created");

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Flix-29 Flix-29 merged commit 0e9aa0c into main Nov 8, 2025
8 checks passed
@Flix-29 Flix-29 deleted the include-exclude-endpoints branch November 8, 2025 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

In/Exclude endpoints from generation

2 participants