Add UserAgents.of(UserAgent, Iterable<Agent>) (#902)

Add UserAgents.of(UserAgent, Iterable<Agent>)
palantir · Oct 4, 2022 · 11238a5 · 11238a5
1 parent 4148974
commit 11238a5
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 7 deletions.
diff --git a/changelog/@unreleased/pr-902.v2.yml b/changelog/@unreleased/pr-902.v2.yml
@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Add UserAgents.of(UserAgent, Iterable<Agent>)
+  links:
+  - https://github.com/palantir/conjure-java-runtime-api/pull/902
diff --git a/service-config/src/main/java/com/palantir/conjure/java/api/config/service/UserAgent.java b/service-config/src/main/java/com/palantir/conjure/java/api/config/service/UserAgent.java
@@ -18,6 +18,7 @@
 
 import com.palantir.logsafe.Preconditions;
 import com.palantir.logsafe.SafeArg;
+import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
 import java.util.List;
 import java.util.Optional;
 import org.immutables.value.Value;
@@ -56,6 +57,16 @@ static UserAgent of(Agent agent) {
         return ImmutableUserAgent.builder().primary(agent).build();
     }
 
+    /**
+     * Returns a user agent with the given base agent combined with specified additional informational agents.
+     */
+    static UserAgent of(UserAgent base, Iterable<Agent> additional) {
+        return ImmutableUserAgent.builder()
+                .from(base)
+                .addAllInformational(additional)
+                .build();
+    }
+
     /**
      * Returns a new {@link UserAgent} instance whose {@link #informational} agents are this instance's agents plus the
      * given agent.
@@ -86,13 +97,14 @@ interface Agent {
 
         @Value.Check
         default void check() {
-            Preconditions.checkArgument(
-                    UserAgents.isValidName(name()), "Illegal agent name format", SafeArg.of("name", name()));
-            // Should never hit the following.
-            Preconditions.checkArgument(
-                    UserAgents.isValidVersion(version()),
-                    "Illegal version format. This is a bug",
-                    SafeArg.of("version", version()));
+            if (!UserAgents.isValidName(name())) {
+                throw new SafeIllegalArgumentException("Illegal agent name format", SafeArg.of("name", name()));
+            }
+            if (!UserAgents.isValidVersion(version())) {
+                // Should never hit the following.
+                throw new SafeIllegalArgumentException(
+                        "Illegal version format. This is a bug", SafeArg.of("version", version()));
+            }
         }
 
         static Agent of(String name, String version) {

diff --git a/service-config/src/test/java/com/palantir/conjure/java/api/config/service/UserAgentTest.java b/service-config/src/test/java/com/palantir/conjure/java/api/config/service/UserAgentTest.java
@@ -19,7 +19,11 @@
 import static com.palantir.logsafe.testing.Assertions.assertThatLoggableExceptionThrownBy;
 import static org.assertj.core.api.Assertions.assertThat;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.palantir.conjure.java.api.config.service.UserAgent.Agent;
 import com.palantir.logsafe.SafeArg;
+import java.util.List;
 import org.junit.jupiter.api.Test;
 
 public class UserAgentTest {
@@ -59,6 +63,29 @@ public void testCorrectHeaderFormatWithoutNodeId() {
         assertThat(UserAgents.format(derivedAgent)).isEqualTo("service/1.0.0 conjure/2.0.0");
     }
 
+    @Test
+    void testPrimaryWithInformational() {
+        UserAgent baseUserAgent = UserAgent.of(Agent.of("service", "1.0.0"));
+        List<Agent> info = ImmutableList.of(Agent.of("conjure", "1.2.3"), Agent.of("jdk", "17.0.4.1"));
+        UserAgent first = UserAgent.of(baseUserAgent, info);
+        assertThat(first).satisfies(agent -> {
+            assertThat(agent.primary()).isEqualTo(baseUserAgent.primary());
+            assertThat(agent.informational()).hasSize(2).isEqualTo(info);
+            assertThat(UserAgents.format(agent)).isEqualTo("service/1.0.0 conjure/1.2.3 jdk/17.0.4.1");
+            assertThat(UserAgents.parse(UserAgents.format(agent))).isEqualTo(agent);
+            assertThat(UserAgent.of(agent, ImmutableList.of())).isEqualTo(agent);
+        });
+
+        List<Agent> moreInfo = ImmutableList.of(Agent.of("test", "4.5.6"));
+        assertThat(UserAgent.of(first, moreInfo)).satisfies(agent -> {
+            assertThat(agent.primary()).isEqualTo(baseUserAgent.primary());
+            assertThat(agent.informational()).hasSize(3).containsExactlyElementsOf(Iterables.concat(info, moreInfo));
+            assertThat(UserAgents.format(agent)).isEqualTo("service/1.0.0 conjure/1.2.3 jdk/17.0.4.1 test/4.5.6");
+            assertThat(UserAgents.parse(UserAgents.format(agent))).isEqualTo(agent);
+            assertThat(UserAgent.of(agent, ImmutableList.of())).isEqualTo(agent);
+        });
+    }
+
     @Test
     public void testInvalidServiceName() {
         assertThatLoggableExceptionThrownBy(() -> UserAgent.Agent.of("invalid service name", "1.0.0"))