Skip to content

Commit

Permalink
Resolve TODOs under Shicong's name (#930)
Browse files Browse the repository at this point in the history
  • Loading branch information
hstonec committed Jan 15, 2021
1 parent 9e6f99f commit 9b5805f
Show file tree
Hide file tree
Showing 12 changed files with 14 additions and 29 deletions.
1 change: 0 additions & 1 deletion core/build.gradle
Expand Up @@ -600,7 +600,6 @@ task compileProdJS(type: JavaExec) {
closureArgs << "--js=${jsDir}/soyutils_usegoog.js"
closureArgs << "--js=${cssSourceDir}/registrar_bin.css.js"
closureArgs << "--js=${jsSourceDir}/**.js"
// TODO(shicong) Verify the compiled JS file works in Alpha
closureArgs << "--js=${externsDir}/json.js"
closureArgs << "--js=${soySourceDir}/**.js"
args closureArgs
Expand Down
13 changes: 0 additions & 13 deletions core/src/main/java/google/registry/model/domain/GracePeriod.java
Expand Up @@ -193,19 +193,6 @@ public GracePeriod cloneWithRecurringBillingEvent(VKey<BillingEvent.Recurring> r
return clone;
}

/**
* Returns a clone of this {@link GracePeriod} with prepopulated {@link #gracePeriodId} generated
* by {@link ObjectifyService#allocateId()}.
*
* <p>TODO(shicong): Figure out how to generate the id only when the entity is used for Cloud SQL.
*/
@VisibleForTesting
public GracePeriod cloneWithPrepopulatedId() {
GracePeriod clone = clone(this);
clone.gracePeriodId = ObjectifyService.allocateId();
return clone;
}

/** Entity class to represent a historic {@link GracePeriod}. */
@Entity(name = "GracePeriodHistory")
@Table(indexes = @Index(columnList = "domainRepoId"))
Expand Down
Expand Up @@ -35,7 +35,7 @@
*
* @see <a href="http://tools.ietf.org/html/rfc5910">RFC 5910</a>
* @see <a href="http://tools.ietf.org/html/rfc4034">RFC 4034</a>
* <p>TODO(shicong): Rename this class to DomainDsData.
* <p>TODO(b/177567432): Rename this class to DomainDsData.
*/
@Embed
@XmlType(name = "dsData")
Expand Down
Expand Up @@ -59,8 +59,8 @@
public class Address extends ImmutableObject implements Jsonifiable {

/** The schema validation will enforce that this has 3 lines at most. */
// TODO(shicong): Remove this field after migration. We need to figure out how to generate same
// XML from streetLine[1,2,3].
// TODO(b/177569726): Remove this field after migration. We need to figure out how to generate
// same XML from streetLine[1,2,3].
@XmlJavaTypeAdapter(NormalizedStringAdapter.class)
@Transient
List<String> street;
Expand Down
Expand Up @@ -234,7 +234,7 @@ public enum State {
* Unique registrar client id. Must conform to "clIDType" as defined in RFC5730.
*
* @see <a href="http://tools.ietf.org/html/rfc5730#section-4.2">Shared Structure Schema</a>
* <p>TODO(shicong): Rename this field to clientId
* <p>TODO(b/177568946): Rename this field to registrarId.
*/
@Id
@javax.persistence.Id
Expand Down
Expand Up @@ -58,7 +58,7 @@ static void trySave(ClaimsListShard claimsList) {

/**
* Returns the most recent revision of the {@link ClaimsListShard} in Cloud SQL, if it exists.
* TODO(shicong): Change this method to package level access after dual-read phase.
* TODO(b/177569979): Change this method to package level access after dual-read phase.
* ClaimsListShard uses this method to retrieve claims list in Cloud SQL for the comparison, and
* ClaimsListShard is not in this package.
*/
Expand Down
Expand Up @@ -69,7 +69,7 @@ public abstract class TransferData<
Set<VKey<? extends TransferServerApproveEntity>> serverApproveEntities;

// The following 3 fields are the replacement for serverApproveEntities in Cloud SQL.
// TODO(shicong): Add getter/setter for these 3 fields and use them in the application code.
// TODO(b/177589157): Make transfer flows work with Cloud SQL.
@Ignore
@Column(name = "transfer_gaining_poll_message_id")
Long gainingTransferPollMessageId;
Expand Down
Expand Up @@ -351,8 +351,8 @@ public final int getValue() {
@interface BeamPipelineCloudSqlConfigs {}

/** Dagger qualifier for the default Hibernate configurations. */
// TODO(shicong): Change annotations in this class to none public or put them in a top level
// package
// TODO(b/177568911): Change annotations in this class to none public or put them in a top level
// package.
@Qualifier
@Documented
public @interface DefaultHibernateConfigs {}
Expand Down
Expand Up @@ -20,7 +20,6 @@

/**
* JPA {@link AttributeConverter} for storing/retrieving {@code List<CidrAddressBlock>} objects.
* TODO(shicong): Investigate if we can have one converter for any List type
*/
@Converter(autoApply = true)
public class CidrAddressBlockListConverter extends StringListConverterBase<CidrAddressBlock> {
Expand Down
Expand Up @@ -76,8 +76,9 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
// EntityManagerFactory is thread safe.
private final EntityManagerFactory emf;
private final Clock clock;
// TODO(shicong): Investigate alternatives for managing transaction information. ThreadLocal adds
// an unnecessary restriction that each request has to be processed by one thread synchronously.
// TODO(b/177588434): Investigate alternatives for managing transaction information. ThreadLocal
// adds an unnecessary restriction that each request has to be processed by one thread
// synchronously.
private final ThreadLocal<TransactionInfo> transactionInfo =
ThreadLocal.withInitial(TransactionInfo::new);

Expand Down
Expand Up @@ -221,8 +221,7 @@ private static DomainBase addGracePeriodForSql(DomainBase domainBase) {
.setGracePeriods(
ImmutableSet.of(
GracePeriod.create(
GracePeriodStatus.ADD, "domainRepoId", END_OF_TIME, "clientId", null)
.cloneWithPrepopulatedId()))
GracePeriodStatus.ADD, "domainRepoId", END_OF_TIME, "clientId", null)))
.build();
}

Expand Down
Expand Up @@ -48,8 +48,8 @@
* Unit tests for SQL only APIs defined in {@link JpaTransactionManagerImpl}. Note that the tests
* for common APIs in {@link TransactionManager} are added in {@link TransactionManagerTest}.
*
* <p>TODO(shicong): Remove duplicate tests that covered by TransactionManagerTest by refactoring
* the test schema.
* <p>TODO(b/177587763): Remove duplicate tests that covered by TransactionManagerTest by
* refactoring the test schema.
*/
class JpaTransactionManagerImplTest {

Expand Down

0 comments on commit 9b5805f

Please sign in to comment.