👻 Items moved from internal/api to shared/api.

[jortel]

Export As() convenience.
Also, move and export common api errors to shared/api.

Summary by CodeRabbit

• Refactor

  • Consolidated document deserialization handling (internal) to streamline how documents are processed.

• Bug Fixes

  • Standardized error types and messages across the API so reported Bad Request, Forbidden, and Not Found errors are more consistent and clearer to callers.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Document.As was removed from the internal package and added to the shared package; numerous error types and error-construction sites were standardized to use shared/api error types with named fields and pointer receivers.

Changes

Cohort / File(s)	Summary
Document.As relocation internal/api/jsd/document.go, shared/api/jsd.go	Document.As(object any) removed from internal package and added to shared package; implementation delegates to d.Content.As(object).
New shared error types shared/api/error.go	Added exported error types: BadRequestError, Forbidden, NotFound with Error() and Is() implementations.
Local error aliases & handler updates internal/api/error.go	Replaced local error structs with aliases to shared/api types; adjusted BatchError receiver to pointer; updated errors.As usages and error matching logic.
BadRequestError field initialization fixes internal/api/analysis.go, internal/api/application.go, internal/api/base.go, internal/api/bucket.go, internal/api/context.go, internal/api/file.go, internal/api/ruleset.go, internal/api/insight_writer.go*	Replaced positional string composite literals with named-field initialization BadRequestError{Reason: ...} (and similar Forbidden{Reason: ...}) across multiple handlers and MIME/error paths.
BatchError pointer usage internal/api/batch.go, internal/api/error.go	Changed BatchError usage to pointers where constructed and updated Error()/Is() receiver to pointer.

*internal/api/insight_writer.go is representative of other MIME-not-supported/error branches updated similarly.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hopped from inner shade to shared daylight,
Errors now tidy, handlers hold tight.
A method moved, fields named true—
I nibble carrots and say, "Review!" 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: moving and exporting the Document.As() method to the shared/api package, which aligns with the primary objective and the bulk of the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1ae9be and a0eb1c1.

📒 Files selected for processing (10)

• internal/api/analysis.go
• internal/api/application.go
• internal/api/base.go
• internal/api/batch.go
• internal/api/bucket.go
• internal/api/context.go
• internal/api/error.go
• internal/api/file.go
• internal/api/ruleset.go
• shared/api/error.go

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-08-28T21:41:21.035Z

Learnt from: jortel
Repo: konveyor/tackle2-hub PR: 893
File: migration/migrate_test.go:93-93
Timestamp: 2025-08-28T21:41:21.035Z
Learning: VersionError in migration/pkg.go implements the Is() method, making errors.Is(err, &VersionError{}) a valid way to check for this error type in tests.

Applied to files:

• internal/api/error.go

📚 Learning: 2025-08-28T21:41:21.035Z

Learnt from: jortel
Repo: konveyor/tackle2-hub PR: 893
File: migration/migrate_test.go:93-93
Timestamp: 2025-08-28T21:41:21.035Z
Learning: VersionError in migration/pkg.go implements the Is() method properly using errors.As internally, making errors.Is(err, &VersionError{}) a valid and correct way to check for this error type in tests.

Applied to files:

• internal/api/error.go

🧬 Code graph analysis (10)

shared/api/error.go (1)

internal/api/error.go (3)

• BadRequestError (22-22)
• Forbidden (25-25)
• NotFound (28-28)

internal/api/analysis.go (2)

internal/api/error.go (1)

• BadRequestError (22-22)

shared/api/error.go (1)

• BadRequestError (9-11)

internal/api/file.go (2)

internal/api/error.go (1)

• BadRequestError (22-22)

shared/api/error.go (1)

• BadRequestError (9-11)

internal/api/error.go (1)

shared/api/error.go (3)

• BadRequestError (9-11)
• Forbidden (24-26)
• NotFound (39-42)

internal/api/application.go (2)

internal/api/error.go (1)

• BadRequestError (22-22)

shared/api/error.go (1)

• BadRequestError (9-11)

internal/api/batch.go (1)

internal/api/error.go (1)

• BatchError (31-34)

internal/api/base.go (2)

internal/api/error.go (1)

• BadRequestError (22-22)

shared/api/error.go (1)

• BadRequestError (9-11)

internal/api/context.go (2)

internal/api/error.go (1)

• BadRequestError (22-22)

shared/api/error.go (1)

• BadRequestError (9-11)

internal/api/bucket.go (2)

internal/api/error.go (1)

