Skip to content

Conversation

@berezovskyi
Copy link
Member

@berezovskyi berezovskyi commented Feb 7, 2026

This PR addresses the compiler warning MA0070: Obsolete attributes should include explanations.
It adds explanatory messages to [Obsolete] attributes in several files:

  • OSLC4Net.Client/Oslc/OslcClient.cs: Added messages suggesting replacements like HttpClient, DeleteResourceAsync, UpdateResourceRawAsync.
  • OSLC4Net.Client/Oslc/Resources/ChangeRequest.cs: Added message to AddRdfType.
  • OSLC4Net.Client/Oslc/Resources/OslcQuery.cs: Added message to GetQueryUrl.
  • OSLC4Net.ChangeManagement/ChangeRequest.cs: Added messages to GetRdfTypes and SetRdfTypes.
  • OSLC4Net.Core/Model/OslcMediaType.cs: Added messages to obsolete media type constants.
  • OSLC4Net.Domains.RequirementsManagement/Requirement.cs and RequirementCollection.cs: Added messages to obsolete Type property.

No new warnings were introduced. Code style enforced with dotnet format. Existing tests passed.


PR created automatically by Jules for task 1406957271129390419 started by @berezovskyi

Summary by CodeRabbit

  • New Features

    • Added a public Types property to access resource RDF types.
  • Chores

    • Clarified deprecation messages across many public APIs to point to the new Types accessor and to prefer HttpClient-based/async alternatives; updated guidance on legacy media-type constants and certificate-validation helpers.
  • Tests

    • Added tests verifying the Types property, its interaction with existing type methods, serialization round-trip, and expected metadata.

Added descriptive messages to [Obsolete] attributes across the project to comply with MA0070.
Updated OslcClient, ChangeRequest, OslcQuery, OslcMediaType, Requirement, and RequirementCollection.
Ran dotnet format.
Verified with dotnet build and tests.

Co-authored-by: berezovskyi <64734+berezovskyi@users.noreply.github.com>
@google-labs-jules
Copy link
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 7, 2026

Walkthrough

Adds a public Types property to AbstractResource and replaces bare [Obsolete] attributes with explicit deprecation messages across multiple public APIs (pointing to .Types, QueryUri, async/HttpClient usage); no signature or runtime behavior changes.

Changes

Cohort / File(s) Summary
Core model — AbstractResource
OSLC4Net_SDK/OSLC4Net.Core/Model/AbstractResource.cs
Added public ICollection<Uri> Types { get => types; set => types = value; }, coexisting with existing GetTypes/SetTypes/AddType.
Core model — Media types
OSLC4Net_SDK/OSLC4Net.Core/Model/OslcMediaType.cs
Replaced plain [Obsolete] on media-type constants/fields with explicit messages recommending RDF/XML, JSON‑LD, or Turtle alternatives.
Client HTTP API
OSLC4Net_SDK/OSLC4Net.Client/Oslc/OslcClient.cs
Annotated legacy constructors, synchronous HTTP methods, and AcceptAllServerCertificates with Obsolete messages directing callers to HttpClient-based constructors/async methods and HttpClientHandler callbacks.
Client resources — ChangeRequest & Query
OSLC4Net_SDK/OSLC4Net.Client/Oslc/Resources/ChangeRequest.cs, OSLC4Net_SDK/OSLC4Net.Client/Oslc/Resources/OslcQuery.cs
Standardized Obsolete messages on AddRdfType, GetRdfTypes, SetRdfTypes, and GetQueryUrl to point to .Types or QueryUri.
ChangeManagement domain — ChangeRequest
OSLC4Net_SDK/OSLC4Net.ChangeManagement/ChangeRequest.cs
Updated Obsolete messages on GetRdfTypes() and SetRdfTypes() to recommend .Types.
RequirementsManagement domain
OSLC4Net_SDK/OSLC4Net.Domains.RequirementsManagement/Requirement.cs, OSLC4Net_SDK/OSLC4Net.Domains.RequirementsManagement/RequirementCollection.cs
Replaced bare [Obsolete] on Type properties with [Obsolete("Use Types instead")].
Tests
OSLC4Net_SDK/Tests/OSLC4Net.Core.Tests/AbstractResourceTests.cs, OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/AbstractResourceSerializationTests.cs
Added tests covering Types property behavior, AddType/GetTypes/SetTypes interactions, collection replacement, serialization round-trip, and attribute expectations. Review duplicate-handling assertions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PR #322 — Overlapping changes to resource modelling and migration to property-based Types APIs.
  • PR #464 — Introduces HttpClient-based OslcClient constructor and helpers that the new Obsolete messages reference.
  • PR #389 — Adjusts mapping for collection types to resource shapes; directly related to exposing Types as a collection.

