Commit c5dac691 authored by Arthur Bogaart's avatar Arthur Bogaart

HHP-12 Remove release() from HTMLProcessor codebase

The release() method was added to support Wicket's detach() but we
realized that Wicket can handle the correct detaching of models itself,
thus release() should be removed from the HTMLProcessor codebase.
parent 0f287833
/*
* Copyright 2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2017-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -68,11 +68,6 @@ public class HtmlProcessorModel implements Model<String> {
}
}
@Override
public void release() {
valueModel.release();
}
public List<TagVisitor> getVisitors() {
return null;
}
......
/*
* Copyright 2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2017-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -23,8 +23,6 @@ public interface Model<T> extends Serializable {
void set(final T value);
void release();
static <T> Model<T> of(final T o) {
return new SimpleModel<>(o);
}
......@@ -47,9 +45,6 @@ public interface Model<T> extends Serializable {
this.value = value;
}
@Override
public void release() {
}
}
}
......
/*
* Copyright 2010-2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2010-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -26,9 +26,8 @@ public interface RichTextImageFactory extends Serializable {
boolean isValid(Model<Node> linkTarget);
RichTextImage loadImageItem(String uuid, String type) throws RichTextException;
RichTextImage createImageItem(Model<Node> model) throws RichTextException;
void release();
RichTextImage loadImageItem(String uuid, String type) throws RichTextException;
}
/*
* Copyright 2008-2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2008-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -187,9 +187,4 @@ public class RichTextImageFactoryImpl implements RichTextImageFactory {
}
}
@Override
public void release() {
nodeModel.release();
}
}
/*
* Copyright 2010-2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2010-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -52,7 +52,7 @@ public class RichTextImageURLProvider implements URLProvider {
final String facetName = StringUtils.substringBefore(link, PATH_SEPARATOR);
final String type = StringUtils.substringAfterLast(link, PATH_SEPARATOR);
final String uuid = FacetUtil.getChildDocBaseOrNull(node, facetName);
if (uuid != null && linkFactory.getLinkUuids().contains(uuid)) {
if (uuid != null && linkFactory.hasLink(uuid)) {
final RichTextImage rti = imageFactory.loadImageItem(uuid, type);
return rti.getUrl();
}
......
/*
* Copyright 2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2017-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -51,18 +51,18 @@ public class JcrNodeFactory implements NodeFactory {
@Override
public Model<Node> getNodeModelByNode(final Node node) {
if (node == null) {
return null;
}
try {
return new NodeModel(node);
} catch (final RepositoryException e) {
if (log.isInfoEnabled()) {
log.error("Failed to create node model from node {}", JcrUtils.getNodePathQuietly(node), e);
} else {
log.error("Failed to create node model from node {}", JcrUtils.getNodePathQuietly(node));
if (node != null) {
try {
return new NodeModel(node.getIdentifier());
} catch (final RepositoryException e) {
if (log.isInfoEnabled()) {
log.error("Failed to create node model from node {}", JcrUtils.getNodePathQuietly(node), e);
} else {
log.error("Failed to create node model from node {}", JcrUtils.getNodePathQuietly(node));
}
}
}
return null;
}
......@@ -77,23 +77,15 @@ public class JcrNodeFactory implements NodeFactory {
private class NodeModel implements Model<Node> {
private String uuid;
private Node node;
public NodeModel(final Node node) throws RepositoryException {
this.uuid = node.getIdentifier();
this.node = node;
}
public NodeModel(final String uuid) {
NodeModel(final String uuid) {
this.uuid = uuid;
}
@Override
public Node get() {
try {
if (node == null) {
node = getNodeByIdentifier(uuid);
}
return getNodeByIdentifier(uuid);
} catch (final RepositoryException e) {
if (log.isInfoEnabled()) {
log.error("Failed to load node with uuid {}", uuid, e);
......@@ -101,14 +93,13 @@ public class JcrNodeFactory implements NodeFactory {
log.error("Failed to load node with uuid {}", uuid);
}
}
return node;
return null;
}
@Override
public void set(final Node value) {
try {
uuid = value.getIdentifier();
node = value;
} catch (final RepositoryException e) {
if (log.isInfoEnabled()) {
log.error("Failed to retrieve uuid from node {}", JcrUtils.getNodePathQuietly(value), e);
......@@ -117,10 +108,5 @@ public class JcrNodeFactory implements NodeFactory {
}
}
}
@Override
public void release() {
node = null;
}
}
}
/*
* Copyright 2010-2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2010-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -16,7 +16,6 @@
package org.onehippo.cms7.services.htmlprocessor.richtext.link;
import java.io.Serializable;
import java.util.Set;
import javax.jcr.Node;
......@@ -25,13 +24,12 @@ import org.onehippo.cms7.services.htmlprocessor.richtext.RichTextException;
public interface RichTextLinkFactory extends Serializable {
Set<String> getLinkUuids();
boolean isValid(Model<Node> targetModel);
RichTextLink createLink(Model<Node> targetModel) throws RichTextException;
RichTextLink loadLink(String uuid) throws RichTextException;
void release();
boolean hasLink(String uuid);
}
/*
* Copyright 2010-2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2010-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -15,9 +15,6 @@
*/
package org.onehippo.cms7.services.htmlprocessor.richtext.link;
import java.util.Set;
import java.util.TreeSet;
import javax.jcr.Node;
import javax.jcr.NodeIterator;
import javax.jcr.RepositoryException;
......@@ -26,10 +23,10 @@ import org.hippoecm.repository.api.HippoNodeType;
import org.hippoecm.repository.api.NodeNameCodec;
import org.hippoecm.repository.util.JcrUtils;
import org.onehippo.cms7.services.htmlprocessor.model.Model;
import org.onehippo.cms7.services.htmlprocessor.util.FacetUtil;
import org.onehippo.cms7.services.htmlprocessor.util.StringUtil;
import org.onehippo.cms7.services.htmlprocessor.richtext.RichTextException;
import org.onehippo.cms7.services.htmlprocessor.richtext.jcr.NodeFactory;
import org.onehippo.cms7.services.htmlprocessor.util.FacetUtil;
import org.onehippo.cms7.services.htmlprocessor.util.StringUtil;
import org.onehippo.repository.util.JcrConstants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
......@@ -41,8 +38,6 @@ public class RichTextLinkFactoryImpl implements RichTextLinkFactory {
private final Model<Node> nodeModel;
private final NodeFactory nodeFactory;
private transient Set<String> links = null;
public RichTextLinkFactoryImpl(final Model<Node> nodeModel, final NodeFactory nodeFactory) {
this.nodeModel = nodeModel;
this.nodeFactory = nodeFactory;
......@@ -63,6 +58,25 @@ public class RichTextLinkFactoryImpl implements RichTextLinkFactory {
}
}
@Override
public boolean hasLink(final String uuid) {
try {
final NodeIterator nodeIterator = nodeModel.get().getNodes();
while (nodeIterator.hasNext()) {
final Node child = nodeIterator.nextNode();
if (child.isNodeType(HippoNodeType.NT_FACETSELECT)) {
final String linkUuid = child.getProperty(HippoNodeType.HIPPO_DOCBASE).getString();
if (linkUuid.equals(uuid)) {
return true;
}
}
}
} catch (final RepositoryException ex) {
log.error("Error checking if link with UUID {} already exists", uuid, ex);
}
return false;
}
@Override
public RichTextLink createLink(final Model<Node> targetModel) throws RichTextException {
if (targetModel == null) {
......@@ -99,30 +113,4 @@ public class RichTextLinkFactoryImpl implements RichTextLinkFactory {
return false;
}
}
@Override
public Set<String> getLinkUuids() {
if (links == null) {
links = new TreeSet<>();
try {
final NodeIterator iter = nodeModel.get().getNodes();
while (iter.hasNext()) {
final Node child = iter.nextNode();
if (child.isNodeType(HippoNodeType.NT_FACETSELECT)) {
final String uuid = child.getProperty(HippoNodeType.HIPPO_DOCBASE).getString();
links.add(uuid);
}
}
} catch (final RepositoryException ex) {
log.error("Error retrieving links", ex);
}
}
return links;
}
@Override
public void release() {
nodeModel.release();
links = null;
}
}
/*
* Copyright 2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2017-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -36,21 +36,13 @@ import org.onehippo.cms7.services.htmlprocessor.richtext.visit.ImageAndLinkVisit
public class RichTextProcessorModel extends HtmlProcessorModel {
private final Model<Node> nodeModel;
private final List<TagVisitor> visitors = new ArrayList<>();
public RichTextProcessorModel(final Model<String> valueModel, final Model<Node> nodeModel,
final HtmlProcessorFactory processorFactory, final NodeFactory nodeFactory) {
this(valueModel, nodeModel, processorFactory, nodeFactory, URLEncoder.OPAQUE);
}
public RichTextProcessorModel(final Model<String> valueModel, final Model<Node> nodeModel,
final HtmlProcessorFactory processorFactory, final NodeFactory nodeFactory,
final URLEncoder encoder) {
super(valueModel, processorFactory);
this.nodeModel = nodeModel;
final RichTextLinkFactory linkFactory = new RichTextLinkFactoryImpl(nodeModel, nodeFactory);
final RichTextImageFactory imageFactory = new RichTextImageFactoryImpl(nodeModel, nodeFactory, encoder);
final URLProvider imageProvider = createImageURLProvider(nodeModel, linkFactory, imageFactory);
......@@ -66,12 +58,4 @@ public class RichTextProcessorModel extends HtmlProcessorModel {
return visitors;
}
@Override
public void release() {
super.release();
if (nodeModel != null) {
nodeModel.release();
}
visitors.forEach(TagVisitor::release);
}
}
/*
* Copyright 2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2017-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -28,11 +28,6 @@ public abstract class NodeVisitor implements TagVisitor {
this.nodeModel = nodeModel;
}
@Override
public void release() {
nodeModel.release();
}
protected Node getNode() {
return nodeModel.get();
}
......
......@@ -277,9 +277,6 @@ public class HtmlProcessorTest {
public void after() {
}
@Override
public void release() {
}
}
}
/*
* Copyright 2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2017-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -122,19 +122,6 @@ public class HtmlProcessorModelTest {
verify(processor, processorFactory);
}
@Test
public void testRelease() throws Exception {
final Model<String> valueModel = EasyMock.createMock(Model.class);
valueModel.release();
expectLastCall();
replay(valueModel);
final HtmlProcessorModel processorModel = new HtmlProcessorModel(valueModel, null);
processorModel.release();
verify(valueModel);
}
private static HtmlProcessorFactory createProcessorFactory(final HtmlProcessor processor) {
final HtmlProcessorFactory processorFactory = EasyMock.createMock(HtmlProcessorFactory.class);
expect(processorFactory.getProcessor()).andReturn(processor);
......
/*
* Copyright 2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2017-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -122,8 +122,7 @@ public class RichTextImageFactoryTest {
assertFalse(factory.isValid(nodeFactory.getNodeModelByNode(handleWithWrongChild)));
final Node mockNode = EasyMock.createMock(Node.class);
expect(mockNode.getIdentifier()).andReturn("broken-handle-node-uuid");
expect(mockNode.isNodeType(HippoNodeType.NT_HANDLE)).andThrow(new RepositoryException("Expected exception"));
expect(mockNode.getIdentifier()).andReturn("broken-node-uuid");
replay(mockNode);
assertFalse(factory.isValid(nodeFactory.getNodeModelByNode(mockNode)));
......
/*
* Copyright 2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2017-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -15,8 +15,6 @@
*/
package org.onehippo.cms7.services.htmlprocessor.richtext.image;
import java.util.Collections;
import javax.jcr.Node;
import javax.jcr.NodeIterator;
import javax.jcr.RepositoryException;
......@@ -276,7 +274,7 @@ public class RichTextImageTagProcessorTest {
expect(mockImageFactory.loadImageItem(eq(image.getIdentifier()), eq("hippogallery:original"))).andReturn(richTextImage);
final RichTextLinkFactory mockLinkFactory = createMock(RichTextLinkFactory.class);
expect(mockLinkFactory.getLinkUuids()).andReturn(Collections.singleton(image.getIdentifier()));
expect(mockLinkFactory.hasLink(eq(image.getIdentifier()))).andReturn(true);
replay(mockImageFactory, mockLinkFactory);
......
/*
* Copyright 2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2017-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -15,8 +15,6 @@
*/
package org.onehippo.cms7.services.htmlprocessor.richtext.image;
import java.util.Collections;
import javax.jcr.Node;
import org.apache.commons.lang.StringUtils;
......@@ -101,7 +99,7 @@ public class RichTextImageUrlProviderTest {
.andReturn(richTextImage);
final RichTextLinkFactory mockLinkFactory = EasyMock.createMock(RichTextLinkFactory.class);
expect(mockLinkFactory.getLinkUuids()).andReturn(Collections.singleton(image.getIdentifier()));
expect(mockLinkFactory.hasLink(eq(image.getIdentifier()))).andReturn(true);
replay(mockImageFactory, mockLinkFactory);
......@@ -138,7 +136,7 @@ public class RichTextImageUrlProviderTest {
final RichTextImageFactory mockImageFactory = EasyMock.createMock(RichTextImageFactory.class);
final RichTextLinkFactory mockLinkFactory = EasyMock.createMock(RichTextLinkFactory.class);
expect(mockLinkFactory.getLinkUuids()).andReturn(Collections.emptySet());
expect(mockLinkFactory.hasLink(eq(image.getIdentifier()))).andReturn(false);
replay(mockImageFactory, mockLinkFactory);
......@@ -160,7 +158,7 @@ public class RichTextImageUrlProviderTest {
.andThrow(new RichTextException("Expected exception"));
final RichTextLinkFactory mockLinkFactory = EasyMock.createMock(RichTextLinkFactory.class);
expect(mockLinkFactory.getLinkUuids()).andReturn(Collections.singleton(image.getIdentifier()));
expect(mockLinkFactory.hasLink(eq(image.getIdentifier()))).andReturn(true);
replay(mockImageFactory, mockLinkFactory);
......
/*
* Copyright 2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2017-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -95,7 +95,6 @@ public class JcrNodeFactoryTest {
};
final Model<Node> nodeModel = nodeFactory.getNodeModelByNode(root);
nodeModel.release();
try (Log4jInterceptor interceptor = Log4jInterceptor.onError().trap(JcrNodeFactory.class).build()) {
nodeModel.get();
assertLogMessage(interceptor, "Failed to load node with uuid cafebabe-cafe-babe-cafe-babecafebabe", Level.ERROR);
......
/*
* Copyright 2008-2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2008-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -15,13 +15,10 @@
*/
package org.onehippo.cms7.services.htmlprocessor.richtext.link;
import java.util.Set;
import javax.jcr.Node;
import javax.jcr.RepositoryException;
import org.easymock.EasyMock;
import org.hamcrest.CoreMatchers;
import org.hippoecm.repository.api.HippoNodeType;
import org.junit.Before;
import org.junit.Test;
......@@ -34,7 +31,6 @@ import org.onehippo.repository.mock.MockNode;
import static org.easymock.EasyMock.expect;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
......@@ -102,10 +98,6 @@ public class RichTextLinkFactoryTest {
@Override
public void set(final Node value) {
}
@Override
public void release() {
}
});
fail("Should throw an exception");
} catch (final RichTextException e) {
......@@ -138,8 +130,7 @@ public class RichTextLinkFactoryTest {
assertTrue(factory.isValid(nodeFactory.getNodeModelByNode(reference)));
final Node mockNode = EasyMock.createMock(Node.class);
expect(mockNode.getIdentifier()).andReturn("mock-node-uuid");
expect(mockNode.isNodeType("mix:referenceable")).andThrow(new RepositoryException("Expected exception"));
expect(mockNode.getIdentifier()).andReturn("broken-node-uuid");
EasyMock.replay(mockNode);
assertFalse(factory.isValid(nodeFactory.getNodeModelByNode(mockNode)));
......@@ -165,9 +156,8 @@ public class RichTextLinkFactoryTest {
final MockNode second = root.addNode("second", "nt:unstructured");
factory.createLink(nodeFactory.getNodeModelByNode(second));
final Set<String> linkUuids = factory.getLinkUuids();
assertEquals(linkUuids.size(), 2);
assertThat(linkUuids, CoreMatchers.hasItems(targetHandle.getIdentifier(), second.getIdentifier()));
assertTrue(factory.hasLink(targetHandle.getIdentifier()));
assertTrue(factory.hasLink(second.getIdentifier()));
}
......
/*
* Copyright 2017 Hippo B.V. (http://www.onehippo.com)
*
* 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.
*/
package org.onehippo.cms7.services.htmlprocessor.richtext.model;
import javax.jcr.Node;
import org.junit.Test;
import org.onehippo.cms7.services.htmlprocessor.model.HtmlProcessorModel;
import org.onehippo.cms7.services.htmlprocessor.model.Model;
import static org.easymock.EasyMock.createMock;
import static org.easymock.EasyMock.expectLastCall;
import static org.easymock.EasyMock.replay;
import static org.easymock.EasyMock.verify;
public class RichTextModelTest {
@Test
public void testRelease() throws Exception {
final Model<String> valueModel = createMock(Model.class);
valueModel.release();
expectLastCall();
final Model<Node> nodeModel = createMock(Model.class);
nodeModel.release();
// The node model is released by the RichTextProcessorModel directly and indirectly by the ImageAndLinkVisitor
expectLastCall().times(2);
replay(valueModel, nodeModel);
final HtmlProcessorModel processorModel = new RichTextProcessorModel(valueModel, nodeModel, null, null);
processorModel.release();
verify(valueModel, nodeModel);
}
}
/*
* Copyright 2017 Hippo B.V. (http://www.onehippo.com)
* Copyright 2017-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -29,7 +29,6 @@ import static org.easymock.EasyMock.createMock;
import static org.easymock.EasyMock.eq;
import static org.easymock.EasyMock.expectLastCall;
import static org.easymock.EasyMock.replay;
import static org.easymock.EasyMock.verify;
public class FacetVisitorTest {
......@@ -48,17 +47,6 @@ public class FacetVisitorTest {
doc2 = root.addNode("doc2", "nt:unstructured");
}
@Test
public void nodeModelIsReleased() throws Exception {
final Model<Node> mockNodeModel = createMock(Model.class);
mockNodeModel.release();
expectLastCall();
replay(mockNodeModel);
final FacetVisitor visitor = new FacetVisitor(mockNodeModel);
visitor.release();
verify(mockNodeModel);
}
@Test
public void callsTagProcessors() throws Exception {
......
#
# Copyright 2017 Hippo B.V. (http://www.onehippo.com)
# Copyright 2017-2018 Hippo B.V. (http://www.onehippo.com)
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
......@@ -14,11 +14,9 @@
# limitations under the License.
#
Expected exception
Failed to load node with uuid cafebabe-cafe-babe-cafe-babecafebabe
Failed to retrieve uuid from node /broken-node
Failed to create node model from node /broken-node
Cannot create facet node below '/document', target UUID does not exist: 'non-existing-uuid'
Error creating image link for input
Expected exception
Failed to load node with uuid broken-node-uuid
HtmlProcessor id 'org.hippoecm.frontend.plugins.richtext.IHtmlCleanerService' has been replaced by 'richtext', please update the configuration.
HtmlProcessor id 'org.hippoecm.frontend.plugins.richtext.DefaultHtmlCleanerService' has been replaced by 'formatted', please update the configuration.
Cannot create facet node below '/document', target UUID does not exist: 'non-existing-uuid'
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment