* Resolve merge conflict
* Include reason and requestedByRegistrar in URS test file
* Modify test cases for new parameters in renew flow
* Add reason and registrar_request to renew domain command
* Update comments for new params in renew flow
* Make changes based on feedback
* Implement several fixes affecting test flakiness
- Continued to do transaction manager cleanups on reply failure (lack of this
may be causing cascading failures.
- Fix UpdateDomainCommandTest's output check (the test was checking for error
output in standard error, but the command writes its output to the logs.
Apparently, these may or may not be reflected in standard error depending on
current global state)
- Remove unnecessary locking and incorrect comment in CommandTestCase. The
JUnit tests are not run in parallel in the same JVM and, in general, there
are much bigger obstacles to this than standard output stream locking.
* Fix bad log message check
We're seeing some of these in CreateSyntheticHistoryEntriesAction and I
can't tell why from the logs (it doesn't appear to print the repo ID or
domain/host name)
* Add TmOverrideExtension for more safe TM overrides in tests
This is safer to use than calling setTmForTest() directly because this extension
also handles the corresponding call to removeTmOverrideForTest() automatically,
the forgetting of which has been a source of test flakiness/instability in the
past.
There are now broadly two ways to get tests to run in JPA: either use
DualDatabaseTest, an AppEngineExtension, and the corresponding JPA-specific
@Test annotations, OR use this override alongside a
JpaTransactionManagerExtension.
* Add a reference to RDAP conformance checker
Make a note of the RDAP conformance checker for the next time that we deal
with the RDAP code - would be nice to have this in the test suite.
* Reformat comment
* Add autorenews to URS (#1343)
* Add autorenews to URS
* Add autorenews to existing xml files for test cases
* Harmonize domain.get() in existing code
* Fix typo in test case name
* Modify existing test helper method to allow testing with different domain bases
* Fix BigQuery data set name handling in activity reporting
This is not a constant (as it depends on runtime state), so it can't be named
using UPPER_SNAKE_CASE. Additionally, it's not good practice to use field
initialization when there's logic depending on runtime state involved. So this
PR changes the class to use constructor injection and moves the logic into the
constructor.
* Add fix for ICANN reporting provide
* Extract out ICANN reporting data set
* Inject TransactionManager
* Make TransactionInfo static (per Mike)
* Use ofyTm() in BackupTestStore
* Revert extraneous formatting
* Use auditedOfy in CommitLogMutationTest
This PR adds the final step in RDE pipeline (enqueueing the next action
to Cloud Tasks) and makes some necessary changes, namely by making all
CloudTasksUtils related classes serializable, so that they can be used
on Beam.
<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/google/nomulus/1330)
<!-- Reviewable:end -->
Both `DatastoreEntityExtension.PlaceholderEnvironment` and `AppEngineEnvironment` does the same thing, so there is no point having both of them exist. To use `AppEngineEnvionrment` as an autoclosable requires the user to be mindful of where a fake App Engine environment is required. It is better to set this either in the `DatastoreEntityExtension` for tests, or in the worker initializer in Beam. It also makes it easier to remove the fake environment when we are completely datastore free.
Also made a change to how `IdService` allocate Ids in Beam.
<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/google/nomulus/1348)
<!-- Reviewable:end -->
* Add a cron job to periodically empty out fields on deleted entities that are at least 18 months old
* Process ContactHistory entities via batching
* Improve test cases by not making assertions in a loop
* Add/use more DatabaseHelper convenience methods
This also fixes up some existing uses of "put" in test code that should be
inserts or updates (depending on which is intended). Doing an insert/update
makes stronger guarantees about an entity either not existing or existing,
depending on what you're doing.
* Convert more Object -> ImmutableObject
* Merge branch 'master' into tx-manager-sigs
* Revert breaking PremiumListDao change
* Refactor more insertInDb()
* Fight more testing errors
* Merge branch 'master' into tx-manager-sigs
* Merge branch 'master' into tx-manager-sigs
* Merge branch 'master' into tx-manager-sigs
* Merge branch 'master' into tx-manager-sigs
* Add removeTmOverrideForTest() calls
* Merge branch 'master' into tx-manager-sigs
* Add handling for UpdateAutoTimestamp when not in a transaction
It's not clear why this is sometimes causing test flakes, but getting better
logging involved should help clear it up.
This also changes AppEngineExtension to insert without reloading the initial
test data, rather than putting it (potentially involving a merge) and reloading
it in a separate transaction. This should hopefully reduce the chance of weird
conflicts.
There are actions for which we want to provide an override for the database
to use, like when launching Spec11 and Invoicing pipelines. It make sense to
consolidate around the same parameter provided from the same module for
consistency in all cases, instead of defining an override for each action.
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/google/nomulus/1331)
<!-- Reviewable:end -->
* Add locking and a response in ReplicateToDatastoreAction
The response is necessary to get nicer logs in GAE and nicer cron job
behavior.
In addition:
- fix issues where locks would be backed up and replayed to Datastore
(they shouldn't be replayed)
- do ignore-read-only writes when replaying the transactions
The test for this also required a bit of a fix in the Cursor scope
initialization. If you persist a Key<?> in some object in Datastore, it
persists just the standard data you'd expect, basically the parent, the
kind, and the object's ID/name. Then, when you load it back in from
Datastore it uses the app ID of whatever environment you're loading in
(the Key contains this info even though it isn't included in the
toString() of Key)
If you persist the websafe string format of a Key (which is what we do
for Cursors), it includes the app ID so when you load it, it contains
the old app ID not the new one (if the app ID has changed).
In the pipelines, we use the standard default environment which has a
different app ID from the test environment that's set up by the
DatastoreEntityExtension.
As a result, we should check in Cursor to see if the key is *any*
cross-tld-key, rather than the exact one that exists within this app
environment.
* Clean up tx manager insert() signature and add convenience helper method
This is the first of a series of PRs to clean up the type signatures on the
TransactionManager methods (which are way too generic), along with creating some
helper methods for use in tests only that don't require creating transactions
all over the place, thus reducing visual noise at callsites. This first method
is DatabaseHelper.insertInDb(), but there will be plenty of others. Note that
this is only for the Cloud SQL transaction manager -- I'm not bothering to
migrate any Datastore-only code, as that will be going away soon enough.
* Skip synthetic history entries for resources that don't need them
The reason for creating synthetic history entries is so that we can
guarantee that each EppResource's most recent *History object contains
that resource at that point in time. If the most recent *History object
in SQL contains that resource already, there is no need to create a
synthetic *History object for that resource.
The build is generating the following lint warnings:
core/src/main/java/google/registry/flows/certs/CertificateChecker.java:246:
warning: [ReferenceEquality] Compariso
n using reference equality instead of value equality
&& (lastExpiringNotificationSentDate == START_OF_TIME
^
(see https://errorprone.info/bugpattern/ReferenceEquality)
core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java:350:
warning: [UnnecessaryParenthes
es] These grouping parentheses are unnecessary; it is unlikely the code will
be misinterpreted without them
.that(jpaTm().transact((() -> jpaTm().loadByEntity(contactResource))))
* Rename client ID to registrar ID in most places
This is a code-only change, that shouldn't require any sort of data
migration. Correspondingly, there are some existing uses of clientId that are
not migrated (e.g. Datastore fields, task queue payloads, URL parameters for
actions that might be hit from task queues, etc.). And it of course doesn't
modify any fields in EPP XML. Note that the Cloud SQL schema fields are
already named using the registar_id pattern.
This also doesn't yet touch on the -c parameters in nomulus tools; that will be
coming later (since that is an external manual touch-point, it will require a
lot more in the way of changes to various meta scripts and documentation).
* Change more client IDs
* Merge branch 'master' into clientid-to-registrarid
* Rename Spec11Pipeline's Subdomain -> DomainNameInfo
"Subdomain" never made any sense as a class name because these are all
second-level domain names, along with a little bit of metadata such as some
registrar info. "DomainNameInfo" is a better fit.
* Add the DNS refresh request time field to the Domain tables
This isn't used yet, but it will eventually be the replacement for the dns-pull
task queue once we get further in the migration.
* Merge branch 'master' into domain-dns-dirty
* Remove VKeyTranslatorFactory.createVKey(String)
This method serves the same function as VKey.fromWebsafeKey(), and isn't used
anywhere. Move the test for it into VKeyTest and use it to instead test
fromWebsafeKey() (which didn't previously have a test).
I'm not sure why this test is failing. It's failing saying that the
listObjects call is failing to include
"soy_2000-01-01_thin_S1_R1.xml.ghostryde" in the results, however the
verifyFiles method that we call right beforehand verifies that file and
its contents
This includes a change to how the JPA transaction manager handles
existence and load checks for entities with compound IDs. Previously, we
relied on the fields all being named the same in the ID entity and the
parent entity. This didn't work for History objects (e.g. DomainHistory)
so existence checks were broken. Now, we use the methods the same way
that Hibernate does (if possible).
Note as well that there's a bit of semi-duplicated logic in
DeleteProberDataAction (between the mapper and the SQL logic). The
mapper code will be deleted once we've shifted to SQL, and for now it's
better to keep it in place for logging purposes.
This involves:
- Altering both transaction managers to check for a read-only mode at
the start of standard write actions (e.g. delete, put).
- Altering both raw layers (entity manager, ofy) to throw exceptions on
write actions as well
- Implementing bypass routes for reading / setting / removing the schedule itself
so that we don't get "stuck"
* Clean up ReplicateToDatastoreAction and tests
1. applyTransaction should throw an error if it fails; this allows us to
have more information in the caller (and it shouldn't usually happen)
2. Set a response code + payload now, since this is an action that is
called by cron
3. Add a method to the test log subject that allows us to check if a
severe log with a particular Throwable cause was logged (since the cause
isn't contained in the log message itself directly)
* Save indexes when replaying EppResources SQL->DS
We implement this similarly to how we implement the
beforeSqlSaveOnReplay callback in the other direction -- a
beforeDatastoreSaveOnReplay method that is called when replaying a
Mutation to Datastore. This means that the asynchronous replay will
create the relevant ForeignKeyIndex and EppResourceIndex objects for
EppResources saved when SQL is primary.
* Implement a util class to manage push queues using Cloud Tasks API
Push queues were part of App Engine when they debuted. As a result the
Task Queue API were part of the App Engine SDK and can only be used in
App Engine classic runtime. The new Cloud Tasks API can be used in any
runtime but it only supports push queues. In this PR we implement a util
class (CloudTasksUtils) like TaskQueueUtils to handle enqueuing tasks to
push queues using Cloud Tasks. One action (TldFanoutAction) was
converted to use the new API as a demo. Mass migration of other call sites of
the old API will follow in a separate PR.
TESTED=deployed to alpha and verified that tasks are corrected enqueued
and executed.
Add double-replay to the Host*Flow tests to show how this works. The
only change to the double replay itself is that now we store the
Datastore entity in the TransactionEntity object -- this is because we
use Objectify to serialize the objects into bytes and we need it to know
about the entity in question.