Suggested labels

ci:full, xtra:smoke-test

Poem

🐇 I hop through code with tiny, tidy bites,
"Use .Types instead" I whisper in the night.
Old sync paths nod, new properties cheer,
Tests snugly guard each change that's near.
A carrot for progress — small and bright!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.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 clearly and concisely describes the main change: adding explanatory messages to [Obsolete] attributes across the project to fix MA0070 compiler warnings.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-ma0070-obsolete-warnings-1406957271129390419

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/AbstractResourceSerializationTests.cs (2)

17-37: Consider adding a Verify snapshot of the intermediate RDF/XML.

The round-trip assertion is solid and covers the key properties. Per project guidelines, you could additionally snapshot the serialized rdfXml with Verify to catch unexpected changes in the RDF/XML output (e.g., extra triples, namespace drift) that pure round-trip checks won't surface.

Something like:

await Verify(rdfXml);  // snapshot the serialized RDF/XML

This is optional — the existing round-trip assertions already provide meaningful coverage.

Based on learnings: "Prefer Verify and round-tripping checks when dealing with RDF data in tests."


10-10: Nit: extract the duplicated type URI to a constant.

"http://example.com/ns#ConcreteResource" appears in both the OslcResourceShape attribute (Line 10) and the assertion (Line 35). A const would keep them in sync.

