Skip to content

PR merge test #2779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 55 commits into
base: master
Choose a base branch
from
Open

PR merge test #2779

wants to merge 55 commits into from

Conversation

weiminyu
Copy link
Collaborator

@weiminyu weiminyu commented Jun 30, 2025

Test: do not submit.


This change is Reviewable

qrtp and others added 30 commits May 2, 2025 13:57
…comments

Update configuration mgmt from review comments
qrtp and others added 25 commits June 26, 2025 16:33
We plan on using this for EPP password resets and registry lock password
resets for now.
This avoids potential replication lag issues when requesting info on
domains that were just created.
* Remove registrar id from invoice grouping key

* Fix formatting issues

* Update BillingEventTests
This is just so that we can add an additional layer of security on
verification
If contacts are optional, they should be optional in the command too.
The Cloud DNS rest api is now case-sensitive about enum names (must be
lower case, counterintuitively).
We can probably improve on this in the future if we want, but there's a
lot of boilerplate that we don't need to repeat over and over
This picks up a few changes including aligning the placement of quotes
in text blocks with the Google style guide.
Add a new DnsWritersModule for use by the component classes.

To override the set of writers installed, we can easily overwrite this
file with a private version.
This implements two type of changes:
1. changing the link type for things like the terms of service
2. adding the request URL to each and every link with the "value" field.
   This is a bit tricky to implement because the links are generated in
various places, but we can implement it by adding it to the results
after generation.

See b/418782147 for more information
It was making the undo nomulus command look like this:

)nomulus ...
A future PR will add the actions that save and use this object. That
future PR will also require loading RegistrarPoc objects given the
registrar ID, hence the change in that class.
Pubapi actions should always use cache, regardless of the config
settings on caching.

In EppResource.java, the original `loadCached(Iterable<VKey>)`
method is renamed to `loadByCacheIfEnabled`. The original
`loadCached(Vkey)` method is renamed to `loadByCache` and always
uses cache.

In EppResourceUtils.java, the original `loadByForeignKeyCached`
method is renamed to `loadByForeignKeyByCacheIfEnabled`. A new
`loadByForeignKeyByCache` method, which always uses cache.

In ForeighKeyUtils.java, the original `loadCached` method is
renamed to `loadByCacheIfEnabled`, and a new `loadCached` method
is added which always uses cache.

Also added a `getContactsFromReplica` method in Registrar,
for use by RDAP actions.
Scripts needed by cron jobs wrongly removed by PR 2661.

TESTED: in crash.
In RDAP, domain queries are the most common by a factor of like 40,000
so we should optimize these as much as possible. We already have an EPP
resource / foreign key cache which does improve performance somewhat but
looking at some sample logs, it only cuts the RDAP request times by like
40% (looking at requests for the same domain a few seconds apart).

History entries don't change often, so we should cache them to make
subsequent queries faster as well. In addition, we're only caching two
fields per repo ID (modification time, registrar ID) so we can cache
more entries than we can for the EPP resource cache (which stores large
objects).
…lot-commit

Remove commit from co-pilot due to conflict with google repo requirements
@weiminyu weiminyu added the do not merge Do not merge this PR. label Jun 30, 2025
Copy link

google-cla bot commented Jun 30, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Comment on lines +186 to +188
Boolean isAnyDomainUpdate =
update.getSection(Section.UPDATE).stream()
.anyMatch(record -> record.getName().equals(r.getName()) && !isDeleteRecord(record));

Check warning

Code scanning / CodeQL

Boxed variable is never null Warning

The variable 'isAnyDomainUpdate' is only assigned values of primitive type and is never 'null', but it is declared with the boxed type 'Boolean'.
.orElse(null);

// if both keys are found, complete the ZSK rollover
if (activeZsk != null && inactiveZsk != null) {

Check warning

Code scanning / CodeQL

Useless null check Warning

This check is useless.
activeZsk
cannot be null at this check, since it is guarded by
... == ...
.
* @return true if the record is a delete record, false otherwise
*/
private Boolean isDeleteRecord(Record r) {
return r.getTTL() == 0 && r.rdataToString().equals("");

Check notice

Code scanning / CodeQL

Inefficient empty string test Note

Inefficient comparison to empty string, check for zero length instead.
this.changetype = changetype;
}

public List<RecordObject> getRecords() {

Check notice

Code scanning / CodeQL

Exposing internal representation Note

getRecords exposes the internal representation stored in field records. The value may be modified
after this call to getRecords
.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Do not merge this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants