Memcache is already off but now it's not in the code anymore.
This includes removing domain creation failfast, since that is actually
slower now than just running the flow - all you gain is a non-transactional
read over a transactional read, but the cost is that you always pay that
read, which is going to drive up latency.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=158183506
This replaces the memcache caching, which we think is overall a bad idea.
We load all registrars at once instead of caching each as needed, so that
the loadAllCached() methods can be cached as well, and therefore will
always produce results consistent with loadByClientIdCached()'s view of the
registrar's values. All of our prod registrars together total 300k of data
right now, so this is hardly worth optimizing further, and in any case this
will likely reduce latency even further since most requests will be
served out of memory.
While I was in the Registrar file I standardized the error messages for incorrect
password and clientId length to be the same format, and cleaned up a few
random things I noticed in the code.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=156151828
We've determined that getting correctness semantics right, even
in the few cases that it is possible to do so (see linked bug for
audit) is not worth the bother in terms of highly complicated code
and potential bugs. This CL turns off memcache at the Ofy level
but doesn't rip out the annotations etc. so that we can quickly
turn it back on if this turns out to have been a mistake.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=155227761
TESTED=For all tests, I added @Cache to DomainBase because otherwise the tests will
fail. We aren't ready to do this in prod yet, which is why the tests have a TODO
in them. The new tests fail if you change line 134 in Ofy to not use memcache
and either use the unchanged original flow code, or use the new
inlined code and change loadWithMemcache() to load(). They pass with the new
inlined code that calls loadWithMemcache(), as long as the @Cache is added to
DomainResource.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154457655
Only Ofy itself and its two helpers (AugmentedSaver and
AugmentedDeleter) need to use the real ofy(). All other
callers should be using Ofy. Fixing this even though it
doesn't change anything because I found it baffling to
follow the code while trying to make a small change.
Update: added a presubmit to enforce this.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154456603
TESTED=The test fails if you change line 134 in Ofy to not use memcache
and use the unchanged original Registry.get() code. This is the
expected behavior.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154226534
TESTED=For all tests, I added @Cache to DomainBase because otherwise the tests will
fail. We aren't ready to do this in prod yet, which is why the tests are still
marked @Ignore. The new tests fail if you change line 134 in Ofy to not use memcache
and either use the unchanged original DomainCreateFlow code, or use the new
inlined code and change loadWithMemcache() to load(). They pass with the new
inlined code that calls loadWithMemcache(), as long as the @Cache is added to
DomainResource.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154224748
This avoids loading all contacts and hosts before the failfast runs
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154179321
No tests break because of this, but some tests that were previously
intentionally clearing memcache (for example, the test in
DomainInfoFlow that counts datastore calls) are now doing something
that is not a no-op anymore.
Also fix an incorrect comment about memcache in DatastoreHelper.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154087182
In fact, completely eviscerate cloneProjectedAtTime (to be removed in
a followup CL) in favor of doing the projection of transfers and the
loading of values from the superordinate domain at call sites. This
is one of the issues that blocked the memcache audit work, since the
load inside of cloneProjectedAtTime could not be controlled by the
caller.
Note: fixed a minor bug where a subordinate host created after its superordinate domain was last transferred should have lastTransferTime==null but was previously reporting the domain's lastTransferTime.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=149769125
This is a cleanup in preparation for the next change that does a lot
of work with subordinate hosts, to make it easier to reason about in
complex code.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=146689904
This bug is about a bad use of Optional.
We were checking == null instead of .isPresent(), so the check
always passed, and we always set a lastSubordinateTime when
updating hosts, even if the host was external and should have
had a null value in that field.
There is almost certainly bad data in prod in the sense
that any external host that was ever updated will have a value
for this field instead of null. However, this is not
consequential as the field is entirely meaningless for
external hosts, and will be properly reset if the host is
ever moved to be internal.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=146363178
In general we insist that you assign the return of checkRegistrarConsoleLogin
to something, since it's annotated with @CheckReturnValue, but in this
specific test which should always throw the value is unused, so suppress the
"unused" warning.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=146265522
Even though the outer class is imported, a Javadoc reference to
a nested class needs to be fully qualified (or imported on its
own, which our style guide prohibits).
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=146160210
This generalizes the "LINKED can't be anywhere" idea into more
targeted restrictions.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=146158204
We already have an @OnLoad in EppResource that removes LINKED
from any status values, so there's no reason to filter in info.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=146146501
Warning: This is a breaking change to custom logic for domain info.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=146033749
* Remove LINKED when loading an EppResource
* Enforce that you can't add it to a resource
* Ignore LINKED on xjc import of contacts and hosts
After running ResaveAllEppResourcesAction we will no
longer have persisted LINKED statuses in datastore.
In the process of writing this I discovered that RDAP
treats LINKED like any other status value and returns
the persisted value rather than the derived one. Since
this is an existing bug and is orthogonal to the changes
in this CL, I am addressing it in a separate CL.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=145585227
LINKED is a virtual status that needs to be computed on the fly
when creating an RDAP response.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=145583415
This was used in DomainInfoFlow to return a DomainResource with no
nameservers without making INACTIVE show up. It was nominally used
in DomainApplicationInfoFlow for the same reason, but that's just
an artifact of the old flow hierarchy since applications never have
INACTIVE set anyways. In either case, now that we have the DomainInfo
return object instead of returning DomainResource directly from the
flow, it's better handled within the flow.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=145582317
Now that we return an Info object rather than the resource itself,
there's no reason for the cloning pattern.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=145426484
This is preparatory refactoring for the next CL, which gets rid
of cloneWithLinkedStatus
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=145424322
LINKED is supposed to be a virtual status that gets added/removed
when needed. It's mistakenly been persisted to datastore on many
resources, but the persisted value is meaningless and may not
represent reality at all. There is no reason to check for LINKED
status before kicking off a delete, since the smoke test and
[] will catch all actual linked objects, and the LINKED
status can be causing false positives for objects that are no
longer LINKED.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=145422457
This is probably best from a code-cleanliness perspective anyways,
but the rationale is that tightly coupling the resources to the
info responses was a straightjacket that required all status
values and fields to be directly available on the resource. With
this change, I already was able to get rid of the preMarshal()
hackery, and I will be able to get rid of cloneWithLinkedStatus()
and most of the contents of cloneProjectedAtTime() for non-domains.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=144252924
In the previous CL I added filter support for the test server, but
even though I could verify that filters were being run when debugging,
in practice the side effects of the filters (notably, ObjectifyFilter
clearing the session cache) were somehow not present in tests (and
therefore causing new as-yet unsubmitted tests that rely on proper
session caching to break).
Investigating further, the way TestServer works is that it creates
a wrapper Servlet for each route, and in that wrapper just pushes
a future onto a queue and waits on it. The actual target servlet
is run within the queue, not within the wrapper servlet's context.
I had added filters to the *wrapper* servlet, which meant that even
though they were invoked before adding the task to the queue, they
were not invoked in the process of actually running the task.
In this CL I pushed the filters into the task itself, just like the
target servlet.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=144123637
The lack of ObjectifyFilter means that in any tests using
RegistryTestServer the Objectify session cache persists
between "requests" in the same test method. This is wrong
but had not caused any failures because we didn't assert
anything that mattered. However, a CL I'm working on
asserts that there is a creationTime on a created resource,
which is set automatically on Datastore save, and therefore
is still null in the session cache's version of the resource
if you don't clear it before the next command. Fixing it here
separately from that CL.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=141939330
This would have saved me 2+ hours yesterday of mucking around
in datastore and manually copying out the xml bytes so that
I could base64decode them.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=139472542
While I am doing this, promote LoadAndResaveEntityCommand from
javascrap to the main tool since it's repeatedly been useful, and
rename it and update its documentation to better reflect the
difference between that command and the one I am adding.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=139460841
Currently we pass in null. However, from the spec:
<domain:acDate> element that contains the date and time of a
required or completed response. For a PENDING request, the value
identifies the date and time by which a response is required
before an automated response action will be taken by the server.
For all other status types, the value identifies the date and time
when the request was completed."
- https://tools.ietf.org/html/rfc5731#page-16, section 3.1.3
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=139363370
and HostResource.
DomainApplication is not transferable and has no need for this
field. HostResource needs it because it can be transferred with
a domain.
This is all in service of removing the ofy().load() inside of
host's cloneProjectedAtTime.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=139346925
HostResource and DomainApplication are not transferable, (or at
least, not directly in the case of hosts) and have no need for
the TransferData field. In a flat-flow world, we can push it down
to where it's actually used.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=139201423
1) Prefer .getConstructor().newInstance() over .newInstance()
because otherwise checked exceptions can be propagated from
the constructor even though they aren't declared.
2) Use the type T in the parameters to instantiate().
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=138874730
Calling setAccessible() cross-package used to fail in
the custom security manager. It never shouldn have failed,
and now it works correctly (not sure when it was fixed) so
remove these trampolines.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=138549703
I added shared base classes to all of the Fee extension types that
make it possible to fully ignore the version in the flows. (You
ask for a FeeCreateCommandExtension, for example, and you get one
without having to worry about which). This is an improvement over
the old code that asked you to provide a list of possible fee
extensions and then ask for the first one in preference order.
As part of this I was able to make the Fee implementation a bit
simpler as well.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=137992390
1) Don't do ofy().load() inside a model class (in DomainAuthInfo)
2) Move the one use of verify into the one caller in ResourceFlowUtils
3) Hosts don't support authInfo, so remove useless code
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=137984809
This concludes your flow flattening experience. Please
fill out a flow flattening satisfaction survey before
exiting.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=137903095
This was meant for log replay and has long been ignored/useless.
As part of this, remove execution time from EppResponse since this
was the only thing consuming it.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=137293734
There are no applications extant that predate the creationTrid field.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=136047561
It is no longer needed since we don't do log-replay and it's
annoying to support in the flat flows.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=135805088
aka regexing for fun and profit.
This also makes sure that there are no statements after the
throwing statement, since these would be dead code. There
were a surprising number of places with assertions after
the throw, and none of these are actually triggered in tests
ever. When I found these, I replaced them with try/catch/rethrow
which makes the assertions actually happen:
before:
// This is the ExceptionRule that checks EppException marshaling
thrown.expect(FooException.class);
doThrowingThing();
assertSomething(); // Dead code!
after:
try {
doThrowingThing();
assertWithMessage("...").fail();
} catch (FooException e) {
assertSomething();
// For EppExceptions:
assertAboutEppExceptins().that(e).marshalsToXml();
}
To make this work, I added EppExceptionSubject.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=135793407
aka regexing for fun and profit.
This also makes sure that there are no statements after the
throwing statement, since these would be dead code. There
were a surprising number of places with assertions after
the throw, and none of these are actually triggered in tests
ever. When I found these, I replaced them with try/catch/rethrow
which makes the assertions actually happen:
before:
// This is the ExceptionRule that checks EppException marshaling
thrown.expect(FooException.class);
doThrowingThing();
assertSomething(); // Dead code!
after:
try {
doThrowingThing();
assertWithMessage("...").fail();
} catch (FooException e) {
assertSomething();
// For EppExceptions:
assertAboutEppExceptins().that(e).marshalsToXml();
}
To make this work, I added EppExceptionSubject.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=135793407
There was a circular reference when hydrating a domain with a
subordinate host, since the host references the domain. To fix
this, I redid @DoNotHydrate to be the way it should have been,
rather than the hack I had originally submitted. I also beefed
up the unit tests of the epp resource types to check for cycles.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=135792416
Very few flows actually check the phase. Push the checks down to the leaf
flows so that we can remove the inherited code from ResourceFlow and replace
it with utility methods. In the process, document and test two places that
throw the exception but did not previously test it.
This introduces a temporary hack in BaseDomainCreateFlow that does something
specific for DomainApplicationCreateFlow. It will go away literally tomorrow
when I flatten that flow.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=135480538
This is how it should be in every test, but we used to not be
so careful about this. I had to touch this file anyways, so
I fixed it here.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=135387267
These have already been totally replaced by flattened flows.
There are a few left that will get deleted when the remaining
flat flows go in.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=134077510
These were historically separate due to the old flow
structure, but now they should be one exception.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=133984858
Also pull out a small bit of common functionality across contact and host checks.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=133977324
Also make minor javadoc tweaks to domain transfer flows
to match what we are changing in contact/host flows.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=133955677
Rename existingResource flows variable to be specific to EPP resource type and replace some explicit checks with helper methods.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=133774229
cgoldfederandBen McIlwain