Proposed change
 public class AbstractResourceSerializationTests
 {
+    private const string ConcreteResourceTypeUri = "http://example.com/ns#ConcreteResource";
+
-    [OslcResourceShape(describes = ["http://example.com/ns#ConcreteResource"])]
+    [OslcResourceShape(describes = [ConcreteResourceTypeUri])]
     private class ConcreteResource : AbstractResource

And in the assertion:

-        await Assert.That(deserializedResource.Types).Contains(new Uri("http://example.com/ns#ConcreteResource"));
+        await Assert.That(deserializedResource.Types).Contains(new Uri(ConcreteResourceTypeUri));

Also applies to: 35-35


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

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

@codecov
Copy link

codecov bot commented Feb 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 50.87%. Comparing base (a1d78f8) to head (7013873).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #594   +/-   ##
=======================================
  Coverage   50.86%   50.87%           
=======================================
  Files         174      174           
  Lines       10202    10204    +2     
  Branches     1010     1010           
=======================================
+ Hits         5189     5191    +2     
  Misses       4756     4756           
  Partials      257      257           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…esource

- Added `Types` property to `AbstractResource` to support the recommendation.
- Updated `[Obsolete]` messages in `ChangeRequest.cs` (both versions) to recommend using `.Types` instead of `GetRdfTypes`/`SetRdfTypes`/`AddRdfType`.
- Deleted temporary build output file.

Co-authored-by: berezovskyi <64734+berezovskyi@users.noreply.github.com>
Copy link
Contributor

@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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
OSLC4Net_SDK/OSLC4Net.ChangeManagement/ChangeRequest.cs (1)

126-129: ⚠️ Potential issue | 🟡 Minor

AddRdfType is not marked [Obsolete] here but is in the client-side ChangeRequest.

Per the relevant snippets, OSLC4Net.Client/Oslc/Resources/ChangeRequest.cs marks AddRdfType with [Obsolete("Use .Types instead")]. For consistency, this method should also be deprecated.

Suggested fix
+    [Obsolete("Use .Types instead")]
     public void AddRdfType(Uri rdfType)
     {
         AddType(rdfType);
     }
🧹 Nitpick comments (1)
OSLC4Net_SDK/OSLC4Net.Core/Model/AbstractResource.cs (1)

44-48: Consider adding a null guard on the Types setter.

Assigning null via Types = null would cause a NullReferenceException on subsequent calls to AddType() (Line 109) or GetTypes(). While the existing SetTypes() method has the same gap, this new property is the recommended replacement going forward—worth making it defensive from the start.

🛡️ Suggested fix
     public ICollection<Uri> Types
     {
         get => types;
-        set => types = value;
+        set => types = value ?? throw new ArgumentNullException(nameof(value));
     }

Also, per Microsoft Framework Design Guidelines, collection properties generally should not have public setters (callers should use Clear/Add on the existing collection instead of replacing it). However, since the existing SetTypes method follows this same pattern throughout the codebase, this is noted for awareness rather than as a blocking concern. As per coding guidelines, "Review the C# code against the Microsoft Framework design guidelines".

google-labs-jules bot and others added 2 commits February 8, 2026 11:48
Removed build_output.txt which was accidentally included in the previous commit.

Co-authored-by: berezovskyi <64734+berezovskyi@users.noreply.github.com>
Added `AbstractResourceTests.cs` to verify the functionality of the new `Types` property on `AbstractResource`.
Verified that `Types` behaves consistently with `GetTypes`, `SetTypes`, and `AddType`.

Co-authored-by: berezovskyi <64734+berezovskyi@users.noreply.github.com>
Copy link
Contributor

@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: 1

🤖 Fix all issues with AI agents
In `@OSLC4Net_SDK/Tests/OSLC4Net.Core.Tests/AbstractResourceTests.cs`:
- Around line 13-38: Split the single TestTypesProperty into multiple focused
tests each covering one behavior: create TestAddType_UpdatesTypes to exercise
ConcreteResource.AddType and assert Types contains the new Uri and count
increments; TestGetTypes_ReturnsSameCollectionAsTypes to assert GetTypes()
returns the same collection instance/value as the Types property;
TestTypeSetter_ReplacesCollection to set Types to a new List<Uri> and assert old
entries are removed and new entries present; TestSetTypes_UpdatesTypes to call
SetTypes and assert Types equals the provided list. Additionally add edge-case
tests: TestInitialTypes_IsEmpty to verify initial Types state,
TestAddType_DuplicateDoesNotCreateExtra (or documents expected duplicate
behavior) to assert adding a duplicate type behaves as intended, and
TestSetTypes_EmptyListClearsTypes to set Types to an empty list and verify Types
is empty; reference ConcreteResource, AddType, GetTypes, Types (getter/setter)
and SetTypes when implementing the new test methods to improve branch coverage.
🧹 Nitpick comments (1)
OSLC4Net_SDK/Tests/OSLC4Net.Core.Tests/AbstractResourceTests.cs (1)

7-11: Unused constructor ConcreteResource(Uri about).

The Uri-accepting constructor is never exercised in this test class. Consider removing it to keep the test helper minimal, or add a test that uses it (e.g., verifying About is set correctly).

google-labs-jules bot and others added 3 commits February 8, 2026 18:20
Refactored TestTypesProperty into multiple focused tests.
Added checks for round-tripping capability (verifying attributes).
Fixed usage of OslcPropertyDefinition attribute in tests.

Co-authored-by: berezovskyi <64734+berezovskyi@users.noreply.github.com>
- Refactored `AbstractResourceTests.cs` to test `Types` property behaviors individually.
- Added `AbstractResourceSerializationTests.cs` to verify round-trip serialization of `Types` using `OSLC4Net.Core.DotNetRdfProvider`.
- Ensured proper RDF mapping attributes are verified.

Co-authored-by: berezovskyi <64734+berezovskyi@users.noreply.github.com>
Added verification that the type URI from [OslcResourceShape] is present in the deserialized object in AbstractResourceSerializationTests.

Co-authored-by: berezovskyi <64734+berezovskyi@users.noreply.github.com>
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.

1 participant