kstateome / canvas-api Goto Github PK

Hello:

The assignment object in canvas has an attribute that is not in the model (and curiously is not in the API documentation but it comes when you get assignments).

The attribute name is "secure_params" and it is a JWT string, signed, that contains such interesting things like the ResourceLinkId, that can be used to call the LTI AGS Lineitems endpoint to get the lineitem(s) for that assignment.

Basically the change will be adding this in the Assignment model:

private String secureParams;

    public String getSecureParams() {
        return secureParams;
    }

    public void setSecureParams(String secureParams) {
        this.secureParams = secureParams;
    }

We recently changed the settings on a large number of courses and did this by loading the existing settings, changing a value and saving them back, this meant we sent the whole course settings object back. When we did this it resulted in a large number of notifications going out saying that the grade policy on the course had changed (we hadn't changed it).

It appears theres a bug in Canvas that means that if the default value is false when you query the API you get the false value back, if you then set the value to false users will get a notification that the value has changed.

The workaround (until Instructure fix it) is to create a new course object with just the values that you want to update and save that course back through the API instead.

I don't see any other issues in the repo, so let me know if there's another preferred place to discuss things.

We at UBC have a need to edit sections, including clearing the start_at and end_at properties. With the Canvas REST JSON API this is done by setting them to null. Unfortunately this conflicts with the default behaviour of GSON, which is to not serialize null fields, and GSON only let's you toggle this per GsonBuilder instance. So no field-level control.

Fortunately (for this case) a new builder is created for each request, so it would be possible to add some extra control in the API to affect it. For example:

1. Make this a class-level option. Add a new field to the CanvasObject annotation (or a whole new annotation), say includeNullValues, defaulting to false and use that when building the GsonBuilder instance. This would satisfy our use case, as we initialize the Section model instance with all existing values before applying changes and thus won't lose any data. However any existing clients that are relying on the current behaviour could start losing data.
2. New setters could be created for those fields (setStartAtToNull? clearStartAt?) and the new null behaviour only triggered if they are called. This is obviously safer but not as clean design-wise.
3. Switch to a different JSON library, like Jackson, which let's you scope null handling on a per-field basis. Would still have to do something like 1 and 2, but could limit the nulls to only fields that actually matter.
4. More complicated designs using Optional or better yet a new monad tailored for this case. This would be a lot of work for dubious gain, however.

What does everyone think? Any better ideas?

Of course the root issue is that the model classes are used for both representing values and changes to values, but solving that properly is not trivial with Java.

If you don't depend on log4j explicitly in the project consuming canvas-api then you get an exception when using this library:

java.lang.NoClassDefFoundError: org/apache/log4j/Logger

at edu.ksu.canvas.CanvasApiFactory.<clinit>(CanvasApiFactory.java:26)

log4j should be listed as a dependency.

Hi there!

ListCalendarEventsOptions needs to be able to set "type", currently this is not possible, therefore only events can be fetched, no assignments.

Also, i assume these items should be end_date, not start_date.

public ListCalendarEventsOptions endDate(LocalDate endDate) {
        addSingleItem("start_date", DateTimeFormatter.ISO_LOCAL_DATE.format(endDate));
        return this;
    }

    public ListCalendarEventsOptions endDate(TemporalAccessor endDate) {
        addSingleItem("start_date", DateTimeFormatter.ISO_INSTANT.format(endDate));
        return this;
    }

Hey,

I have a set of users who are running into an interesting error when trying to communicate with their canvas institution.
com.google.gson.JsonSyntaxException: java.lang.NumberFormatException: Expected an int but was 5000000000 at line 1 column 507 path $[0].storage_quota_mb

From the looks of things in the source code a bunch of models use Integer which in Java is a 32 bit number while according to the canvas docs all of their integers are 64 bit.

I would think you would want to change basically all of your Integers in the models to Longs

Canvas api: https://canvas.instructure.com/doc/api/index.html
Right under "Schema" says "All integer ids in Canvas are 64 bit integers. String ids are also used in Canvas."

If I attempt to get an External Tool from Canvas using:

    ExternalToolReader reader = apiFactory.getReader(ExternalToolReader.class, token);
    Optional<ExternalTool> tool = reader.getExternalToolInAccount(accountId, toolId)

then if the tool doesn't exist in Canvas I don't get back an empty Optional but instead a edu.ksu.canvas.exception.ObjectNotFoundException is thrown.

It seems like either when the toolId doesn't exist we should either:

• get an empty Optional
• throw an exception, but not have the method return an Optional
  or is there another reason why we should have both the Optional and the exception?

This is something that I ran into recently while developing a service for Canvas - it happens very, very rarely, but should I get back a 5xx exception, the API will throw an unchecked exception (CanvasException) all the way to the client.

For most normal cases, this should be okay; it is a truly exception scenario which is no fault of the API. However, the context in which this causes a problem is inside of a running ScheduledFuture which actively polls once ever X seconds (where X >= 10). I've noticed that ScheduledFutures don't like to have unhandled exceptions thrown in them, where the ultimate resolution is that the exception will ultimately be exposed once the Future times out. This poses a problem for Futures which aren't meant to time out, or time out after a very long period of time.

The way I'd have to resolve this today would be to catch an unchecked exception, which seems and feels incredibly wrong. I'm posting this more as a question to the maintainers than anything else: would it be appropriate to revise the checkHeaders method in SimpleRequestClient to not throw an exception, and then add a new method to deal with 5xx errors such that a downstream client wouldn't have to catch the exception?

Pretty obvious when you look at BaseImpl, but reader & writer instances are not thread safe when using responseCallback, masqueradeAs and masqueradeType. Actually hit this today while testing, figure it would be nice to at least document it for future users.

The quick fix on my end was to use proxies & ThreadLocal to cache instances.

Please tell me how can I work from files with your library? I interest FilesController :

• Get quota informationList files;
• Get public inline preview url;
• Get file;
• Update file;
• Delete file.

I leave the link: https://canvas.instructure.com/doc/api/files.html

The 1.0.24 release is nearing 18 months old, and there are a number of updates that have been committed that I would like to incorporate into my local projects.

Is it possible to request a new release that incorporates these changes?

Thanks for your consideration.

Hi,

Looking at the Canvas API, Assignment seems to be missing the attribute for AssignmentOverride: assignment[assignment_overrides][]

This might be very helpful to add because it seems like we can create the assignment and the override at the same time in one request rather than making two separate requests - creating an assignment before creating the override for it. We can also read in these overrides on an assignment when we request them to be included in the assignment from Canvas.

As this is a low-level library it can be helpful for libraries to use a logging wrapper rather than a logging framework directly. This was when the API gets integrated with another project it can pickup the implementation that the actual project uses for logging, rather than forcing the consuming project to use log4j.

How would you feel about a PR that switches from log4j to slf4j?

There's an expanded explanation in this post: https://softwareengineering.stackexchange.com/questions/145078/should-you-log-from-library-code