From edf8c7e9f084023e1866c5c1260962df6e486bf1 Mon Sep 17 00:00:00 2001 From: Kriss Seglins <101052651+krissvaa@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:16:11 +0300 Subject: [PATCH] fix: generator use only public getters and setters (#2783) * Use only public getters and setters in generator Fixes: #2587 * Use only public getters and setters in generator Fixes: #2587 * Add integration test --------- Co-authored-by: Anton Platonov --- .../plugins/backbone/PropertyPlugin.java | 10 ++- .../simpletype/SimpleTypeEndpoint.java | 8 ++ .../tests/spring/endpoints/frontend/index.ts | 2 + .../endpoints/frontend/src/test-access-mod.ts | 29 +++++++ .../flow/connect/AccessModifierEndpoint.java | 19 +++++ .../ObjectWithDifferentAccessMethods.java | 69 ++++++++++++++++ .../vaadin/flow/connect/AccessModifierIT.java | 81 +++++++++++++++++++ 7 files changed, 216 insertions(+), 2 deletions(-) create mode 100644 packages/java/tests/spring/endpoints/frontend/src/test-access-mod.ts create mode 100644 packages/java/tests/spring/endpoints/src/main/java/com/vaadin/flow/connect/AccessModifierEndpoint.java create mode 100644 packages/java/tests/spring/endpoints/src/main/java/com/vaadin/flow/connect/ObjectWithDifferentAccessMethods.java create mode 100644 packages/java/tests/spring/endpoints/src/test/java/com/vaadin/flow/connect/AccessModifierIT.java diff --git a/packages/java/parser-jvm-plugin-backbone/src/main/java/com/vaadin/hilla/parser/plugins/backbone/PropertyPlugin.java b/packages/java/parser-jvm-plugin-backbone/src/main/java/com/vaadin/hilla/parser/plugins/backbone/PropertyPlugin.java index fa4eced256..c8db40fbbf 100644 --- a/packages/java/parser-jvm-plugin-backbone/src/main/java/com/vaadin/hilla/parser/plugins/backbone/PropertyPlugin.java +++ b/packages/java/parser-jvm-plugin-backbone/src/main/java/com/vaadin/hilla/parser/plugins/backbone/PropertyPlugin.java @@ -1,12 +1,13 @@ package com.vaadin.hilla.parser.plugins.backbone; import java.lang.reflect.InvocationTargetException; -import java.util.Comparator; import java.util.HashMap; import java.util.Objects; import java.util.Set; import java.util.stream.Stream; +import com.fasterxml.jackson.annotation.JsonAutoDetect; +import com.fasterxml.jackson.annotation.PropertyAccessor; import com.fasterxml.jackson.databind.BeanDescription; import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.SerializationConfig; @@ -28,7 +29,12 @@ public final class PropertyPlugin extends AbstractPlugin { private SerializationConfig serializationConfig = new JacksonObjectMapperFactory.Json() - .build().getSerializationConfig(); + .build() + .setVisibility(PropertyAccessor.SETTER, + JsonAutoDetect.Visibility.PUBLIC_ONLY) + .setVisibility(PropertyAccessor.GETTER, + JsonAutoDetect.Visibility.PUBLIC_ONLY) + .getSerializationConfig(); @Override public void enter(NodePath nodePath) { diff --git a/packages/java/parser-jvm-plugin-backbone/src/test/java/com/vaadin/hilla/parser/plugins/backbone/simpletype/SimpleTypeEndpoint.java b/packages/java/parser-jvm-plugin-backbone/src/test/java/com/vaadin/hilla/parser/plugins/backbone/simpletype/SimpleTypeEndpoint.java index 261d138c6f..d031653151 100644 --- a/packages/java/parser-jvm-plugin-backbone/src/test/java/com/vaadin/hilla/parser/plugins/backbone/simpletype/SimpleTypeEndpoint.java +++ b/packages/java/parser-jvm-plugin-backbone/src/test/java/com/vaadin/hilla/parser/plugins/backbone/simpletype/SimpleTypeEndpoint.java @@ -88,4 +88,12 @@ public Short getShortWrapper() { public String getString() { return "test"; }; + + protected String getProtectedValue() { + return "protected"; + } + + protected void setProtectedValue(String protectedValue) { + // ignore + } } diff --git a/packages/java/tests/spring/endpoints/frontend/index.ts b/packages/java/tests/spring/endpoints/frontend/index.ts index aa48758c9b..aa5ec301a2 100644 --- a/packages/java/tests/spring/endpoints/frontend/index.ts +++ b/packages/java/tests/spring/endpoints/frontend/index.ts @@ -2,12 +2,14 @@ import { Router } from '@vaadin/router'; import './src/test-component.js'; import './src/test-flux.js'; import './src/test-type-script.js'; +import './src/test-access-mod.js'; const routes = [ { path: 'flux', component: 'test-flux' }, { path: 'type-script', component: 'test-type-script' }, { path: '', component: 'test-component' }, { path: 'login', component: 'test-component' }, + { path: 'access-mod', component: 'test-access-mod' }, ]; // Vaadin router needs an outlet in the index.html page to display views diff --git a/packages/java/tests/spring/endpoints/frontend/src/test-access-mod.ts b/packages/java/tests/spring/endpoints/frontend/src/test-access-mod.ts new file mode 100644 index 0000000000..a5b705eeb8 --- /dev/null +++ b/packages/java/tests/spring/endpoints/frontend/src/test-access-mod.ts @@ -0,0 +1,29 @@ +import { html, LitElement } from 'lit'; +import { customElement, query } from 'lit/decorators.js'; +import { AccessModifierEndpoint } from 'Frontend/generated/endpoints'; + +@customElement('test-access-mod') +export class TestAccessModifierComponent extends LitElement { + @query('#content') + private content!: HTMLOutputElement; + @query('#methods') + private methods!: HTMLOutputElement; + + render() { + return html` + + + + `; + } + + public async getEntity() { + const entity = await AccessModifierEndpoint.getEntity(); + if (entity === undefined) { + throw new Error('Missing entity object result from endpoint'); + } + + this.content.textContent = JSON.stringify(entity); + this.methods.textContent = Object.keys(entity).join(', '); + } +} diff --git a/packages/java/tests/spring/endpoints/src/main/java/com/vaadin/flow/connect/AccessModifierEndpoint.java b/packages/java/tests/spring/endpoints/src/main/java/com/vaadin/flow/connect/AccessModifierEndpoint.java new file mode 100644 index 0000000000..68c2a4ebf6 --- /dev/null +++ b/packages/java/tests/spring/endpoints/src/main/java/com/vaadin/flow/connect/AccessModifierEndpoint.java @@ -0,0 +1,19 @@ +package com.vaadin.flow.connect; + +import com.vaadin.flow.server.auth.AnonymousAllowed; +import com.vaadin.hilla.Endpoint; +import jakarta.annotation.security.PermitAll; + +/** + * Simple Vaadin Connect Service definition. + */ +@Endpoint +@AnonymousAllowed +class AccessModifierEndpoint { + + @PermitAll + public ObjectWithDifferentAccessMethods getEntity() { + return new ObjectWithDifferentAccessMethods("private", "protected", + "public", "package-private", "public-getter", "public-setter"); + } +} diff --git a/packages/java/tests/spring/endpoints/src/main/java/com/vaadin/flow/connect/ObjectWithDifferentAccessMethods.java b/packages/java/tests/spring/endpoints/src/main/java/com/vaadin/flow/connect/ObjectWithDifferentAccessMethods.java new file mode 100644 index 0000000000..35858c9df3 --- /dev/null +++ b/packages/java/tests/spring/endpoints/src/main/java/com/vaadin/flow/connect/ObjectWithDifferentAccessMethods.java @@ -0,0 +1,69 @@ +package com.vaadin.flow.connect; + +public class ObjectWithDifferentAccessMethods { + private String privateProp; + private String protectedProp; + private String publicProp; + private String packagePrivateProp; + private String publicGetterProp; + private String publicSetterProp; + + public ObjectWithDifferentAccessMethods(String privateProp, + String protectedProp, String publicProp, String packagePrivateProp, + String publicGetterProp, String publicSetterProp) { + this.privateProp = privateProp; + this.protectedProp = protectedProp; + this.publicProp = publicProp; + this.packagePrivateProp = packagePrivateProp; + this.publicGetterProp = publicGetterProp; + this.publicSetterProp = publicSetterProp; + } + + private String getPrivateProp() { + return privateProp; + } + + private void setPrivateProp(String privateProp) { + this.privateProp = privateProp; + } + + protected String getProtectedProp() { + return protectedProp; + } + + protected void setProtectedProp(String protectedProp) { + this.protectedProp = protectedProp; + } + + public String getPublicProp() { + return publicProp; + } + + public void setPublicProp(String publicProp) { + this.publicProp = publicProp; + } + + String getPackagePrivateProp() { + return packagePrivateProp; + } + + void setPackagePrivateProp(String packagePrivateProp) { + this.packagePrivateProp = packagePrivateProp; + } + + public String getPublicGetterProp() { + return publicGetterProp; + } + + private void setPublicGetterProp(String publicGetterProp) { + this.publicGetterProp = publicGetterProp; + } + + private String getPublicSetterProp() { + return publicSetterProp; + } + + public void setPublicSetterProp(String publicSetterProp) { + this.publicSetterProp = publicSetterProp; + } +} diff --git a/packages/java/tests/spring/endpoints/src/test/java/com/vaadin/flow/connect/AccessModifierIT.java b/packages/java/tests/spring/endpoints/src/test/java/com/vaadin/flow/connect/AccessModifierIT.java new file mode 100644 index 0000000000..e0d1fabe35 --- /dev/null +++ b/packages/java/tests/spring/endpoints/src/test/java/com/vaadin/flow/connect/AccessModifierIT.java @@ -0,0 +1,81 @@ +/* + * Copyright 2000-2017 Vaadin Ltd. + * + * 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 com.vaadin.flow.connect; + +import com.vaadin.flow.testutil.ChromeBrowserTest; +import com.vaadin.testbench.TestBenchElement; +import org.checkerframework.checker.units.qual.A; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.openqa.selenium.WebElement; + +/** + * Class for testing issues in a spring-boot container. + */ +public class AccessModifierIT extends ChromeBrowserTest { + + private void openTestUrl(String url) { + getDriver().get(getRootURL() + url); + } + + private TestBenchElement testAccessMod; + private TestBenchElement methods; + + @Override + @Before + public void setup() throws Exception { + super.setup(); + open(); + TestBenchElement testComponent = $("test-component").waitForFirst(); + if (testComponent != null) { + testComponent.$(TestBenchElement.class).id("username") + .sendKeys("user"); + testComponent.$(TestBenchElement.class).id("password") + .sendKeys("user"); + testComponent.$(TestBenchElement.class).id("login").click(); + open(); + } + testAccessMod = $("test-access-mod").waitForFirst(); + methods = testAccessMod.$(TestBenchElement.class).id("methods"); + } + + @Override + protected void open() { + openTestUrl("/access-mod"); + } + + @Test + public void getEntity() { + String endpoint = "getEntity"; + exec(endpoint); + String actualText = waitUntil(driver -> methods.getText(), 25); + Assert.assertNotNull(actualText); + Assert.assertTrue(actualText.contains("publicProp")); + Assert.assertFalse(actualText.contains("protectedProp")); + Assert.assertFalse(actualText.contains("packagePrivateProp")); + Assert.assertFalse(actualText.contains("privateProp")); + Assert.assertTrue(actualText.contains("publicGetterProp")); + // Assert.assertTrue(actualText.contains("publicSetterProp")); // + // property is public, but not present in prototype + } + + private void exec(String id) { + methods.setProperty("innerText", ""); + WebElement button = testAccessMod.$(TestBenchElement.class).id(id); + button.click(); + } +}