Fixed the ID conversion issue for R4

bunsen/bunsen-avro/src/main/java/com/cerner/bunsen/avro/converters/DefinitionToAvroVisitor.java

@@ -80,6 +80,10 @@ public Schema getDataType() {
 
   private static final Schema DOUBLE_SCHEMA = Schema.create(Type.DOUBLE);
 
+  private static final String ID_TYPE = "id";
+
+  private static final String R4_STRING_TYPE = "http://hl7.org/fhirpath/System.String";
+
   // We cannot use Avro logical type `decimal` to represent FHIR `decimal` type. The reason is that
   // Avro `decimal` type  has a fixed scale and a maximum precision but with a fixed scale we have
   // no guarantees on the precision of the FHIR `decimal` type. See this for more details:
@@ -97,7 +101,7 @@ public Schema getDataType() {
   // TODO refactor/consolidate these two.
   static final Map<String, HapiConverter<Schema>> TYPE_TO_CONVERTER =
       ImmutableMap.<String, HapiConverter<Schema>>builder()
-          .put("id", ID_CONVERTER)
+          .put(ID_TYPE, ID_CONVERTER)
           .put("boolean", BOOLEAN_CONVERTER)
           .put("code", ENUM_CONVERTER)
           .put("markdown", STRING_CONVERTER)
@@ -116,8 +120,8 @@ public Schema getDataType() {
           .put("base64Binary", STRING_CONVERTER) // FIXME: convert to Base64
           .put("uri", STRING_CONVERTER)
           // These are added for R4 support.
+          .put(R4_STRING_TYPE, STRING_CONVERTER)
           .put("http://hl7.org/fhirpath/System.Boolean", BOOLEAN_CONVERTER)
-          .put("http://hl7.org/fhirpath/System.String", STRING_CONVERTER)
           .put("http://hl7.org/fhirpath/System.Integer", INTEGER_CONVERTER)
           .put("http://hl7.org/fhirpath/System.Long", INTEGER_CONVERTER)
           .put("http://hl7.org/fhirpath/System.Decimal", DOUBLE_CONVERTER)
@@ -387,11 +391,17 @@ public DefinitionToAvroVisitor(FhirConversionSupport fhirSupport,
 
   @Override
   public HapiConverter<Schema> visitPrimitive(String elementName, String primitiveType) {
-    // TODO for R4, `id` elements use STRING_CONVERTER because of type changes; fix it!
+    // For R4, `id` elements use `System.String` type instead of `id`; that's why we do this check.
+    // Note for resource `id`s we should use ID_CONVERTER but for type `id`s use STRING_CONVERTER;
+    // this is handled in ID_CONVERTER.
+    String realType = primitiveType;
+    if (ID_TYPE.equals(elementName) && R4_STRING_TYPE.equals(primitiveType)) {
+      realType = ID_TYPE;
+    }
     Preconditions.checkNotNull(
-        TYPE_TO_CONVERTER.get(primitiveType),
-        "No converter found for the primitive type " + primitiveType);
-    return TYPE_TO_CONVERTER.get(primitiveType);
+        TYPE_TO_CONVERTER.get(realType),
+        "No converter found for the primitive type " + primitiveType + " real: " + realType);
+    return TYPE_TO_CONVERTER.get(realType);
   }
 
   private static final Schema NULL_SCHEMA = Schema.create(Type.NULL);

bunsen/bunsen-core/src/main/java/com/cerner/bunsen/definitions/IdConverter.java

@@ -12,7 +12,11 @@ public IdConverter(T dataType) {
 
   @Override
   protected Object fromHapi(IPrimitiveType primitive) {
-    Preconditions.checkArgument(primitive instanceof IIdType);
-    return ((IIdType) primitive).getIdPart();
+    // We do this hack to work around the issue that `id` has type `System.String` in R4 (not `id`).
+    // Note `id` elements of FHIR _types_ (not resources) are strings, not `IIdType`!
+    if (primitive instanceof IIdType) {
+      return ((IIdType) primitive).getIdPart();
+    }
+    return super.fromHapi(primitive);
   }
 }

bunsen/bunsen-core/src/main/java/com/cerner/bunsen/definitions/StructureDefinitions.java

@@ -132,4 +132,5 @@ public static StructureDefinitions create(FhirContext context) {
       throw new IllegalStateException(exception);
     }
   }
+
 }

pipelines/batch/src/main/java/org/openmrs/analytics/FetchSearchPageFn.java

@@ -16,6 +16,7 @@
 package org.openmrs.analytics;
 
 import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.context.ParserOptions;
 import ca.uhn.fhir.parser.IParser;
 import com.google.common.annotations.VisibleForTesting;
 import java.beans.PropertyVetoException;
@@ -159,7 +160,19 @@ private static synchronized JdbcConnectionUtil getJdbcConnectionUtil(
   public void setup() throws SQLException, PropertyVetoException {
     log.info("Starting setup for stage " + stageIdentifier);
     fhirContext = FhirContext.forR4Cached();
+    // The documentation for `FhirContext` claims that it is thread-safe but looking at the code,
+    // it is not obvious if it is. This might be an issue when we write to it, like the next line.
+    fhirContext.setParserOptions(
+        // We want to keep the original IDs; this is particularly useful when the `fullUrl` is not
+        // a URL but a URN, e.g., `urn:uuid:...`; for example when Bundles come from JSON files.
+        // Note `IIdType.getIdPart` extracts only the logical ID part when exporting but that code
+        // path does not work for URNs (e.g., when importing files).
+        new ParserOptions().setOverrideResourceIdWithBundleEntryFullUrl(false));
     fhirContext.getRestfulClientFactory().setSocketTimeout(20000);
+    // Note this parser is not used when fetching resources from a HAPI server. That's why we need
+    // to change the `setOverrideResourceIdWithBundleEntryFullUrl` globally above such that the
+    // parsers used in the HAPI client code is impacted too.
+    parser = fhirContext.newJsonParser();
     fhirStoreUtil =
         FhirStoreUtil.createFhirStoreUtil(
             sinkPath, sinkUsername, sinkPassword, fhirContext.getRestfulClientFactory());
@@ -185,12 +198,6 @@ public void setup() throws SQLException, PropertyVetoException {
       jdbcWriter =
           new JdbcResourceWriter(dataSource, sinkDbTableName, useSingleSinkDbTable, fhirContext);
     }
-    parser = fhirContext.newJsonParser();
-    // We want to keep the original IDs; this is particularly useful when the `fullUrl` is not
-    // a URL but a URN, e.g., `urn:uuid:...`; for example when Bundles come from JSON files.
-    // This should not have an effect when `fullUrl` is a URL, because with `IIdType.getIdPart` we
-    // extract only the logical ID part when exporting (but that code path does not work for URNs).
-    parser.setOverrideResourceIdWithBundleEntryFullUrl(false);
   }
 
   @Teardown

pipelines/batch/src/test/java/org/openmrs/analytics/FhirSearchUtilTest.java

@@ -65,7 +65,7 @@ public class FhirSearchUtilTest {
 
   @BeforeClass
   public static void setupFhirContext() {
-    fhirContext = FhirContext.forR4();
+    fhirContext = FhirContext.forR4Cached();
   }
 
   @Before