From 817deeb3909f5a4088075eed76288d34c665c1e9 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 16 Apr 2021 18:21:46 -0700 Subject: [PATCH] Fix long-standing bug where running the EHR's check for missing properties in datasets ends up reparenting shared property descriptors from /Shared into the EHR folder --- ehr/src/org/labkey/ehr/EHRManager.java | 80 ++++++++++++-------------- 1 file changed, 37 insertions(+), 43 deletions(-) diff --git a/ehr/src/org/labkey/ehr/EHRManager.java b/ehr/src/org/labkey/ehr/EHRManager.java index b40bf2f5e..f39dd2bd1 100644 --- a/ehr/src/org/labkey/ehr/EHRManager.java +++ b/ehr/src/org/labkey/ehr/EHRManager.java @@ -488,21 +488,20 @@ public List ensureDatasetPropertyDescriptors(Container c, User u, boolea List datasets = study.getDatasets(); + // Hack - adding a shared EHR property to a domain ends up reparenting it to the EHR folder. They + // need to keep living in the /Shared project. Rather than introducing a brand new API for adding a shared + // PD to an existing domain, we'll directly update the owning container afterwards + Set pdsToReparentInShared = new HashSet<>(); + for (Dataset dataset : datasets) { Domain domain = dataset.getDomain(); List dprops = domain.getProperties(); - if (dprops == null) - { - _log.error("domain.getProperties() was null for: " + domain.getName()); - continue; - } boolean changed = false; List toUpdate = new ArrayList<>(); - List props = new ArrayList<>(); - props.addAll(properties); + List props = new ArrayList<>(properties); if (dataset.getCategory() != null && dataset.getCategory().equals("ClinPath") && !dataset.getName().equalsIgnoreCase("Clinpath Runs")) { String propertyURI = EHRProperties.RUNID.getPropertyDescriptor().getPropertyURI(); @@ -555,11 +554,6 @@ public List ensureDatasetPropertyDescriptors(Container c, User u, boolea } } -// if (!pd.getContainer().equals(dp.getContainer()) && !pd.getProject().equals(ContainerManager.getSharedContainer())) -// { -// messages.add("Containers do not match for: " + dp.getName() + ". Expected: " + dp.getContainer().getPath() + ", but was: " + pd.getContainer().getPath() + "."); -// } - if (!dp.getName().equals(pd.getName())) { messages.add("Case mismatch for property in dataset: " + dataset.getName() + ". Expected: " + pd.getName() + ", but was: " + dp.getName() + ". This has not been automatically changed"); @@ -576,6 +570,7 @@ public List ensureDatasetPropertyDescriptors(Container c, User u, boolea d.setPropertyURI(pd.getPropertyURI()); d.setRangeURI(pd.getRangeURI()); d.setName(pd.getName()); + pdsToReparentInShared.add(pd); changed = true; } } @@ -611,19 +606,27 @@ public List ensureDatasetPropertyDescriptors(Container c, User u, boolea } } + if (commitChanges) + { + for (PropertyDescriptor sharedPD : pdsToReparentInShared) + { + sharedPD.setContainer(ContainerManager.getSharedContainer()); + sharedPD.setProject(ContainerManager.getSharedContainer()); + OntologyManager.updatePropertyDescriptor(sharedPD); + } + } + //ensure keymanagement type if (commitChanges) { - DbSchema studySchema = DbSchema.get("study"); SQLFragment sql = new SQLFragment("UPDATE study.dataset SET keymanagementtype=?, keypropertyname=? WHERE demographicdata=? AND container=?", "GUID", "objectid", false, c.getEntityId()); - long total = new SqlExecutor(studySchema).execute(sql); + long total = new SqlExecutor(StudyService.get().getDatasetSchema()).execute(sql); messages.add("Non-demographics datasets updated to use objectId as a managed key: "+ total); } else { - DbSchema studySchema = DbSchema.get("study"); SQLFragment sql = new SQLFragment("SELECT * FROM study.dataset WHERE keymanagementtype!=? AND demographicdata=? AND container=?", "GUID", false, c.getEntityId()); - long total = new SqlExecutor(studySchema).execute(sql); + long total = new SqlExecutor(StudyService.get().getDatasetSchema()).execute(sql); if (total > 0) messages.add("Non-demographics datasets that are not using objectId as a managed key: " + total); } @@ -1127,8 +1130,7 @@ private void rebuildIndex(String table, String indexName) //NOTE: this assumes the property already exists private void updatePropertyURI(Domain d, PropertyDescriptor pd) throws SQLException { - DbSchema expSchema = DbSchema.get(ExpSchema.SCHEMA_NAME); - TableInfo propertyDomain = expSchema.getTable("propertydomain"); + DbSchema expSchema = ExperimentService.get().getSchema(); TableInfo propertyDescriptor = expSchema.getTable("propertydescriptor"); //find propertyId @@ -1279,22 +1281,22 @@ public int discardTask(Container c, User u, String taskId) throws SQLException @Override public void exec(Map map) { - Map row = new CaseInsensitiveHashMap<>(); - row.putAll(map); + Map row = new CaseInsensitiveHashMap<>(); + row.putAll(map); - if (row.containsKey("requestid") && row.get("requestid") != null) - { - row.put("requestid", null); - row.put("qcstate", null); - row.put("taskid", null); - row.put("qcstateLabel", "Request: Approved"); + if (row.containsKey("requestid") && row.get("requestid") != null) + { + row.put("requestid", null); + row.put("qcstate", null); + row.put("taskid", null); + row.put("qcstateLabel", "Request: Approved"); - requestsToQueue.add(row); - } - else - { - keysToDelete.add(row); - } + requestsToQueue.add(row); + } + else + { + keysToDelete.add(row); + } } }); @@ -1302,24 +1304,16 @@ public void exec(Map map) { if (!keysToDelete.isEmpty()) { - ti.getUpdateService().deleteRows(u, c, keysToDelete, null, new HashMap()); + ti.getUpdateService().deleteRows(u, c, keysToDelete, null, new HashMap<>()); } if (!requestsToQueue.isEmpty()) { - ti.getUpdateService().updateRows(u, c, requestsToQueue, requestsToQueue, null, new HashMap()); + ti.getUpdateService().updateRows(u, c, requestsToQueue, requestsToQueue, null, new HashMap<>()); } } - catch (InvalidKeyException e) - { - throw new RuntimeException(e); - } - catch (QueryUpdateServiceException e) - { - throw new RuntimeException(e); - } - catch (BatchValidationException e) + catch (InvalidKeyException | QueryUpdateServiceException | BatchValidationException e) { throw new RuntimeException(e); }