[srp-server] introduce LeaseTracker base class for Host and Service

[abtink]

This commit extracts the common lease tracking variables (mLease, mKeyLease, mTtl, and mUpdateTime) and their associated methods from the Srp::Server::Host and Srp::Server::Service classes into a new shared base class, LeaseTracker.

By having both Host and Service inherit from LeaseTracker, we eliminate duplicated logic for calculating expiration times, handling lease info, and processing TTL updates. This refactoring simplifies the SRP server codebase and ensures consistent lease management behavior across both entities.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the SRP server's lease management by introducing a dedicated LeaseTracker base class. This change centralizes the logic for handling lease, key-lease, and time-to-live (TTL) properties, which were previously duplicated across the Host and Service classes. The refactoring aims to streamline the codebase, reduce redundancy, and ensure a unified approach to lease expiration and update processing for both host and service entities within the SRP server.

Highlights

• Refactoring Lease Management: Extracted common lease tracking variables (mLease, mKeyLease, mTtl, mUpdateTime) and their associated methods from Srp::Server::Host and Srp::Server::Service into a new shared base class, LeaseTracker.
• Inheritance for Consistency: Both Host and Service classes now inherit from LeaseTracker, eliminating duplicated logic for calculating expiration times, handling lease information, and processing TTL updates, ensuring consistent behavior.
• API Modernization: Updated direct member access to lease-related variables with corresponding getter and setter methods, improving encapsulation and maintainability.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• src/core/net/srp_advertising_proxy.cpp
  • Updated aHost.mKeyLease direct member access to aHost.GetKeyLease() in multiple locations within AdvertiseRemovalOf and Advertise methods.
  • Changed aService.mKeyLease direct member access to aService.GetKeyLease() in AdvertiseRemovalOf.
• src/core/net/srp_server.cpp
  • Implemented the Server::LeaseTracker class methods, including Init, GetExpireTime, GetKeyExpireTime, GetLeaseInfo, and ProcessTtl.
  • Modified Server::RemoveHost to use aHost->SetLease(0) and aHost->SetKeyLease(0) instead of direct member assignment.
  • Updated Server::CommitSrpUpdate to use service.SetLease(), service.SetKeyLease(), and service.SetTtl() for service lease updates.
  • Changed Server::ProcessServiceDescriptionInstructions to use service.SetTtl().
  • Modified Server::ProcessAdditionalSection to use service.SetLease() and service.SetKeyLease() for service lease options.
  • Updated Server::HandleUpdate to use service.SetKeyLease() when handling existing service key leases.
  • Removed duplicated lease and TTL related methods (GetExpireTime, GetKeyExpireTime, GetLeaseInfo, ProcessTtl) from Server::Service and Server::Host.
  • Updated Server::Service::Init and Server::Host constructor to call LeaseTracker::Init and removed redundant member initializations.
• src/core/net/srp_server.hpp
  • Introduced the Server::LeaseTracker class definition, including its member variables (mLease, mKeyLease, mTtl, mUpdateTime) and public/protected methods (GetLease, GetKeyLease, GetTtl, GetExpireTime, GetKeyExpireTime, GetLeaseInfo, Init, SetTtl, SetLease, SetKeyLease, ProcessTtl).
  • Modified Server::Service to inherit publicly from Server::LeaseTracker.
  • Removed duplicated lease-related member variables (mTtl, mLease, mKeyLease, mUpdateTime) and methods (GetTtl, GetLease, GetKeyLease, GetExpireTime, GetKeyExpireTime, GetLeaseInfo) from Server::Service.
  • Modified Server::Host to inherit publicly from Server::LeaseTracker.
  • Updated Server::Host::IsDeleted() to use GetLease() from the base class.
  • Removed duplicated lease-related member variables (mTtl, mLease, mKeyLease, mUpdateTime) and methods (GetTtl, GetLease, GetKeyLease, GetLeaseInfo, GetExpireTime, GetKeyExpireTime, ProcessTtl, SetTtl, SetLease, SetKeyLease) from Server::Host.

Activity

• No specific pull request activity (comments, reviews, progress updates) was provided in the context.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request refactors the SRP server code by extracting common lease tracking logic from Srp::Server::Host and Srp::Server::Service into a new base class LeaseTracker. This change successfully reduces code duplication and improves maintainability. The implementation is clean, follows the project's coding style, and correctly preserves or improves the existing logic. I have reviewed the changes and found no issues.

