Skip to content

Commit 5ae9f20

Browse files
authored
Refactor BannedElement (#1506)
* refactor BannedElement * merge with master * address review
1 parent bdd47c1 commit 5ae9f20

14 files changed

Lines changed: 139 additions & 32 deletions

File tree

plugins/com.google.cloud.tools.eclipse.appengine.validation.test/src/com/google/cloud/tools/eclipse/appengine/validation/AbstractXmlValidatorTest.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
public class AbstractXmlValidatorTest {
4040

4141
private final String ELEMENT_MESSAGE = "Project ID should be specified at deploy time.";
42-
private final String SERVLET_MARKER = "com.google.cloud.tools.eclipse.appengine.validation.servletMarker";
4342
private static IResource resource;
4443
private static IProject project;
4544

@@ -64,9 +63,9 @@ public void tearDown() throws CoreException {
6463
@Test
6564
public void testCreateMarker() throws CoreException {
6665
BannedElement element = new BannedElement(ELEMENT_MESSAGE);
67-
AppEngineWebXmlValidator.createMarker(
68-
resource, element, 0, SERVLET_MARKER, IMarker.SEVERITY_ERROR);
69-
IMarker[] markers = resource.findMarkers(SERVLET_MARKER, true, IResource.DEPTH_ZERO);
66+
String markerId = "org.eclipse.core.resources.problemmarker";
67+
AppEngineWebXmlValidator.createMarker(resource, element, 0);
68+
IMarker[] markers = resource.findMarkers(markerId, true, IResource.DEPTH_ZERO);
7069
assertEquals(ELEMENT_MESSAGE, (String) markers[0].getAttribute(IMarker.MESSAGE));
7170
}
7271

plugins/com.google.cloud.tools.eclipse.appengine.validation.test/src/com/google/cloud/tools/eclipse/appengine/validation/BannedElementTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public void testBannedElementConstructor_nullElementName() {
2727

2828
@Test(expected = NullPointerException.class)
2929
public void testBannedElementConstructor_nullLocation() {
30-
new BannedElement("test", null, 0);
30+
new BannedElement("test", "org.eclipse.core.resources.problemmarker", 1, null, 0);
3131
}
3232

3333
}

plugins/com.google.cloud.tools.eclipse.appengine.validation.test/src/com/google/cloud/tools/eclipse/appengine/validation/ValidationUtilsTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public class ValidationUtilsTest {
5959
@Before
6060
public void setUp() {
6161
DocumentLocation start = new DocumentLocation(2, 1);
62-
element = new BannedElement("application", start, 1);
62+
element = new BannedElement("application", "", 1, start, 1);
6363
blacklist.add(element);
6464
}
6565

@@ -98,7 +98,7 @@ public void testGetOffsetMap_mixedXml() throws IOException {
9898
byte[] bytes = MIXED_XML_WITH_PROJECT_ID.getBytes(StandardCharsets.UTF_8);
9999
blacklist.clear();
100100
DocumentLocation start = new DocumentLocation(3, 1);
101-
BannedElement element = new BannedElement("application", start, 1);
101+
BannedElement element = new BannedElement("application", "", 1, start, 1);
102102
blacklist.add(element);
103103
SaxParserResults result = new SaxParserResults(blacklist, "UTF-8");
104104
Map<BannedElement, Integer> map = ValidationUtils.getOffsetMap(bytes, result);
@@ -121,7 +121,7 @@ public void testGetOffsetMap_lineWithWhitespace() throws IOException {
121121
public void testGetOffsetMap_firstElement() throws IOException {
122122
blacklist.clear();
123123
DocumentLocation start = new DocumentLocation(1, 1);
124-
BannedElement newElement = new BannedElement("application", start, 1);
124+
BannedElement newElement = new BannedElement("application", "", 1, start, 1);
125125
blacklist.add(newElement);
126126
byte[] bytes = XML_WITH_PROJECT_ID_FIRST.getBytes(StandardCharsets.UTF_8);
127127
SaxParserResults result = new SaxParserResults(blacklist, "UTF-8");

plugins/com.google.cloud.tools.eclipse.appengine.validation/src/com/google/cloud/tools/eclipse/appengine/validation/AbstractXmlValidator.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ static void deleteMarkers(IResource resource) throws CoreException {
6767
/**
6868
* Creates a marker from a given {@link BannedElement}
6969
*/
70-
static void createMarker(IResource resource, BannedElement element,
71-
int elementOffset, String markerId, int severity) throws CoreException {
72-
IMarker marker = resource.createMarker(markerId);
73-
marker.setAttribute(IMarker.SEVERITY, severity);
70+
static void createMarker(IResource resource, BannedElement element, int elementOffset)
71+
throws CoreException {
72+
IMarker marker = resource.createMarker(element.getMarkerId());
73+
marker.setAttribute(IMarker.SEVERITY, element.getSeverity());
7474
marker.setAttribute(IMarker.MESSAGE, element.getMessage());
7575
marker.setAttribute(IMarker.LOCATION, "line " + element.getStart().getLineNumber());
7676
marker.setAttribute(IMarker.CHAR_START, elementOffset);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Copyright 2017 Google Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.cloud.tools.eclipse.appengine.validation;
18+
19+
import org.eclipse.core.resources.IMarker;
20+
21+
/**
22+
* A blacklisted element that will receive an App Engine blacklist marker.
23+
*/
24+
public class AppEngineBlacklistElement extends BannedElement {
25+
26+
private static final String markerId =
27+
"com.google.cloud.tools.eclipse.appengine.validation.appEngineBlacklistMarker";
28+
private static final int severity = IMarker.SEVERITY_WARNING;
29+
30+
public AppEngineBlacklistElement(String message, DocumentLocation start, int length) {
31+
super(message, markerId, severity, start, length);
32+
}
33+
34+
}

plugins/com.google.cloud.tools.eclipse.appengine.validation/src/com/google/cloud/tools/eclipse/appengine/validation/AppEngineWebXmlValidator.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.util.Map;
2121
import javax.xml.parsers.ParserConfigurationException;
2222

23-
import org.eclipse.core.resources.IMarker;
2423
import org.eclipse.core.resources.IResource;
2524
import org.eclipse.core.runtime.CoreException;
2625
import org.xml.sax.SAXException;
@@ -31,7 +30,7 @@
3130
public class AppEngineWebXmlValidator extends AbstractXmlValidator {
3231

3332
/**
34-
* Clears all problem markers from the resource, then adds a blacklist marker to
33+
* Clears all problem markers from the resource, then adds a marker to
3534
* appengine-web.xml for every {@link BannedElement} found in the file.
3635
*/
3736
@Override
@@ -42,10 +41,8 @@ protected void validate(IResource resource, byte[] bytes)
4241
SaxParserResults parserResults = BlacklistSaxParser.readXml(bytes);
4342
Map<BannedElement, Integer> bannedElementOffsetMap =
4443
ValidationUtils.getOffsetMap(bytes, parserResults);
45-
String markerId = "com.google.cloud.tools.eclipse.appengine.validation.appEngineBlacklistMarker";
4644
for (Map.Entry<BannedElement, Integer> entry : bannedElementOffsetMap.entrySet()) {
47-
createMarker(resource, entry.getKey(), entry.getValue(),
48-
markerId, IMarker.SEVERITY_WARNING);
45+
createMarker(resource, entry.getKey(), entry.getValue());
4946
}
5047
} catch (SAXException ex) {
5148
createSaxErrorMessage(resource, ex);

plugins/com.google.cloud.tools.eclipse.appengine.validation/src/com/google/cloud/tools/eclipse/appengine/validation/BannedElement.java

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,29 @@ public class BannedElement {
2626
private final String message;
2727
private final DocumentLocation start;
2828
private final int length;
29+
private final String markerId;
30+
private final int severity;
2931

3032
/**
3133
* @param length the length of the marker underline. Length == 0 results in a
3234
* marker in the vertical ruler and no underline
3335
*/
34-
public BannedElement(String message, DocumentLocation start, int length) {
36+
public BannedElement(String message, String markerId, int severity,
37+
DocumentLocation start, int length) {
3538
Preconditions.checkNotNull(message, "message is null");
39+
Preconditions.checkNotNull(markerId, "markerId is null");
3640
Preconditions.checkNotNull(start, "start is null");
3741
Preconditions.checkArgument(length >= 0, "length < 0");
3842
this.message = message;
3943
this.start = start;
4044
this.length = length;
45+
this.markerId = markerId;
46+
this.severity = severity;
4147
}
4248

4349
public BannedElement(String message) {
44-
this(message, new DocumentLocation(0, 0), 0);
50+
this(message, "org.eclipse.core.resources.problemmarker",
51+
1, new DocumentLocation(0, 0), 0);
4552
}
4653

4754
public String getMessage() {
@@ -55,5 +62,13 @@ public DocumentLocation getStart() {
5562
public int getLength() {
5663
return length;
5764
}
65+
66+
String getMarkerId() {
67+
return markerId;
68+
}
69+
70+
int getSeverity() {
71+
return severity;
72+
}
5873

5974
}

plugins/com.google.cloud.tools.eclipse.appengine.validation/src/com/google/cloud/tools/eclipse/appengine/validation/BlacklistScanner.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public void startElement(String uri, String localName, String qName, Attributes
3434
DocumentLocation start = new DocumentLocation(locator.getLineNumber(),
3535
locator.getColumnNumber() - qName.length() - 2);
3636
String message = AppEngineWebBlacklist.getBlacklistElementMessage(qName);
37-
BannedElement element = new BannedElement(message, start, qName.length() + 2);
37+
BannedElement element = new AppEngineBlacklistElement(message, start, qName.length() + 2);
3838
addToBlacklist(element);
3939
}
4040
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Copyright 2017 Google Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.cloud.tools.eclipse.appengine.validation;
18+
19+
import org.eclipse.core.resources.IMarker;
20+
21+
/**
22+
* A blacklisted java servlet element that will receive a servlet marker.
23+
*/
24+
public class JavaServletElement extends BannedElement {
25+
26+
private static final String markerId =
27+
"com.google.cloud.tools.eclipse.appengine.validation.servletMarker";
28+
private static final int severity = IMarker.SEVERITY_ERROR;
29+
30+
public JavaServletElement(String message, DocumentLocation start, int length) {
31+
super(message, markerId, severity, start, length);
32+
}
33+
34+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Copyright 2017 Google Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.cloud.tools.eclipse.appengine.validation;
18+
19+
import org.eclipse.core.resources.IMarker;
20+
21+
/**
22+
* A blacklisted group ID element that will receive an App Engine Maven plugin marker.
23+
*/
24+
public class MavenPluginElement extends BannedElement {
25+
26+
private static final String markerId =
27+
"com.google.cloud.tools.eclipse.appengine.validation.mavenPluginMarker";
28+
private static final int severity = IMarker.SEVERITY_WARNING;
29+
30+
public MavenPluginElement(String message, DocumentLocation start, int length) {
31+
super(message, markerId, severity, start, length);
32+
}
33+
34+
}

0 commit comments

Comments
 (0)