stefanseifert commented on code in PR #31: URL: https://github.com/apache/sling-org-apache-sling-testing-osgi-mock/pull/31#discussion_r1356540400
########## core/src/main/java/org/apache/sling/testing/mock/osgi/config/annotations/ConfigType.java: ########## @@ -0,0 +1,79 @@ +/* + * 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.testing.mock.osgi.config.annotations; + +import java.lang.annotation.Repeatable; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +/** + * Defines an instance of an OSGi R7 Component Property Type as a combination of a {@link Class} and an array of strings + * defining property values in the form expected by {@link org.osgi.service.component.annotations.Component#property()}. + * This provides both runtime retention for OSGi config annotations that do not have {@link RetentionPolicy#RUNTIME}, + * allowing for simple construction through reflection for explicit passing to SCR component constructors and lifecycle + * methods, as well as repeatability to support defining sequenced, heterogeneous lists of desired types on any single + * {@link java.lang.reflect.AnnotatedElement}. + * + * @see <a href="https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.component.html#service.component-component.property.types";>Component Property Types</a> + */ +@Repeatable(ConfigTypes.class) +@Retention(RetentionPolicy.RUNTIME) +public @interface ConfigType { + + /** + * Required type to construct. This can be an annotation or an interface. + * + * @return the type to construct + */ + Class<?> type(); + + /** + * Optionally specify a configuration pid to load, any defined properties of which will override annotation defaults + * and values specified by {@link #property()}. In order to specify the name of the {@link #component()} class as a + * configuration PID, set this value to {@link org.osgi.service.component.annotations.Component#NAME}. The default + * value is the empty string, which skips loading any configuration from ConfigurationAdmin. + * + * @return a configuration pid, or an empty string + */ + String pid() default ""; + + /** + * When {@link #pid()} is set to {@link org.osgi.service.component.annotations.Component#NAME}, set this attribute + * to a class whose name should be used instead. + * + * @return the configurable component class + */ + Class<?> component() default Object.class; + + /** + * Treat like {@link org.osgi.service.component.annotations.Component#property()}. + * + * @return osgi component properties + */ + String[] property() default {}; + + /** + * Set to true to throw a {@link org.apache.sling.testing.mock.osgi.config.ConfigTypeSelfTestFailure} on construction + * if there is not an exact one-to-one mapping between property names specified in {@link #property()} and the + * addressable attributes of {@link #type()}. + * + * @return true to perform the self test on construction + */ + boolean selfTest() default false; Review Comment: property "selfTest" sounds strange (also the related exception). probably a name like "strict" or "strictChecking" would be better? or, comparing with mockito, there is a "lenient" mode with strict enabled by default and the option to make to more lenient if wished. i think enabling strict by default would be useful here as well? e.g. to detect orphaned property names nor longer in effect after refactoring the config annotation. ########## core/src/main/java/org/apache/sling/testing/mock/osgi/config/annotations/UpdateConfig.java: ########## @@ -0,0 +1,61 @@ +/* + * 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.testing.mock.osgi.config.annotations; + +import org.osgi.service.component.annotations.Component; + +import java.lang.annotation.Repeatable; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +/** + * Define this annotation on a test class or method to use the {@link org.osgi.service.cm.ConfigurationAdmin} service + * to update the persisted properties for the configuration whose pid matches the {@link #pid()} attribute. + * Updates should be applied top-down for each test context scope, from with the outermost (class-level) to the + * innermost (method-level). + */ +@Repeatable(UpdateConfigs.class) +@Retention(RetentionPolicy.RUNTIME) +public @interface UpdateConfig { + + /** + * Specify a configuration pid to update with values specified by {@link #property()}. The default value is + * {@link Component#NAME}, which is a special string ("$") that can be used to specify the name of the + * {@link #component()} class as a configuration PID. + * + * @return a configuration pid + */ + String pid() default Component.NAME; Review Comment: there is a different in default value between ConfigType ("") and UpdateConfig (Component.NAME), which may be unexpected for the users. wouldn't it be easier to just use the component property when the pid is not set (empty string), and leave out the special treatment of Component.NAME? ########## core/src/main/java/org/apache/sling/testing/mock/osgi/config/annotations/ConfigType.java: ########## @@ -0,0 +1,79 @@ +/* + * 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.testing.mock.osgi.config.annotations; + +import java.lang.annotation.Repeatable; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +/** + * Defines an instance of an OSGi R7 Component Property Type as a combination of a {@link Class} and an array of strings + * defining property values in the form expected by {@link org.osgi.service.component.annotations.Component#property()}. + * This provides both runtime retention for OSGi config annotations that do not have {@link RetentionPolicy#RUNTIME}, + * allowing for simple construction through reflection for explicit passing to SCR component constructors and lifecycle + * methods, as well as repeatability to support defining sequenced, heterogeneous lists of desired types on any single + * {@link java.lang.reflect.AnnotatedElement}. + * + * @see <a href="https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.component.html#service.component-component.property.types";>Component Property Types</a> + */ +@Repeatable(ConfigTypes.class) +@Retention(RetentionPolicy.RUNTIME) +public @interface ConfigType { + + /** + * Required type to construct. This can be an annotation or an interface. + * + * @return the type to construct + */ + Class<?> type(); + + /** + * Optionally specify a configuration pid to load, any defined properties of which will override annotation defaults + * and values specified by {@link #property()}. In order to specify the name of the {@link #component()} class as a + * configuration PID, set this value to {@link org.osgi.service.component.annotations.Component#NAME}. The default + * value is the empty string, which skips loading any configuration from ConfigurationAdmin. + * + * @return a configuration pid, or an empty string + */ + String pid() default ""; Review Comment: see also my comment below on UpdateConfig.pid() - maybe simpler to leave out the special treatment of Component.NAME and look for a set component property if this property is empty ("")? ########## core/src/main/java/org/apache/sling/testing/mock/osgi/config/annotations/UpdateConfig.java: ########## @@ -0,0 +1,61 @@ +/* + * 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.testing.mock.osgi.config.annotations; + +import org.osgi.service.component.annotations.Component; + +import java.lang.annotation.Repeatable; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +/** + * Define this annotation on a test class or method to use the {@link org.osgi.service.cm.ConfigurationAdmin} service + * to update the persisted properties for the configuration whose pid matches the {@link #pid()} attribute. + * Updates should be applied top-down for each test context scope, from with the outermost (class-level) to the + * innermost (method-level). + */ +@Repeatable(UpdateConfigs.class) +@Retention(RetentionPolicy.RUNTIME) +public @interface UpdateConfig { Review Comment: i'm wondering if "SetConfig" would be a more intuitive name? if i read update i always look for a "create" which happened first, or updating something that was already set before. setting something includes creating new or overwriting existing data. it's called update in "ConfigurationAdmin", but there you are getting a configuration instance first hand and then update it which makes sense. but that's not the case for the annotation here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
