issue #3437 fix remote index processing for new logical_resource_iden…

…t recs

Signed-off-by: Robin Arnold <robin.arnold@ibm.com>

fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/JDBCConstants.java

@@ -119,6 +119,9 @@ public class JDBCConstants {
     // Default code_system_id value
     public static final String DEFAULT_TOKEN_SYSTEM = "default-token-system";
 
+    // Default resource type for references without a resource type
+    public static final String RESOURCE = "Resource";
+
     /**
      * This Calendar object is not thread-safe! Use CalendarHelper#getCalendarForUTC() instead.
      */

fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/ParameterTransportVisitor.java

@@ -134,8 +134,13 @@ public void visit(ReferenceParmVal rpv) throws FHIRPersistenceException {
             logger.warning("Invalid reference parameter type: '" + resourceType + "." + rpv.getName() + "' type=" + refValue.getType().name());
             throw new IllegalArgumentException("Invalid reference parameter value. See server log for details.");
         }
+
         if (refResourceType == null) {
-            refResourceType = JDBCConstants.DEFAULT_TOKEN_SYSTEM;
+            // Prior to V0027, references without a target resource type would be assigned the
+            // DEFAULT_TOKEN_SYSTEM (having a valid system makes queries faster). For V0027,
+            // all reference values get an entry in logical_resource_ident so in order to use
+            // a valid resource type we use "Resource" instead.
+            refResourceType = JDBCConstants.RESOURCE;
         }
         adapter.referenceValue(rpv.getName(), refResourceType, refLogicalId, refVersion, compositeId);
     }

fhir-remote-index/src/main/java/com/ibm/fhir/remote/index/api/IdentityCache.java

@@ -48,4 +48,12 @@ public interface IdentityCache {
      * @return
      */
     Long getCommonCanonicalValueId(short shardKey, String url);
+
+    /**
+     * Get the database resource_type_id value for the given resourceType value
+     * @param resourceType
+     * @return
+     * @throws IllegalArgumentException if resourceType is not a valid resource type name
+     */
+    int getResourceTypeId(String resourceType);
 }

fhir-remote-index/src/main/java/com/ibm/fhir/remote/index/cache/IdentityCacheImpl.java

@@ -7,20 +7,23 @@
 package com.ibm.fhir.remote.index.cache;
 
 import java.time.Duration;
+import java.util.Collection;
 import java.util.concurrent.ConcurrentHashMap;
 
 import com.github.benmanes.caffeine.cache.Cache;
 import com.github.benmanes.caffeine.cache.Caffeine;
 import com.ibm.fhir.remote.index.api.IdentityCache;
 import com.ibm.fhir.remote.index.database.CommonCanonicalValueKey;
 import com.ibm.fhir.remote.index.database.CommonTokenValueKey;
+import com.ibm.fhir.remote.index.database.ResourceTypeValue;
 
 /**
  * Implementation of a cache we use to reduce the number of databases accesses
  * required to find the id for a given object key
  */
 public class IdentityCacheImpl implements IdentityCache {
     private final ConcurrentHashMap<String, Integer> parameterNames = new ConcurrentHashMap<>();
+    private final ConcurrentHashMap<String, Integer> resourceTypes = new ConcurrentHashMap<>();
     private final Cache<String, Integer> codeSystemCache;
     private final Cache<CommonTokenValueKey, Long> commonTokenValueCache;
     private final Cache<CommonCanonicalValueKey, Long> commonCanonicalValueCache;
@@ -47,6 +50,16 @@ public IdentityCacheImpl(int maxCodeSystemCacheSize, Duration codeSystemCacheDur
                 .build();
     }
 
+    /**
+     * Initialize the cache
+     * @param resourceTypeValues the complete list of resource types
+     */
+    public void init(Collection<ResourceTypeValue> resourceTypeValues) {
+        for (ResourceTypeValue rtv: resourceTypeValues) {
+            resourceTypes.put(rtv.getResourceType(), rtv.getResourceTypeId());
+        }
+    }
+
     @Override
     public Integer getParameterNameId(String parameterName) {
         // This should only miss if the parameter name value doesn't actually
@@ -73,4 +86,13 @@ public void addParameterName(String parameterName, int parameterNameId) {
     public Long getCommonCanonicalValueId(short shardKey, String url) {
         return commonCanonicalValueCache.get(new CommonCanonicalValueKey(shardKey, url), k -> NULL_LONG);
     }
+
+    @Override
+    public int getResourceTypeId(String resourceType) {
+        Integer resourceTypeId = resourceTypes.get(resourceType);
+        if (resourceTypeId == null) {
+            throw new IllegalArgumentException("Not a valid resource type: " + resourceType);
+        }
+        return resourceTypeId;
+    }
 }

fhir-remote-index/src/main/java/com/ibm/fhir/remote/index/database/CacheLoader.java

@@ -10,25 +10,41 @@
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.List;
 
 import com.ibm.fhir.persistence.exception.FHIRPersistenceException;
-import com.ibm.fhir.remote.index.api.IdentityCache;
+import com.ibm.fhir.remote.index.cache.IdentityCacheImpl;
 
 /**
  * Preload the cache
  */
 public class CacheLoader {
-    private final IdentityCache cache;
+    private final IdentityCacheImpl cache;
 
     /**
      * Public constructor
      * @param cache
      */
-    public CacheLoader(IdentityCache cache) {
+    public CacheLoader(IdentityCacheImpl cache) {
         this.cache = cache;
     }
 
     public void apply(Connection connection) throws FHIRPersistenceException {
+        // load the static list of resource types
+        List<ResourceTypeValue> resourceTypes = new ArrayList<>();
+        final String SELECT_RESOURCE_TYPES = "SELECT resource_type, resource_type_id FROM resource_types";
+        try (PreparedStatement ps = connection.prepareStatement(SELECT_RESOURCE_TYPES)) {
+            ResultSet rs = ps.executeQuery();
+            while (rs.next()) {
+                resourceTypes.add(new ResourceTypeValue(rs.getString(1), rs.getInt(2)));
+            }
+        } catch (SQLException x) {
+            throw new FHIRPersistenceException("fetch parameter names failed", x);
+        }
+        cache.init(resourceTypes);
+
+        // also seed the cache with all the parameter_names we know so far
         final String SQL = "SELECT parameter_name, parameter_name_id FROM parameter_names";
         try (PreparedStatement ps = connection.prepareStatement(SQL)) {
             ResultSet rs = ps.executeQuery();

fhir-remote-index/src/main/java/com/ibm/fhir/remote/index/database/LogicalResourceIdentValue.java

@@ -12,24 +12,70 @@
  * A DTO representing a record from logical_resource_ident
  */
 public class LogicalResourceIdentValue implements Comparable<LogicalResourceIdentValue> {
+    private final int resourceTypeId;
     private final String resourceType;
     private final String logicalId;
     private Long logicalResourceId;
-    private Integer resourceTypeId;
 
+    @Override
+    public String toString() {
+        StringBuilder result = new StringBuilder();
+        result.append("'");
+        result.append(resourceType);
+        result.append("/");
+        result.append(logicalId);
+        result.append("'");
+        result.append(" => ");
+        result.append(resourceTypeId);
+        result.append("/");
+        result.append(logicalResourceId);
+        return result.toString();
+    }
+
+    /**
+     * Builder for fluent creation of LogicalResourceIdentValue objects
+     */
     public static class Builder {
+        private int resourceTypeId;
         private String resourceType;
         private String logicalId;
         private Long logicalResourceId;
 
+        /**
+         * Set the resourceTypeId
+         * @param resourceTypeId
+         * @return
+         */
+        public Builder withResourceTypeId(int resourceTypeId) {
+            this.resourceTypeId = resourceTypeId;
+            return this;
+        }
+
+        /**
+         * Set the logicalResourceId
+         * @param logicalResourceId
+         * @return
+         */
         public Builder withLogicalResourceId(long logicalResourceId) {
             this.logicalResourceId = logicalResourceId;
             return this;
         }
+
+        /**
+         * Set the resourceType
+         * @param resourceType
+         * @return
+         */
         public Builder withResourceType(String resourceType) {
             this.resourceType = resourceType;
             return this;
         }
+
+        /**
+         * Set the logicalId
+         * @param logicalId
+         * @return
+         */
         public Builder withLogicalId(String logicalId) {
             this.logicalId = logicalId;
             return this;
@@ -40,7 +86,7 @@ public Builder withLogicalId(String logicalId) {
          * @return
          */
         public LogicalResourceIdentValue build() {
-            return new LogicalResourceIdentValue(resourceType, logicalId, logicalResourceId);
+            return new LogicalResourceIdentValue(resourceTypeId, resourceType, logicalId, logicalResourceId);
         }
     }
 
@@ -54,11 +100,13 @@ public static Builder builder() {
 
     /**
      * Public constructor
+     * @param resourceTypeId
      * @param resourceType
      * @param logicalId
      * @param logicalResourceId
      */
-    public LogicalResourceIdentValue(String resourceType, String logicalId, Long logicalResourceId) {
+    public LogicalResourceIdentValue(int resourceTypeId, String resourceType, String logicalId, Long logicalResourceId) {
+        this.resourceTypeId = resourceTypeId;
         this.resourceType = resourceType;
         this.logicalId = Objects.requireNonNull(logicalId);
         this.logicalResourceId = logicalResourceId;
@@ -108,11 +156,4 @@ public int compareTo(LogicalResourceIdentValue that) {
     public Integer getResourceTypeId() {
         return resourceTypeId;
     }
-
-    /**
-     * @param resourceTypeId the resourceTypeId to set
-     */
-    public void setResourceTypeId(Integer resourceTypeId) {
-        this.resourceTypeId = resourceTypeId;
-    }
 }

fhir-remote-index/src/main/java/com/ibm/fhir/remote/index/database/PlainPostgresMessageHandler.java

@@ -319,7 +319,7 @@ protected void process(String tenantId, String requestShard, String resourceType
 
     @Override
     protected void process(String tenantId, String requestShard, String resourceType, String logicalId, long logicalResourceId, ReferenceParameter p) throws FHIRPersistenceException {
-        logger.info("Processing reference parameter value:" + p.toString());
+        logger.fine(() -> "Processing reference parameter value:" + p.toString());
         ParameterNameValue parameterNameValue = getParameterNameId(p);
         LogicalResourceIdentValue lriv = lookupLogicalResourceIdentValue(p.getResourceType(), p.getLogicalId());
         this.batchedParameterValues.add(new BatchReferenceParameter(requestShard, resourceType, logicalId, logicalResourceId, parameterNameValue, p, lriv));
@@ -383,6 +383,7 @@ private LogicalResourceIdentValue lookupLogicalResourceIdentValue(String resourc
         LogicalResourceIdentValue result = this.logicalResourceIdentMap.get(key);
         if (result == null) {
             result = LogicalResourceIdentValue.builder()
+                    .withResourceTypeId(identityCache.getResourceTypeId(resourceType))
                     .withResourceType(resourceType)
                     .withLogicalId(logicalId)
                     .build();
@@ -1100,25 +1101,26 @@ private void resolveLogicalResourceIdents() throws FHIRPersistenceException {
      */
     private PreparedStatement buildLogicalResourceIdentSelectStatement(List<LogicalResourceIdentValue> values) throws SQLException {
         StringBuilder query = new StringBuilder();
-        query.append("SELECT rt.resource_type, lri.logical_id, rt.resource_type_id, lri.logical_resource_id ");
+        query.append("SELECT rt.resource_type, lri.logical_id, lri.logical_resource_id ");
         query.append("  FROM logical_resource_ident AS lri ");
-        query.append("  JOIN resource_types AS rt ON (lri.resource_type_id = rt.resource_type_id) ");
         query.append("  JOIN (VALUES ");
         for (int i=0; i<values.size(); i++) {
             if (i > 0) {
                 query.append(",");
             }
             query.append("(?,?)");
         }
-        query.append(") AS v(resource_type, logical_id) ");
-        query.append(" ON (rt.resource_type = v.resource_type AND lri.logical_id = v.logical_id)");
+        query.append(") AS v(resource_type_id, logical_id) ");
+        query.append("    ON (lri.resource_type_id = v.resource_type_id AND lri.logical_id = v.logical_id)");
+        query.append("  JOIN resource_types AS rt ON (rt.resource_type_id = v.resource_type_id)"); // convenient to get the resource type name here
         PreparedStatement ps = connection.prepareStatement(query.toString());
         // bind the parameter values
         int param = 1;
         for (LogicalResourceIdentValue val: values) {
-            ps.setString(param++, val.getResourceType());
+            ps.setInt(param++, val.getResourceTypeId());
             ps.setString(param++, val.getLogicalId());
         }
+        logger.fine(() -> "logicalResourceIdents: " + query.toString());
         return ps;
     }
 
@@ -1144,6 +1146,9 @@ protected void addMissingLogicalResourceIdents(List<LogicalResourceIdentValue> m
         try (PreparedStatement ps = connection.prepareStatement(insert.toString())) {
             int count = 0;
             for (LogicalResourceIdentValue value: missing) {
+                if (value.getResourceTypeId() == null) {
+                    logger.severe("bad value: " + value);
+                }
                 ps.setInt(1, value.getResourceTypeId());
                 ps.setString(2, value.getLogicalId());
                 ps.addBatch();
@@ -1183,8 +1188,7 @@ private List<LogicalResourceIdentValue> fetchLogicalResourceIdentIds(List<Logica
                     LogicalResourceIdentKey key = new LogicalResourceIdentKey(rs.getString(1), rs.getString(2));
                     LogicalResourceIdentValue csv = this.logicalResourceIdentMap.get(key);
                     if (csv != null) {
-                        csv.setResourceTypeId(rs.getInt(3));
-                        csv.setLogicalResourceId(rs.getLong(4));
+                        csv.setLogicalResourceId(rs.getLong(3));
                     } else {
                         // can't really happen, but be defensive
                         throw new FHIRPersistenceException("logical resource ident query returned an unexpected value");

fhir-remote-index/src/main/java/com/ibm/fhir/remote/index/database/PlainPostgresParameterBatch.java

@@ -399,7 +399,7 @@ public void addSecurity(long logicalResourceId, long commonTokenValueId) throws
      * @param refVersionId
      */
     public void addReference(long logicalResourceId, int parameterNameId, long refLogicalResourceId, Integer refVersionId) throws SQLException {
-        logger.info("Adding reference: parameterNameId:" + parameterNameId + " refLogicalResourceId:" + refLogicalResourceId + " refVersionId:" + refVersionId);
+        logger.fine(() -> "Adding reference: parameterNameId:" + parameterNameId + " refLogicalResourceId:" + refLogicalResourceId + " refVersionId:" + refVersionId);
         if (refs == null) {
             final String tablePrefix = resourceType.toLowerCase();
             final String insertString = "INSERT INTO " + tablePrefix + "_ref_values (parameter_name_id, logical_resource_id, ref_logical_resource_id, ref_version_id) VALUES (?,?,?,?)";

fhir-remote-index/src/main/java/com/ibm/fhir/remote/index/database/ResourceTypeValue.java

@@ -0,0 +1,42 @@
+/*
+ * (C) Copyright IBM Corp. 2022
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package com.ibm.fhir.remote.index.database;
+
+
+/**
+ * A DTO representing a record from the resource_types table
+ */
+public class ResourceTypeValue {
+    private final String resourceType;
+    private final int resourceTypeId;
+
+    /**
+     * Canonical constructor
+     * @param resourceType
+     * @param resourceTypeId
+     */
+    public ResourceTypeValue(String resourceType, int resourceTypeId) {
+        this.resourceType = resourceType;
+        this.resourceTypeId = resourceTypeId;
+    }
+
+
+    /**
+     * @return the resourceType
+     */
+    public String getResourceType() {
+        return resourceType;
+    }
+
+
+    /**
+     * @return the resourceTypeId
+     */
+    public int getResourceTypeId() {
+        return resourceTypeId;
+    }
+}