You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by "OyvindLGjesdal (via GitHub)" <gi...@apache.org> on 2024/06/08 22:45:34 UTC
[PR] GH-1611: supply initial value for acceptHeaderGraph to requestConfig [jena]
OyvindLGjesdal opened a new pull request, #2527:
URL: https://github.com/apache/jena/pull/2527
GitHub issue resolved #1611
Pull request Description:
The type is first set on change, due to being updated on the watch phase. Adding it to requestConfig sets it to turtle initially. contentTypeGraph variable was already set as the initial value for turtle, so reusing variable.
<img width="1511" alt="local fuseki with" src="https://github.com/apache/jena/assets/5328572/19f4d3af-fc21-4c5c-bb28-db2b990a6005">
Have also changed to other serializations sucessfully.
By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the [Contributor's Agreement](https://www.apache.org/licenses/contributor-agreements.html).
----
See the [Apache Jena "Contributing" guide](https://github.com/apache/jena/blob/main/CONTRIBUTING.md).
--
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: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1611: supply initial value for acceptHeaderGraph to requestConfig [jena]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #2527:
URL: https://github.com/apache/jena/pull/2527#issuecomment-2156395137
Thanks @OyvindLGjesdal ! Change looks good, let me just check if it needs a test (I can write a quick one if so).
--
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: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1611: supply initial value for acceptHeaderGraph to requestConfig [jena]
Posted by "OyvindLGjesdal (via GitHub)" <gi...@apache.org>.
OyvindLGjesdal commented on PR #2527:
URL: https://github.com/apache/jena/pull/2527#issuecomment-2156759223
Thank you for the review and added tests. I did two cosmetic force pushes and I still managed to have a a formatting issue left in a one-line PR 🤓
--
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: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1611: supply initial value for acceptHeaderGraph to requestConfig [jena]
Posted by "afs (via GitHub)" <gi...@apache.org>.
afs merged PR #2527:
URL: https://github.com/apache/jena/pull/2527
--
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: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1611: supply initial value for acceptHeaderGraph to requestConfig [jena]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on code in PR #2527:
URL: https://github.com/apache/jena/pull/2527#discussion_r1632305942
##########
jena-fuseki2/jena-fuseki-ui/tests/unit/views/dataset/query.vue.spec.js:
##########
@@ -0,0 +1,134 @@
+/**
+ * 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.
+ */
+import { flushPromises, mount } from '@vue/test-utils'
+import { nextTick } from 'vue'
+import Query from '@/views/dataset/Query.vue'
+import { vi } from 'vitest'
+
+const FAKE_FUSEKI_URL = 'https://localhost:1234/fuseki/'
+
+const $routeMock = {
+ query: {}
+}
+
+const mountFunction = options => {
+ const mountOptions = Object.assign(options || {}, {
+ shallow: true,
+ global: {
+ mocks: {
+ $route: $routeMock,
+ $fusekiService: {
+ getFusekiUrl () {
+ return FAKE_FUSEKI_URL
+ }
+ }
+ }
+ }
+ })
+ return mount(Query, {
+ ...mountOptions
+ })
+}
+
+describe('Query view', () => {
+ let yasrDiv
+ let yasqeDiv
+ beforeEach(() => {
+ // DOM elements required by YASQE/YASR.
+ yasrDiv = document.createElement('div')
+ yasrDiv.setAttribute('id', 'yasr')
+ yasqeDiv = document.createElement('div')
+ yasqeDiv.setAttribute('id', 'yasqe')
+ document.body.append(yasrDiv)
+ document.body.append(yasqeDiv)
+ // we will have to mock setTimeout and nextTick at least, for the component with DOM
+ vi.useFakeTimers({
+ toFake: [
+ 'Date',
+ 'nextTick',
+ 'setTimeout'
+ ],
+ shouldAdvanceTime: true
+ })
+ // jsdom doesn't have getBoundingClientRect
+ document.createRange = () => {
+ const range = new Range();
+
+ range.getBoundingClientRect = () => {
+ return {
+ x: 0,
+ y: 0,
+ bottom: 0,
+ height: 0,
+ left: 0,
+ right: 0,
+ top: 0,
+ width: 0,
+ toJSON: () => {}
+ };
+ };
+
+ range.getClientRects = () => {
+ return {
+ // eslint-disable-next-line no-unused-vars
+ item: (index) => null,
+ length: 0,
+ *[Symbol.iterator](){}
+ };
+ };
+
+ return range;
+ }
+ })
+ afterEach(() => {
+ vi.restoreAllMocks()
+ vi.useRealTimers()
+ })
+ it('is created with the correct initial values', async () => {
+ expect(vi.isFakeTimers()).equals(true)
+ const datasetName = 'test'
+ const wrapper = mountFunction({
+ props: {
+ datasetName: datasetName
+ }
+ })
+
+ // Test the prop value.
+ expect(wrapper.vm.$props.datasetName).equals(datasetName)
+
+ // The component needs to interface with DOM due to YASQE, and it contains
+ // a `nextTick`, that calls `setTimeout` (this is what worked in the end,
+ // although probably a `Teleport` could replace it...). So we need to mock
+ // that here. The timeout is of `300ms`, so we move the clock by `400ms`.
+ await nextTick()
+ await vi.advanceTimersByTime(400)
+ await flushPromises()
+
+ // Now YASQE and YASR must have been initialized.
+ expect(wrapper.vm.yasqe).not.equals(null)
+
+ // Test the initial values.
+ const yasqeOptions = wrapper.vm.yasqe.options
+ expect(yasqeOptions.showQueryButton).true
+ expect(yasqeOptions.resizeable).true
+
+ const requestConfig = yasqeOptions.requestConfig
+ expect(await requestConfig.endpoint).equals(FAKE_FUSEKI_URL)
+ // See issue https://github.com/apache/jena/issues/1611
+ expect(requestConfig.acceptHeaderGraph).equals(wrapper.vm.$data.contentTypeGraph)
Review Comment:
Normally views are tested with e2e, but since the logic for YASQE/YASR is not well separated from the view, ended up having to write a unit test (will fix it someday).
The rest of the code is just preparing functions to mount the component, mocking things that are not available without a browser (like the `getBoundingClientRect` that's missing from jsdom), etc.
Then above this test verifies that the initial values are provided to YASQE< including the one for the accept header, which should guarantee we are defining it and YAS/R/QE/GUI should all use work as expected.
I haven't verified that the behaviour @afs reported is fixed (ran out of time today), but I can do that later, or maybe @afs can check it.
I pushed an extra commit with this test and with a formatting issue for spaces before colon (for consistency). Feel free to amend if there's anything wrong, @OyvindLGjesdal . And thanks for the PR!
--
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: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1611: supply initial value for acceptHeaderGraph to requestConfig [jena]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on code in PR #2527:
URL: https://github.com/apache/jena/pull/2527#discussion_r1632251025
##########
jena-fuseki2/jena-fuseki-ui/src/views/dataset/Query.vue:
##########
@@ -296,6 +296,7 @@ export default {
showQueryButton: true,
resizeable: true,
requestConfig: {
+ acceptHeaderGraph : this.contentTypeGraph,
Review Comment:
Extra space after before `:` (rest of the code doesn't use a space before colon).
--
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: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1611: supply initial value for acceptHeaderGraph to requestConfig [jena]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #2527:
URL: https://github.com/apache/jena/pull/2527#issuecomment-2156619892
(BTW, the unit test for a view does not affect the time to run the unit tests, which remains ridiculously fast, thanks to Vite:)
```
yarn run v1.22.19
$ vitest run --environment jsdom
RUN v1.6.0 /home/kinow/Development/java/jena/jena/jena-fuseki2/jena-fuseki-ui
✓ tests/unit/utils/query.spec.js (1)
✓ tests/unit/components/dataset/pagination.vue.spec.js (4)
✓ tests/unit/components/dataset/tablelisting.vue.spec.js (2)
✓ tests/unit/services/fuseki.service.spec.js (20)
✓ tests/unit/components/serverstatus.vue.spec.js (3)
✓ tests/unit/views/dataset/query.vue.spec.js (1) 323ms
Test Files 6 passed (6)
Tests 31 passed (31)
Start at 16:02:43
Duration 3.44s (transform 1.35s, setup 1ms, collect 3.45s, tests 767ms, environment 5.20s, prepare 1.73s)
Done in 4.50s.
```
--
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: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1611: supply initial value for acceptHeaderGraph to requestConfig [jena]
Posted by "afs (via GitHub)" <gi...@apache.org>.
afs commented on PR #2527:
URL: https://github.com/apache/jena/pull/2527#issuecomment-2217330398
LGTM - sorry for the delay reviewing.
--
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: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org