Исправлена ошибка парсинга прав

[theshadowco]

Описание

Связанные задачи

Closes

Чеклист

Общие

• Ветка PR обновлена из develop
• Отладочные, закомментированные и прочие, не имеющие смысла участки кода удалены
• Изменения покрыты тестами
• Обязательные действия перед коммитом выполнены (запускал команду gradlew precommit)

Дополнительно

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Improved error handling in role data conversion with enhanced logging and graceful failure recovery.
  • Updated role-based access control rules for corrected permissions on specific tasks and documents.
  • Enhanced null value validation with defensive programming practices.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

RoleDataConverter adds Lombok logging support and replaces strict null checks with defensive conditional processing that logs warnings instead of throwing exceptions. Test expectations are updated to reflect revised access rules for certain roles and operations.

Changes

Cohort / File(s)	Summary
Error handling and logging improvements src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/RoleDataConverter.java	Added @Slf4j annotation and replaced Objects.requireNonNull() with conditional null checks that log warnings; defensive reads in readObjectRight and readRight now process values only when fieldClass is non-null; explicit handling of restrictionTemplate node via negated condition instead of early skip
Test expectations update src/test/java/com/github/_1c_syntax/bsl/mdclasses/helpers/RightsTest.java	Updated access rule assertions: Task.ЗадачаИсполнителя.Command.Перенаправить VIEW access changed to true; analytics system client roles increased from 78 to 103; Document.Анкета VIEW access size increased to 2; Task.ЗадачаИсполнителя.Command.Перенаправить VIEW access size increased to 4

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• Добавлена обработка ошибки чтения содержимого прав роли #564: Complements this PR by wrapping RoleConverter.read to catch failures and default to RoleData.EMPTY, addressing role-reading failures at an adjacent layer.
• Фикс ошибки чтения роли #571: Related changes to RoleDataConverter's restrictionTemplate node handling and logging behavior.

Poem

🐰 A hop through the logs, now soft and kind,
Where null checks are caught, no crashes to find,
With warnings instead of exceptions that bite,
The data flows gently, all tested quite right! 🌟

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Исправлена ошибка парсинга прав' (Fixed rights parsing error) accurately describes the main change - fixing parsing errors in the RoleDataConverter for handling role rights data, as evidenced by the error handling improvements and updated test expectations.

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

✨ Finishing touches

• 📝 Generate docstrings

---

🧹 Recent nitpick comments

src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/RoleDataConverter.java (1)

84-87: Consider adding warning logs for consistency.

In unmarshal, a warning is logged when fieldClass is null (line 64), but readObjectRight (lines 84-87) and readRight (lines 102-105) silently skip unknown fields. This inconsistency could make debugging harder when rights data contains unexpected nested structures.

♻️ Suggested improvement for consistency

      } else {
        var fieldClass = (Class<?>) TransformationUtils.fieldType(builder, name);
        if (fieldClass != null) {
          var value = ExtendXStream.readValue(context, fieldClass);
          TransformationUtils.setValue(builder, name, value);
+       } else {
+         LOGGER.warn("Field type not found for ObjectRight.{}", name);
        }
      }

And similarly in readRight:

      var fieldClass = (Class<?>) TransformationUtils.fieldType(builder, name);
      if (fieldClass != null) {
        var value = ExtendXStream.readValue(context, fieldClass);
        TransformationUtils.setValue(builder, name, value);
+     } else {
+       LOGGER.warn("Field type not found for Right.{}", name);
      }

Also applies to: 102-105

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b962a37 and cafdbc7.

⛔ Files ignored due to path filters (1)

• src/test/resources/fixtures/ssl_3_1/Roles.БазовыеПраваБСП.json is excluded by !src/test/resources/**

📒 Files selected for processing (2)

• src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/RoleDataConverter.java
• src/test/java/com/github/_1c_syntax/bsl/mdclasses/helpers/RightsTest.java

⏰ 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). (18)

• GitHub Check: build (21, ubuntu-latest)
• GitHub Check: build (21, windows-latest)
• GitHub Check: build (25, ubuntu-latest)
• GitHub Check: build (17, ubuntu-latest)
• GitHub Check: QA
• GitHub Check: build (21, macOS-latest)
• GitHub Check: build (17, windows-latest)
• GitHub Check: build (25, windows-latest)
• GitHub Check: build (25, macOS-latest)
• GitHub Check: build (21, macOS-latest)
• GitHub Check: build (17, ubuntu-latest)
• GitHub Check: build (25, macOS-latest)
• GitHub Check: build (25, windows-latest)
• GitHub Check: build (17, windows-latest)
• GitHub Check: build (17, macOS-latest)
• GitHub Check: build (21, ubuntu-latest)
• GitHub Check: build (21, windows-latest)
• GitHub Check: build (25, ubuntu-latest)

🔇 Additional comments (4)

src/test/java/com/github/_1c_syntax/bsl/mdclasses/helpers/RightsTest.java (2)

60-60: Updated assertion reflects the parsing fix.

The change from isFalse() to isTrue() aligns with the defensive parsing improvements in RoleDataConverter that now correctly processes fields that were previously skipped due to null field types.

---

108-108: Test expectations updated to match corrected parsing behavior.

The increased counts (78→103, 1→2, 0→4) are consistent with the parsing fix. Previously, some rights were being skipped when field types couldn't be resolved; now they are processed correctly.

Also applies to: 115-115, 122-122

src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/RoleDataConverter.java (2)

57-66: Good defensive handling for unknown fields.

The change from Objects.requireNonNull to a null check with warning log is a sensible approach that prevents parsing failures for unknown fields while still providing visibility through logging.

---

30-35: No action required. The @Slf4j annotation correctly generates a LOGGER field in this project because lombok.config explicitly sets lombok.log.fieldName=LOGGER.

_{✏️ 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.}

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
80.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Test Results

  603 files  ±0    603 suites  ±0   9m 34s ⏱️ -6s
  284 tests ±0    284 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 646 runs  ±0  2 646 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit cafdbc7. ± Comparison against base commit b962a37.

♻️ This comment has been updated with latest results.