This is an automated email from the ASF dual-hosted git repository.
bdelacretaz pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-schema-aggregator.git
The following commit(s) were added to refs/heads/master by this push:
new d842942 SLING-10620 - validate section names
d842942 is described below
commit d842942b2b23227ad39daf844639cf5a52c04de2
Author: Bertrand Delacretaz <[email protected]>
AuthorDate: Fri Jul 16 11:08:29 2021 +0200
SLING-10620 - validate section names
---
.../aggregator/impl/DefaultSchemaAggregator.java | 24 ++++++------
.../graphql/schema/aggregator/impl/Partial.java | 13 ++++++-
.../schema/aggregator/impl/PartialConstants.java | 28 --------------
.../schema/aggregator/impl/PartialReader.java | 45 ++++++++++++++--------
.../schema/aggregator/impl/CapitalizeTest.java | 15 ++------
.../schema/aggregator/impl/PartialReaderTest.java | 24 ++++++++----
.../aggregator/impl/ProviderBundleTrackerTest.java | 4 +-
.../partials/duplicate.section.partial.txt | 4 +-
src/test/resources/partials/example.partial.txt | 4 +-
9 files changed, 76 insertions(+), 85 deletions(-)
diff --git
a/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/DefaultSchemaAggregator.java
b/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/DefaultSchemaAggregator.java
index 8942b20..0f83f41 100644
---
a/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/DefaultSchemaAggregator.java
+++
b/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/DefaultSchemaAggregator.java
@@ -37,11 +37,6 @@ import org.osgi.service.component.annotations.Reference;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import static
org.apache.sling.graphql.schema.aggregator.impl.PartialConstants.S_MUTATION;
-import static
org.apache.sling.graphql.schema.aggregator.impl.PartialConstants.S_PROLOGUE;
-import static
org.apache.sling.graphql.schema.aggregator.impl.PartialConstants.S_QUERY;
-import static
org.apache.sling.graphql.schema.aggregator.impl.PartialConstants.S_TYPES;
-
@Component(service = SchemaAggregator.class)
public class DefaultSchemaAggregator implements SchemaAggregator {
private static final Logger log =
LoggerFactory.getLogger(DefaultSchemaAggregator.class.getName());
@@ -50,17 +45,20 @@ public class DefaultSchemaAggregator implements
SchemaAggregator {
@Reference
private ProviderBundleTracker tracker;
- static String capitalize(String s) {
- if(s == null) {
+ static String capitalize(Partial.SectionName name) {
+ if(name == null) {
return null;
- } else if(s.length() > 1) {
+ }
+
+ final String s = name.toString();
+ if(s.length() > 1) {
return String.format("%s%s", s.substring(0, 1).toUpperCase(),
s.substring(1).toLowerCase());
} else {
return s.toUpperCase();
}
}
- private void copySection(Set<Partial> selected, String sectionName,
boolean inBlock, Writer target) throws IOException {
+ private void copySection(Set<Partial> selected, Partial.SectionName
sectionName, boolean inBlock, Writer target) throws IOException {
String prefixToWrite = inBlock ? String.format("%ntype %s {%n",
capitalize(sectionName)) : null;
boolean anyOutput = false;
for(Partial p : selected) {
@@ -103,10 +101,10 @@ public class DefaultSchemaAggregator implements
SchemaAggregator {
}
// copy sections that belong in the output SDL
- copySection(selected, S_PROLOGUE, false, target);
- copySection(selected, S_QUERY, true, target);
- copySection(selected, S_MUTATION, true, target);
- copySection(selected, S_TYPES, false, target);
+ copySection(selected, Partial.SectionName.PROLOGUE, false, target);
+ copySection(selected, Partial.SectionName.QUERY, true, target);
+ copySection(selected, Partial.SectionName.MUTATION, true, target);
+ copySection(selected, Partial.SectionName.TYPES, false, target);
final StringBuilder partialNames = new StringBuilder();
selected.forEach(p -> {
diff --git
a/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/Partial.java
b/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/Partial.java
index 9b3d418..21f02e5 100644
--- a/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/Partial.java
+++ b/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/Partial.java
@@ -31,16 +31,25 @@ import java.util.Set;
interface Partial {
/** A section in the partial */
interface Section {
- String getName();
+ SectionName getName();
String getDescription();
Reader getContent() throws IOException;
}
+ enum SectionName {
+ PARTIAL,
+ REQUIRES,
+ PROLOGUE,
+ QUERY,
+ MUTATION,
+ TYPES
+ };
+
/** The name of this partial */
String getName();
/** Return a specific section of the partial, by name */
- Optional<Section> getSection(String name);
+ Optional<Section> getSection(SectionName name);
/** Names of the Partials on which this one depends */
Set<String> getRequiredPartialNames();
diff --git
a/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/PartialConstants.java
b/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/PartialConstants.java
deleted file mode 100644
index aac50b2..0000000
---
a/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/PartialConstants.java
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you 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.
- */
-package org.apache.sling.graphql.schema.aggregator.impl;
-
-class PartialConstants {
- public static final String S_PARTIAL = "PARTIAL";
- public static final String S_REQUIRES = "REQUIRES";
- public static final String S_PROLOGUE = "PROLOGUE";
- public static final String S_TYPES = "TYPES";
- public static final String S_MUTATION = "MUTATION";
- public static final String S_QUERY = "QUERY";
-}
\ No newline at end of file
diff --git
a/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReader.java
b/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReader.java
index 783a9d3..b7fcea0 100644
---
a/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReader.java
+++
b/src/main/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReader.java
@@ -21,7 +21,7 @@ package org.apache.sling.graphql.schema.aggregator.impl;
import java.io.IOException;
import java.io.Reader;
import java.util.Collections;
-import java.util.HashMap;
+import java.util.EnumMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
@@ -31,7 +31,6 @@ import java.util.function.Supplier;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
-import org.apache.commons.io.IOUtils;
import org.apache.commons.io.input.BoundedReader;
/** Reader for the partials format, which parses a partial file and
@@ -43,7 +42,7 @@ class PartialReader implements Partial {
private static final Pattern SECTION_LINE = Pattern.compile("([A-Z]+)
*:(.*)");
private static final int EOL = '\n';
- private final Map<String, Section> sections = new HashMap<>();
+ private final Map<SectionName, Section> sections = new
EnumMap<>(SectionName.class);
private final String name;
private final Set<String> requiredPartialNames;
@@ -58,12 +57,12 @@ class PartialReader implements Partial {
static class ParsedSection implements Partial.Section {
private final Supplier<Reader> sectionSource;
- private final String name;
+ private final SectionName name;
private final String description;
private final int startCharIndex;
private final int endCharIndex;
- ParsedSection(Supplier<Reader> sectionSource, String name, String
description, int start, int end) {
+ ParsedSection(Supplier<Reader> sectionSource, SectionName name, String
description, int start, int end) {
this.sectionSource = sectionSource;
this.name = name;
this.description = description;
@@ -72,7 +71,7 @@ class PartialReader implements Partial {
}
@Override
- public String getName() {
+ public SectionName getName() {
return name;
}
@@ -92,7 +91,7 @@ class PartialReader implements Partial {
PartialReader(String name, Supplier<Reader> source) throws IOException {
this.name = name;
parse(source);
- final Partial.Section requirements =
sections.get(PartialConstants.S_REQUIRES);
+ final Partial.Section requirements =
sections.get(SectionName.REQUIRES);
if(requirements == null) {
requiredPartialNames = Collections.emptySet();
} else {
@@ -123,7 +122,7 @@ class PartialReader implements Partial {
final Matcher m = SECTION_LINE.matcher(line);
if(m.matches()) {
// Add previous section
- addSectionIfNameIsSet(source, sectionName,
sectionDescription, lastSectionStart, charCount - line.length());
+ addSectionIfNameIsSet(source, toSectionName(sectionName),
sectionDescription, lastSectionStart, charCount - line.length());
// And setup for the new section
sectionName = m.group(1).trim();
sectionDescription = m.group(2).trim();
@@ -137,26 +136,38 @@ class PartialReader implements Partial {
}
// Add last section
- addSectionIfNameIsSet(source, sectionName, sectionDescription,
lastSectionStart, Integer.MAX_VALUE);
+ addSectionIfNameIsSet(source, toSectionName(sectionName),
sectionDescription, lastSectionStart, Integer.MAX_VALUE);
// And validate
- if(!sections.containsKey(PARTIAL_SECTION)) {
+ if(!sections.containsKey(SectionName.PARTIAL)) {
throw new SyntaxException(String.format("Missing required %s
section", PARTIAL_SECTION));
}
}
- private void addSectionIfNameIsSet(Supplier<Reader> sectionSource, String
name, String description, int start, int end) throws SyntaxException {
- if(name != null) {
- if(sections.containsKey(name)) {
- throw new SyntaxException(String.format("Duplicate section
%s", name));
- }
- sections.put(name, new ParsedSection(sectionSource, name,
description, start, end));
+ private void addSectionIfNameIsSet(Supplier<Reader> sectionSource,
SectionName name, String description, int start, int end) throws
SyntaxException {
+ if(name == null) {
+ return;
+ }
+ if(sections.containsKey(name)) {
+ throw new SyntaxException(String.format("Duplicate section '%s'",
name));
+ }
+ sections.put(name, new ParsedSection(sectionSource, name, description,
start, end));
+ }
+
+ private SectionName toSectionName(String str) throws SyntaxException {
+ if(str == null) {
+ return null;
+ }
+ try {
+ return SectionName.valueOf(str);
+ } catch(Exception e) {
+ throw new SyntaxException(String.format("Invalid section name
'%s'", str));
}
}
@Override
- public Optional<Section> getSection(String name) {
+ public Optional<Section> getSection(Partial.SectionName name) {
final Section s = sections.get(name);
return s == null ? Optional.empty() : Optional.of(s);
}
diff --git
a/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/CapitalizeTest.java
b/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/CapitalizeTest.java
index aaca60d..9e418f0 100644
---
a/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/CapitalizeTest.java
+++
b/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/CapitalizeTest.java
@@ -20,24 +20,17 @@ package org.apache.sling.graphql.schema.aggregator.impl;
import static org.junit.Assert.assertEquals;
+import org.apache.sling.graphql.schema.aggregator.impl.Partial.SectionName;
import org.junit.Test;
public class CapitalizeTest {
@Test
- public void normalStrings() throws Exception {
- assertEquals("Voici", DefaultSchemaAggregator.capitalize("voici"));
- assertEquals("Ou bien", DefaultSchemaAggregator.capitalize("OU BIEN"));
+ public void normalValue() throws Exception {
+ assertEquals("Partial",
DefaultSchemaAggregator.capitalize(SectionName.PARTIAL));
}
@Test
- public void emptyStrings() throws Exception {
- assertEquals("", DefaultSchemaAggregator.capitalize(""));
+ public void emptyValue() throws Exception {
assertEquals(null, DefaultSchemaAggregator.capitalize(null));
}
-
- @Test
- public void shortStrings() throws Exception {
- assertEquals("A", DefaultSchemaAggregator.capitalize("a"));
- assertEquals("B", DefaultSchemaAggregator.capitalize("B"));
- }
}
\ No newline at end of file
diff --git
a/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReaderTest.java
b/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReaderTest.java
index 737f9e1..a7863cf 100644
---
a/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReaderTest.java
+++
b/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/PartialReaderTest.java
@@ -22,14 +22,11 @@ import java.io.InputStreamReader;
import java.io.Reader;
import java.io.StringReader;
import java.io.UnsupportedEncodingException;
-import java.net.URL;
-import java.nio.charset.StandardCharsets;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
import java.io.IOException;
import java.io.InputStream;
@@ -38,7 +35,7 @@ import java.util.function.Supplier;
import java.util.regex.Pattern;
import org.apache.commons.io.IOUtils;
-import org.apache.sling.graphql.schema.aggregator.U;
+import org.apache.sling.graphql.schema.aggregator.impl.Partial.SectionName;
import org.junit.Test;
public class PartialReaderTest {
@@ -46,7 +43,7 @@ public class PartialReaderTest {
private static final String NONAME = "<NO NAME>";
private void assertSection(Partial p, String name, String description,
String contentRegexp) throws IOException {
- final Optional<Partial.Section> os = p.getSection(name);
+ final Optional<Partial.Section> os =
p.getSection(SectionName.valueOf(name));
assertTrue("Expecting section " + name, os.isPresent());
final Partial.Section s = os.get();
if(description != null) {
@@ -84,7 +81,7 @@ public class PartialReaderTest {
public void parseExample() throws Exception {
final PartialReader p = new PartialReader(NONAME,
getResourceReaderSupplier("/partials/example.partial.txt"));
assertSection(p, "PARTIAL", "Example GraphQL schema partial", "The
contents.*PARTIAL.*PARTIAL.*PARTIAL.*equired section\\.");
- assertSection(p, "REQUIRE", "base.scalars, base.schema", null);
+ assertSection(p, "REQUIRES", "base.scalars, base.schema", null);
assertSection(p, "PROLOGUE", "", "The prologue content.*the aggregated
schema.*other sections\\.");
assertSection(p, "QUERY", "", "The optional query sections of all
partials are aggregated in a query \\{\\} section in the output\\.");
assertSection(p, "MUTATION", "", "The optional mutation sections of
all partials are aggregated in a mutation \\{\\} section in the output\\.");
@@ -111,19 +108,30 @@ public class PartialReaderTest {
}
@Test
+ public void invalidSectionName() throws Exception {
+ final String invalidName = "REQUIRE";
+ final Exception e = assertThrows(
+ PartialReader.SyntaxException.class,
+ () -> new PartialReader(NONAME,
getStringReaderSupplier(String.format("PARTIAL:test\n%s:something\n",
invalidName)))
+ );
+ final String expected = "Invalid section name 'REQUIRE'";
+ assertTrue(String.format("Expected %s in %s", expected,
e.getMessage()), e.getMessage().contains(expected));
+ }
+
+ @Test
public void duplicateSection() throws Exception {
final Exception e = assertThrows(
PartialReader.SyntaxException.class,
() -> new PartialReader(NONAME,
getResourceReaderSupplier("/partials/duplicate.section.partial.txt"))
);
- final String expected = "Duplicate section DUPLICATE";
+ final String expected = "Duplicate section 'QUERY'";
assertTrue(String.format("Expected %s in %s", expected,
e.getMessage()), e.getMessage().contains(expected));
}
@Test
public void requires() throws Exception {
final PartialReader p = new PartialReader(NONAME,
getResourceReaderSupplier("/partials/c.sdl.txt"));
- assertTrue("Expecting requires section",
p.getSection(PartialConstants.S_REQUIRES).isPresent());
+ assertTrue("Expecting requires section",
p.getSection(Partial.SectionName.REQUIRES).isPresent());
assertEquals("[a.sdl, b.sdl]", p.getRequiredPartialNames().toString());
}
}
\ No newline at end of file
diff --git
a/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/ProviderBundleTrackerTest.java
b/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/ProviderBundleTrackerTest.java
index 7a82e7a..1f78d9b 100644
---
a/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/ProviderBundleTrackerTest.java
+++
b/src/test/java/org/apache/sling/graphql/schema/aggregator/impl/ProviderBundleTrackerTest.java
@@ -87,7 +87,7 @@ public class ProviderBundleTrackerTest {
assertEquals(2, tracker.getSchemaProviders().size());
}
- private void assertSectionContent(Partial p, String name, String expected)
throws IOException {
+ private void assertSectionContent(Partial p, Partial.SectionName name,
String expected) throws IOException {
final Optional<Partial.Section> os = p.getSection(name);
assertTrue("Expecting section " + name, os.isPresent());
assertEquals(expected, IOUtils.toString(os.get().getContent()).trim());
@@ -98,6 +98,6 @@ public class ProviderBundleTrackerTest {
final Bundle a = U.mockProviderBundle(bundleContext, "A", ++bundleId,
"1.txt");
tracker.addingBundle(a, null);
final Partial p =
tracker.getSchemaProviders().values().iterator().next();
- assertSectionContent(p, PartialConstants.S_QUERY, "Fake query for
1.txt");
+ assertSectionContent(p, Partial.SectionName.QUERY, "Fake query for
1.txt");
}
}
diff --git a/src/test/resources/partials/duplicate.section.partial.txt
b/src/test/resources/partials/duplicate.section.partial.txt
index f92e311..81f1d04 100644
--- a/src/test/resources/partials/duplicate.section.partial.txt
+++ b/src/test/resources/partials/duplicate.section.partial.txt
@@ -1,6 +1,6 @@
PARTIAL: some description
-DUPLICATE: will be provided twice
+QUERY: will be provided twice
-DUPLICATE: here's the second one
+QUERY: here's the second one
And the last line, required for correct parsing
\ No newline at end of file
diff --git a/src/test/resources/partials/example.partial.txt
b/src/test/resources/partials/example.partial.txt
index 4c76c53..eddcf3e 100644
--- a/src/test/resources/partials/example.partial.txt
+++ b/src/test/resources/partials/example.partial.txt
@@ -8,8 +8,8 @@ The contents of the PARTIAL section are not included in the
output.
The text that follows PARTIAL is a description of this partial.
PARTIAL is the only required section.
-REQUIRE: base.scalars, base.schema
-The REQUIRE section indicates partials which are required for this
+REQUIRES: base.scalars, base.schema
+The REQUIRES section indicates partials which are required for this
one to be valid. Its contents are not included in the output.
PROLOGUE:
