Validate strings (#907)

* conformShotName * Unittest for shot * Add non-existent show test * conformShowName * Add show creation tests
AcademySoftwareFoundation · Feb 7, 2021 · 67454c9 · 67454c9
1 parent 014bcef
commit 67454c9
Show file tree

Hide file tree

Showing 7 changed files with 166 additions and 5 deletions.
diff --git a/cuebot/src/main/java/com/imageworks/spcue/service/AdminManagerService.java b/cuebot/src/main/java/com/imageworks/spcue/service/AdminManagerService.java
@@ -40,6 +40,7 @@
 import com.imageworks.spcue.dao.LimitDao;
 import com.imageworks.spcue.dao.ShowDao;
 import com.imageworks.spcue.dao.SubscriptionDao;
+import com.imageworks.spcue.service.JobSpec;
 
 @Transactional
 public class AdminManagerService implements AdminManager {
@@ -71,6 +72,8 @@ public boolean showExists(String name) {
 
     public void createShow(ShowEntity show) {
 
+        show.name = JobSpec.conformShowName(show.name);
+
         DepartmentInterface dept = getDefaultDepartment();
         showDao.insertShow(show);
 

diff --git a/cuebot/src/main/java/com/imageworks/spcue/service/JobSpec.java b/cuebot/src/main/java/com/imageworks/spcue/service/JobSpec.java
@@ -157,11 +157,13 @@ public String conformJobName(String name) {
         return String.format("%s_%s", prefix, suffix).toLowerCase();
     }
 
-    public String conformLayerName(String name) {
+    public static String conformName(String type, String name) {
+
+         String lowerType = type.toLowerCase();
 
         if (name.length() < 3) {
             throw new SpecBuilderException(
-                    "The layer name must be at least 3 characters.");
+                    "The " + lowerType + " name must be at least 3 characters.");
         }
 
         String newName = name;
@@ -170,14 +172,26 @@ public String conformLayerName(String name) {
 
         Matcher matcher = NAME_PATTERN.matcher(newName);
         if (!matcher.matches()) {
-            throw new SpecBuilderException("The layer name: " + newName
-                    + " is not in the proper format.  Layer names must be "
+            throw new SpecBuilderException("The " + lowerType + " name: " + newName
+                    + " is not in the proper format.  " + type + " names must be "
                     + "alpha numeric, no dashes or punctuation.");
         }
 
         return newName;
     }
 
+    public static String conformShowName(String name) {
+        return conformName("Show", name);
+    }
+
+    public static String conformShotName(String name) {
+        return conformName("Shot", name);
+    }
+
+    public static String conformLayerName(String name) {
+        return conformName("Layer", name);
+    }
+
     public static final String FRAME_NAME_REGEX = "^([\\d]{4,6})-([\\w]+)$";
 
     public static final Pattern FRAME_NAME_PATTERN = Pattern
@@ -204,7 +218,7 @@ private void handleSpecTag() {
         }
 
         show = rootElement.getChildTextTrim("show");
-        shot = rootElement.getChildTextTrim("shot");
+        shot = conformShotName(rootElement.getChildTextTrim("shot"));
         user = rootElement.getChildTextTrim("user");
         uid = Optional.ofNullable(rootElement.getChildTextTrim("uid")).map(Integer::parseInt);
         email = rootElement.getChildTextTrim("email");

diff --git a/cuebot/src/test/java/com/imageworks/spcue/test/service/AdminManagerTests.java b/cuebot/src/test/java/com/imageworks/spcue/test/service/AdminManagerTests.java
@@ -30,11 +30,16 @@
 
 import com.imageworks.spcue.AllocationEntity;
 import com.imageworks.spcue.LimitInterface;
+import com.imageworks.spcue.SpecBuilderException;
 import com.imageworks.spcue.ShowEntity;
 import com.imageworks.spcue.config.TestAppConfig;
 import com.imageworks.spcue.dao.FacilityDao;
+import com.imageworks.spcue.dao.ShowDao;
 import com.imageworks.spcue.service.AdminManager;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
 @Transactional
 @ContextConfiguration(classes=TestAppConfig.class, loader=AnnotationConfigContextLoader.class)
 public class AdminManagerTests extends AbstractTransactionalJUnit4SpringContextTests  {
@@ -45,6 +50,9 @@ public class AdminManagerTests extends AbstractTransactionalJUnit4SpringContextT
     @Resource
     FacilityDao facilityDao;
 
+    @Resource
+    ShowDao showDao;
+
     private static final String TEST_ALLOC_NAME = "testAlloc";
 
     @Test
@@ -64,6 +72,24 @@ public void createShow() {
         ShowEntity show = new ShowEntity();
         show.name = "testtest";
         adminManager.createShow(show);
+        ShowEntity result = showDao.findShowDetail(show.name);
+        assertEquals(result.name, show.name);
+    }
+
+    @Test
+    @Transactional
+    @Rollback(true)
+    public void createInvalidShow() {
+        ShowEntity show = new ShowEntity();
+        show.name = "test/test";
+        try {
+            adminManager.createShow(show);
+            fail("Expected exception");
+        } catch (SpecBuilderException e) {
+            assertEquals(e.getMessage(),
+                    "The show name: test/test is not in the proper format.  " +
+                    "Show names must be alpha numeric, no dashes or punctuation.");
+        }
     }
 
     @Test

diff --git a/cuebot/src/test/java/com/imageworks/spcue/test/service/JobManagerTests.java b/cuebot/src/test/java/com/imageworks/spcue/test/service/JobManagerTests.java
@@ -34,6 +34,7 @@
 
 import com.imageworks.spcue.DispatchFrame;
 import com.imageworks.spcue.DispatchHost;
+import com.imageworks.spcue.EntityCreationError;
 import com.imageworks.spcue.FrameInterface;
 import com.imageworks.spcue.FrameStateTotals;
 import com.imageworks.spcue.JobDetail;
@@ -70,6 +71,7 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 
 @Transactional
@@ -280,6 +282,21 @@ public void testMisNamedJob2() {
                                          "pipe-dev.cue-testuser_blah_blah_v1");
     }
 
+    @Test
+    @Transactional
+    @Rollback(true)
+    public void testNonExistentShow() {
+        JobSpec spec = jobLauncher.parse(new File("src/test/resources/conf/jobspec/jobspec_nonexistent_show.xml"));
+        try {
+            jobLauncher.launch(spec);
+            fail("Expected exception");
+        } catch (EntityCreationError e) {
+            assertEquals(e.getMessage(),
+                    "The nonexistentshow does not exist. Please contact administrator of your " +
+                    "OpenCue deployment to have this show created.");
+        }
+    }
+
     @Test
     @Transactional
     @Rollback(true)

diff --git a/cuebot/src/test/java/com/imageworks/spcue/test/service/JobSpecTests.java b/cuebot/src/test/java/com/imageworks/spcue/test/service/JobSpecTests.java
@@ -82,4 +82,17 @@ public void testParseNonExistent() {
                     "Failed to parse job spec XML, java.net.MalformedURLException");
         }
     }
+
+    @Test
+    public void testParseInvalidShot() {
+        String xml = readJobSpec("jobspec_invalid_shot.xml");
+        try {
+            jobLauncher.parse(xml);
+            fail("Expected exception");
+        } catch (SpecBuilderException e) {
+            assertEquals(e.getMessage(),
+                    "The shot name: invalid/shot is not in the proper format.  " +
+                    "Shot names must be alpha numeric, no dashes or punctuation.");
+        }
+    }
 }
diff --git a/cuebot/src/test/resources/conf/jobspec/jobspec_invalid_shot.xml b/cuebot/src/test/resources/conf/jobspec/jobspec_invalid_shot.xml
@@ -0,0 +1,47 @@
+<?xml version="1.0"?>
+<!--
+   Copyright Contributors to the OpenCue Project
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+-->
+
+
+
+
+<!DOCTYPE spec PUBLIC "SPI Cue Specification Language" "http://localhost:8080/spcue/dtd/cjsl-1.10.dtd">
+<spec>
+    <facility>local</facility>
+    <show>testing</show>
+    <shot>invalid/shot</shot>
+    <user>testuser</user>
+    <uid>9860</uid>
+
+    <job name="test">
+        <paused>False</paused>
+        <maxretries>2</maxretries>
+        <autoeat>False</autoeat>
+        <env/>
+        <layers>
+            <layer name="test" type="Render">
+                <cmd>echo hello</cmd>
+                <range>1</range>
+                <chunk>1</chunk>
+                <env/>
+                <services>
+                    <service>shell</service>
+                </services>
+            </layer>
+        </layers>
+    </job>
+    <depends/>
+</spec>
diff --git a/cuebot/src/test/resources/conf/jobspec/jobspec_nonexistent_show.xml b/cuebot/src/test/resources/conf/jobspec/jobspec_nonexistent_show.xml
@@ -0,0 +1,41 @@
+<?xml version="1.0"?>
+<!--
+   Copyright Contributors to the OpenCue Project
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+-->
+
+
+
+
+<!DOCTYPE spec SYSTEM "../dtd/cjsl-1.5.dtd">
+<spec>
+
+    <show>nonexistentshow</show>
+    <shot>dev.cue</shot>
+    <user>testuser</user>
+    <email>middle-tier@imageworks.com</email>
+    <uid>9860</uid>
+
+    <job name="test">
+        <paused>true</paused>
+        <maxretries>0</maxretries>
+        <layers>
+            <layer name="test" type="RENDER">
+                <cmd>echo test</cmd>
+                <range>1</range>
+                <chunk>1</chunk>
+            </layer>
+        </layers>
+    </job>
+</spec>