Skip to content

Commit 8f69b48

Browse files
authored
Add a @GetterDelegate annotation for better handling of ImmutableObject fields (#2860)
This allows us to specify a getter delegation to bypass Hibernate's limitations on field types for the purposes of, e.g., using a sorted set in toString() output rather than the base Hibernate unsorted HashSet type. BUG=http://b/448631639
1 parent c33f0dc commit 8f69b48

File tree

7 files changed

+136
-5
lines changed

7 files changed

+136
-5
lines changed
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2025 The Nomulus Authors. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package google.registry.model;
16+
17+
import static java.lang.annotation.ElementType.FIELD;
18+
import static java.lang.annotation.RetentionPolicy.RUNTIME;
19+
20+
import java.lang.annotation.Documented;
21+
import java.lang.annotation.Retention;
22+
import java.lang.annotation.Target;
23+
24+
/**
25+
* A delegate getter method to be used when getting the value of an {@link ImmutableObject} field.
26+
*
27+
* <p>This is useful because Hibernate has limitations on what kinds of types can be used to
28+
* represent a field value, the most relevant being that it must be mutable. Since we use Guava's
29+
* ImmutableCollections widely, this means that a frequent pattern is to e.g. have a field be
30+
* declared as a Set (with a HashSet implementation), but then implement a getter method for that
31+
* field that returns the desired ImmutableSet or ImmutableSortedSet. For purposes where it matters
32+
* that the field be represented using the appropriate type, such as for outputting in sorted order
33+
* via toString, then declare a getter delegate as follows:
34+
*
35+
* <pre>{@code
36+
* @GetterDelegate(methodName = "getAllowedTlds")
37+
* Set<String> allowedTlds;
38+
*
39+
* public ImmutableSortedSet<String> getAllowedTlds() {
40+
* return nullToEmptyImmutableSortedCopy(allowedTlds);
41+
* }
42+
* }</pre>
43+
*/
44+
@Documented
45+
@Retention(RUNTIME)
46+
@Target(FIELD)
47+
public @interface GetterDelegate {
48+
String methodName();
49+
}

core/src/main/java/google/registry/model/ModelUtils.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,20 @@ public static Map<String, Field> getAllFields(Class<?> clazz) {
7676
return ALL_FIELDS_CACHE.get(clazz);
7777
}
7878

79-
/** Retrieves a field value via reflection. */
79+
/**
80+
* Retrieves a field value via reflection, using the field's {@link GetterDelegate} if present.
81+
*/
8082
static Object getFieldValue(Object instance, Field field) {
8183
try {
82-
return field.get(instance);
83-
} catch (IllegalAccessException e) {
84+
if (field.isAnnotationPresent(GetterDelegate.class)) {
85+
return instance
86+
.getClass()
87+
.getMethod(field.getAnnotation(GetterDelegate.class).methodName())
88+
.invoke(instance);
89+
} else {
90+
return field.get(instance);
91+
}
92+
} catch (Exception e) {
8493
throw new IllegalStateException(e);
8594
}
8695
}

core/src/main/java/google/registry/model/registrar/Registrar.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import com.google.re2j.Pattern;
5555
import google.registry.model.Buildable;
5656
import google.registry.model.CreateAutoTimestamp;
57+
import google.registry.model.GetterDelegate;
5758
import google.registry.model.JsonMapBuilder;
5859
import google.registry.model.Jsonifiable;
5960
import google.registry.model.UpdateAutoTimestamp;
@@ -245,12 +246,15 @@ public enum State {
245246
State state;
246247

247248
/** The set of TLDs which this registrar is allowed to access. */
248-
@Expose Set<String> allowedTlds;
249+
@GetterDelegate(methodName = "getAllowedTlds")
250+
@Expose
251+
Set<String> allowedTlds;
249252

250253
/** Host name of WHOIS server. */
251254
@Expose String whoisServer;
252255

253256
/** Base URLs for the registrar's RDAP servers. */
257+
@GetterDelegate(methodName = "getRdapBaseUrls")
254258
Set<String> rdapBaseUrls;
255259

256260
/**

core/src/main/java/google/registry/model/registrar/RegistrarPoc.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import com.google.common.collect.ImmutableSortedSet;
2828
import com.google.gson.annotations.Expose;
2929
import google.registry.model.Buildable;
30+
import google.registry.model.GetterDelegate;
3031
import google.registry.model.ImmutableObject;
3132
import google.registry.model.JsonMapBuilder;
3233
import google.registry.model.Jsonifiable;
@@ -110,6 +111,7 @@ public boolean isRequired() {
110111
* data is internal to the registry.
111112
*/
112113
@Enumerated(EnumType.STRING)
114+
@GetterDelegate(methodName = "getTypes")
113115
@Expose
114116
Set<Type> types;
115117

core/src/test/java/google/registry/model/registrar/RegistrarTest.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import static google.registry.testing.CertificateSamples.SAMPLE_CERT_HASH;
2424
import static google.registry.testing.DatabaseHelper.cloneAndSetAutoTimestamps;
2525
import static google.registry.testing.DatabaseHelper.createTld;
26+
import static google.registry.testing.DatabaseHelper.createTlds;
2627
import static google.registry.testing.DatabaseHelper.newTld;
2728
import static google.registry.testing.DatabaseHelper.persistResource;
2829
import static google.registry.testing.DatabaseHelper.persistResources;
@@ -760,4 +761,16 @@ void testSuccess_nonRealTldDoesntNeedEntryInBillingMap() {
760761
.setAllowedTlds(ImmutableSet.of("tld", "xn--q9jyb4c"))
761762
.build();
762763
}
764+
765+
@Test
766+
void testToString_sortsAllowedTlds() {
767+
createTlds("foo", "bar", "baz", "gon", "tri");
768+
persistResource(
769+
registrar
770+
.asBuilder()
771+
.setAllowedTlds(ImmutableSet.of("gon", "bar", "foo", "tri", "baz"))
772+
.build());
773+
assertThat(Registrar.loadByRegistrarId("registrar").toString())
774+
.contains("allowedTlds=[bar, baz, foo, gon, tri]");
775+
}
763776
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright 2025 The Nomulus Authors. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package google.registry.persistence;
16+
17+
import static com.google.common.collect.ImmutableSet.toImmutableSet;
18+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
19+
20+
import com.google.common.collect.ImmutableSet;
21+
import google.registry.model.GetterDelegate;
22+
import jakarta.persistence.Entity;
23+
import java.lang.reflect.Field;
24+
import org.junit.jupiter.api.Test;
25+
26+
/** Unit tests for Hibernate entities. */
27+
public class EntitiesTest {
28+
29+
@Test
30+
void getterDelegates_allMethodNamesExist() throws Exception {
31+
ImmutableSet<Class<?>> entityClasses =
32+
PersistenceXmlUtility.getManagedClasses().stream()
33+
.filter(clazz -> clazz.isAnnotationPresent(Entity.class))
34+
.collect(toImmutableSet());
35+
for (Class<?> clazz : entityClasses) {
36+
for (Field field : clazz.getDeclaredFields()) {
37+
if (field.isAnnotationPresent(GetterDelegate.class)) {
38+
String methodName = field.getAnnotation(GetterDelegate.class).methodName();
39+
// Note that calling getDeclaredMethod(methodName) specifically looks for a method with
40+
// no parameters; if there were a method that took e.g. one parameter, you'd need
41+
// getDeclaredMethod(methodName, param1) to find it.
42+
assertDoesNotThrow(
43+
() -> clazz.getDeclaredMethod(methodName),
44+
String.format(
45+
"Method %s() specified in the GetterDelegate "
46+
+ "annotation of field %s could not be found on class %s",
47+
methodName, field.getName(), clazz.getCanonicalName()));
48+
}
49+
}
50+
}
51+
}
52+
}

core/src/test/java/google/registry/ui/server/console/ConsoleUpdateRegistrarActionTest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,9 @@ void testSuccess_sendsEmail() throws AddressException, IOException {
203203
"The following changes were made in registry unittest environment to the"
204204
+ " registrar TheRegistrar by admin [email protected]:\n"
205205
+ "\n"
206-
+ "allowedTlds: null -> [app, dev]\n"
206+
+ "allowedTlds:\n"
207+
+ " ADDED: [app, dev]\n"
208+
+ " FINAL CONTENTS: [app, dev]\n"
207209
+ "lastPocVerificationDate: 1970-01-01T00:00:00.000Z ->"
208210
+ " 2023-12-12T00:00:00.000Z\n")
209211
.setRecipients(ImmutableList.of(new InternetAddress("[email protected]")))

0 commit comments

Comments
 (0)