Merge pull request #2683 from ControlSystemStudio/CSSTUDIO-1934

Trim pv name in PVPool

Author: kasemir

core/pv/src/main/java/org/phoebus/pv/PVPool.java

@@ -153,12 +153,13 @@ public String toString()
      */
     public static Set<String> getNameVariants(final String name, final String [] equivalent_pv_prefixes)
     {
+        final String _name = name.trim();
         // First, look for name as given
         final Set<String> variants = new LinkedHashSet<>();
-        variants.add(name);
+        variants.add(_name);
         if (equivalent_pv_prefixes != null  &&  equivalent_pv_prefixes.length > 0)
         {   // Optionally, if the original name is one of the equivalent types ...
-            final TypedName typed = TypedName.analyze(name);
+            final TypedName typed = TypedName.analyze(_name);
             for (String type : equivalent_pv_prefixes)
                 if (type.equals(typed.type))
                 {
@@ -201,15 +202,16 @@ public static Collection<String> getSupportedPrefixes()
      */
     public static PV getPV(final String name) throws Exception
     {
-        if (name.isBlank())
+        final String _name = name.trim();
+        if (_name.isBlank())
             throw new Exception("Empty PV name");
-        final TypedName type_name = TypedName.analyze(name);
+        final TypedName type_name = TypedName.analyze(_name);
         final PVFactory factory = factories.get(type_name.type);
         if (factory == null)
-            throw new Exception(name + " has unknown PV type '" + type_name.type + "'");
+            throw new Exception(_name + " has unknown PV type '" + type_name.type + "'");
 
-        final String core_name = factory.getCoreName(name);
-        final ReferencedEntry<PV> ref = pool.createOrGet(core_name, () -> createPV(factory, name, type_name.name));
+        final String core_name = factory.getCoreName(_name);
+        final ReferencedEntry<PV> ref = pool.createOrGet(core_name, () -> createPV(factory, _name, type_name.name));
         logger.log(Level.CONFIG, () -> "PV '" + ref.getEntry().getName() + "' references: " + ref.getReferences());
         return ref.getEntry();
     }

core/pv/src/test/java/org/phoebus/pv/PVPoolTest.java

@@ -57,14 +57,47 @@ public void equivalentPVs()
         assertThat(pvs, hasItem("ca://ramp"));
         assertThat(pvs, hasItem("pva://ramp"));
 
+        // Repeat to verify trimming of pv name
+        pvs = PVPool.getNameVariants("pva://ramp\n", equivalent_pv_prefixes);
+        assertThat(pvs.size(), equalTo(3));
+        assertThat(pvs, hasItem("ramp"));
+        assertThat(pvs, hasItem("ca://ramp"));
+        assertThat(pvs, hasItem("pva://ramp"));
+
+        pvs = PVPool.getNameVariants("pva://ramp ", equivalent_pv_prefixes);
+        assertThat(pvs.size(), equalTo(3));
+        assertThat(pvs, hasItem("ramp"));
+        assertThat(pvs, hasItem("ca://ramp"));
+        assertThat(pvs, hasItem("pva://ramp"));
+
+
         // For loc or sim which are not in the equivalent list, pass name through
         pvs = PVPool.getNameVariants("loc://ramp", equivalent_pv_prefixes);
         assertThat(pvs.size(), equalTo(1));
         assertThat(pvs, hasItem("loc://ramp"));
 
+        // Repeat to verify trimming of pv name
+        pvs = PVPool.getNameVariants("loc://ramp\n", equivalent_pv_prefixes);
+        assertThat(pvs.size(), equalTo(1));
+        assertThat(pvs, hasItem("loc://ramp"));
+
+        pvs = PVPool.getNameVariants("loc://ramp ", equivalent_pv_prefixes);
+        assertThat(pvs.size(), equalTo(1));
+        assertThat(pvs, hasItem("loc://ramp"));
+
         pvs = PVPool.getNameVariants("sim://ramp", equivalent_pv_prefixes);
         assertThat(pvs.size(), equalTo(1));
         assertThat(pvs, hasItem("sim://ramp"));
+
+        // Repeat to verify trimming of pv name
+        pvs = PVPool.getNameVariants("sim://ramp\n", equivalent_pv_prefixes);
+        assertThat(pvs.size(), equalTo(1));
+        assertThat(pvs, hasItem("sim://ramp"));
+
+        pvs = PVPool.getNameVariants("sim://ramp ", equivalent_pv_prefixes);
+        assertThat(pvs.size(), equalTo(1));
+        assertThat(pvs, hasItem("sim://ramp"));
+
     }
 
 

core/pv/src/test/java/org/phoebus/pv/SimPVTest.java

@@ -37,6 +37,42 @@ public void demoSine() throws Exception
         PVPool.releasePV(pv);
     }
 
+    @Test
+    public void demoSineWithTrimming1() throws Exception
+    {
+        final CountDownLatch done = new CountDownLatch(3);
+
+        System.out.println("Awaiting " + done.getCount() + " updates...");
+        final PV pv = PVPool.getPV("sim://sine\n");
+        final Disposable flow = pv.onValueEvent()
+                .subscribe(value ->
+                {
+                    System.out.println("Received update " + value);
+                    done.countDown();
+                });
+        done.await();
+        flow.dispose();
+        PVPool.releasePV(pv);
+    }
+
+    @Test
+    public void demoSineWithTrimming2() throws Exception
+    {
+        final CountDownLatch done = new CountDownLatch(3);
+
+        System.out.println("Awaiting " + done.getCount() + " updates...");
+        final PV pv = PVPool.getPV("sim://sine ");
+        final Disposable flow = pv.onValueEvent()
+                .subscribe(value ->
+                {
+                    System.out.println("Received update " + value);
+                    done.countDown();
+                });
+        done.await();
+        flow.dispose();
+        PVPool.releasePV(pv);
+    }
+
     @Test
     public void demoConst() throws Exception
     {