_{Note: Security Review did not run due to the size of the PR.}

[github-actions]

Merging #12628 into main

name	branch	text	data	bss	total
ot-cli-ftd	3ac4d4a	482136	860	66540	549536
	3aca578	482136	860	66540	549536
	+/-	+0	+0	+0	+0
ot-ncp-ftd	3ac4d4a	448036	764	61776	510576
	3aca578	448036	764	61776	510576
	+/-	+0	+0	+0	+0
ot-cli-mtd	3ac4d4a	374608	764	50972	426344
	3aca578	374608	764	50972	426344
	+/-	+0	+0	+0	+0
ot-ncp-mtd	3ac4d4a	355756	764	46240	402760
	3aca578	355756	764	46240	402760
	+/-	+0	+0	+0	+0
ot-cli-ftd-br	3ac4d4a	598440	868	136596	735904
	3aca578	598312	868	136596	735776
	+/-	-128	+0	+0	-128
ot-rcp	3ac4d4a	63328	568	20804	84700
	3aca578	63328	568	20804	84700
	+/-	+0	+0	+0	+0

Library files

name	branch	text	data	bss	total
libopenthread-ftd.a	3ac4d4a	249792	95	40319	290206
	3aca578	249792	95	40319	290206
	+/-	+0	+0	+0	+0
libopenthread-cli-ftd.a	3ac4d4a	61727	0	8091	69818
	3aca578	61727	0	8091	69818
	+/-	+0	+0	+0	+0
libopenthread-ncp-ftd.a	3ac4d4a	33581	0	5948	39529
	3aca578	33581	0	5948	39529
	+/-	+0	+0	+0	+0
libopenthread-mtd.a	3ac4d4a	168227	0	24783	193010
	3aca578	168227	0	24783	193010
	+/-	+0	+0	+0	+0
libopenthread-cli-mtd.a	3ac4d4a	41436	0	8059	49495
	3aca578	41436	0	8059	49495
	+/-	+0	+0	+0	+0
libopenthread-ncp-mtd.a	3ac4d4a	25805	0	5948	31753
	3aca578	25805	0	5948	31753
	+/-	+0	+0	+0	+0
libopenthread-ftd-br.a	3ac4d4a	366713	100	110335	477148
	3aca578	366611	100	110335	477046
	+/-	-102	+0	+0	-102
libopenthread-cli-ftd-br.a	3ac4d4a	80934	0	8131	89065
	3aca578	80934	0	8131	89065
	+/-	+0	+0	+0	+0
libopenthread-rcp.a	3ac4d4a	9944	0	5060	15004
	3aca578	9944	0	5060	15004
	+/-	+0	+0	+0	+0
libopenthread-radio.a	3ac4d4a	19715	0	238	19953
	3aca578	19715	0	238	19953
	+/-	+0	+0	+0	+0

[codecov]

Codecov Report

❌ Patch coverage is 85.10638% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.61%. Comparing base (6ea7f24) to head (c938686).
⚠️ Report is 18 commits behind head on main.

Files with missing lines	Patch %	Lines
src/core/net/srp_advertising_proxy.cpp	0.00%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12628      +/-   ##
==========================================
+ Coverage   73.71%   75.61%   +1.90%     
==========================================
  Files         697      695       -2     
  Lines       94709    94656      -53     
==========================================
+ Hits        69812    71576    +1764     
+ Misses      24897    23080    -1817     

Files with missing lines	Coverage Δ
src/core/net/srp_server.cpp	91.35% <100.00%> (+7.05%)	⬆️
src/core/net/srp_server.hpp	93.10% <100.00%> (+4.57%)	⬆️
src/core/net/srp_advertising_proxy.cpp	5.76% <0.00%> (ø)

... and 159 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[abtink]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a LeaseTracker base class for Srp::Server::Host and Srp::Server::Service to consolidate common lease management logic. The refactoring successfully extracts duplicated code for handling lease times, TTLs, and expiration calculations, leading to a cleaner and more maintainable SRP server implementation. The changes are well-executed, adhering to the project's coding style and improving encapsulation by using getters and setters instead of direct member access. A subtle but important correctness improvement is also included, ensuring that deleted services correctly have their lease set to zero during updates. Overall, this is a high-quality contribution that improves the codebase.

_{Note: Security Review did not run due to the size of the PR.}