• BadRequestError (22-22)

shared/api/error.go (1)

• BadRequestError (9-11)

internal/api/ruleset.go (2)

internal/api/error.go (1)

• Forbidden (25-25)

shared/api/error.go (1)

• Forbidden (24-26)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: test-unit
• GitHub Check: build
• GitHub Check: vet

🔇 Additional comments (21)

internal/api/ruleset.go (2)

226-229: LGTM!

The change to use named field initialization (Reason: "...") aligns with the Forbidden struct definition in shared/api/error.go and maintains consistency with the codebase-wide standardization of error construction.

---

281-284: LGTM!

Consistent with the update handler change, correctly uses the named Reason field for the Forbidden error.

internal/api/batch.go (1)

60-60: LGTM!

The change to pointer initialization (&BatchError{...}) aligns with the error handling pattern in internal/api/error.go where errors.As(err, &bErr) is used, which requires BatchError to have pointer receivers for proper type assertion.

internal/api/context.go (1)

351-353: LGTM!

The change to named field initialization (Reason: "...") for BadRequestError aligns with the struct definition in shared/api/error.go and is consistent with the codebase-wide standardization.

internal/api/application.go (1)

746-750: LGTM!

The change to named field initialization for BadRequestError is consistent with the codebase-wide standardization and the struct definition in shared/api/error.go.

internal/api/file.go (4)

91-94: LGTM!

The change to named field initialization (Reason: err.Error()) correctly preserves the original error message while aligning with the standardized BadRequestError struct definition.

---

104-109: LGTM!

Consistent use of named field initialization for the BadRequestError when input.Open() fails.

---

207-211: LGTM!

Consistent pattern for FormFile error handling in createMultipart, using named field initialization.

---

228-232: LGTM!

Consistent pattern for input.Open() error handling, completing the standardization in this file.

internal/api/bucket.go (1)

276-276: LGTM! Consistent use of named field initialization.

The BadRequestError{Reason: err.Error()} pattern aligns with the struct definition in shared/api/error.go and is applied consistently across all error construction sites.

Also applies to: 281-281, 328-328, 334-334

internal/api/base.go (2)

69-74: LGTM! Named field initialization for error construction.

The multi-line format with Reason: field is clear and correctly uses the struct definition from shared/api/error.go.

---

170-173: Consistent error wrapping pattern applied.

The BadRequestError{Reason: ...} construction is consistent with the rest of the codebase. Note that the post-switch error wrapping (lines 172-174, 247-249) will re-wrap any BadRequestError from the unsupported MIME case, but this preserves the existing behavior.

Also applies to: 245-248

internal/api/analysis.go (2)

354-354: LGTM! Consistent error construction in AppCreate.

All BadRequestError instances in the manifest parsing flow correctly use the Reason field, maintaining consistency with the shared error type definition.

Also applies to: 362-362, 369-369, 387-387, 395-395, 406-406, 425-425, 433-433, 445-445

---

2147-2147: LGTM! MIME not supported errors updated consistently.

The InsightWriter.Create and AnalysisWriter.Create methods now use the named field pattern for unsupported MIME type errors.

Also applies to: 2237-2237

shared/api/error.go (3)

8-21: LGTM! Well-structured error type with type-based matching.

The Is() implementation using errors.As internally enables type-based error matching via errors.Is(err, &BadRequestError{}), which is the established pattern in this codebase. Based on learnings, this approach is intentional and consistent with other error types like VersionError.

---

23-36: LGTM! Forbidden error type follows the same pattern.

Consistent implementation with BadRequestError.

---

38-52: LGTM! NotFound error with descriptive formatting.

The Error() method provides a clear message format including the resource name and reason. The Is() implementation is consistent with the other error types.

internal/api/error.go (4)

16-16: LGTM! Clean type aliasing to shared error types.

The import and type aliases (BadRequestError, Forbidden, NotFound) properly expose the shared API error types within the internal package, maintaining backward compatibility while centralizing the definitions.

Also applies to: 21-28

---

41-49: LGTM! BatchError updated to pointer receivers.

Consistent with the other error types and enables proper error matching via errors.Is.

---

61-65: LGTM! TrackerError.Is() updated to correct pattern.

Using var target *TrackerError with errors.As(err, &target) is the proper way to implement type-based error matching.

---

157-158: LGTM! Correct errors.As usage.

The errors.As(err, &bErr) call is correct—bErr is *BatchError, so &bErr provides the **BatchError that errors.As needs to populate the target variable.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}